Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core::iter: Refactor iterators #16225

Closed
wants to merge 1 commit into from

Conversation

pczarn
Copy link
Contributor

@pczarn pczarn commented Aug 3, 2014

Simplifying the code of methods: nth, fold, rposition, and iterators: Filter, FilterMap, SkipWhile.

before
test iter::bench_multiple_take      ... bench:        15 ns/iter (+/- 0)
test iter::bench_rposition          ... bench:       349 ns/iter (+/- 94)
test iter::bench_skip_while         ... bench:       158 ns/iter (+/- 6)

after
test iter::bench_multiple_take      ... bench:        15 ns/iter (+/- 0)
test iter::bench_rposition          ... bench:       314 ns/iter (+/- 2)
test iter::bench_skip_while         ... bench:       107 ns/iter (+/- 0)

@koalazen has the code for Skip.

Once #16011 is fixed, min_max could use a for loop.

@@ -720,21 +715,10 @@ pub trait ExactSize<A> : DoubleEndedIterator<A> {
/// If no element matches, None is returned.
#[inline]
fn rposition(&mut self, predicate: |A| -> bool) -> Option<uint> {
let (lower, upper) = self.size_hint();
assert!(upper == Some(lower));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the removal of this is why you're reporting better numbers on the benchmark.

Right now clients of ExactSize typically include this test. I don't think all clients should be including the test, instead len() should do any assertions necessary, but I'm not sure if sneaking that into this PR is appropriate (especially since you're only updating one client and not all).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't len() (as defined just below) already include this same assertion, though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it already does the assertions? Then why is everyone using size_hint() and asserting manually?

@lilyball
Copy link
Contributor

lilyball commented Aug 6, 2014

r=me with the one change.

Simplifying the code of methods: nth, fold, rposition
and iterators: Filter, FilterMap, SkipWhile
Adding basic benchmarks
@pczarn
Copy link
Contributor Author

pczarn commented Aug 6, 2014

@kballard Pushed the change.

bors added a commit that referenced this pull request Aug 6, 2014
Simplifying the code of methods: `nth`, `fold`, `rposition`, and iterators: `Filter`, `FilterMap`, `SkipWhile`.

```
before
test iter::bench_multiple_take      ... bench:        15 ns/iter (+/- 0)
test iter::bench_rposition          ... bench:       349 ns/iter (+/- 94)
test iter::bench_skip_while         ... bench:       158 ns/iter (+/- 6)

after
test iter::bench_multiple_take      ... bench:        15 ns/iter (+/- 0)
test iter::bench_rposition          ... bench:       314 ns/iter (+/- 2)
test iter::bench_skip_while         ... bench:       107 ns/iter (+/- 0)
```
@koalazen has the code for `Skip`.

Once #16011 is fixed, `min_max` could use a for loop.
@bors bors closed this Aug 6, 2014
@pczarn pczarn deleted the iter-refactoring branch January 14, 2015 16:56
lnicola pushed a commit to lnicola/rust that referenced this pull request Jan 3, 2024
minor: Render more crate information in status command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants