Skip to content

Commit

Permalink
Simplify some Iterator methods.
Browse files Browse the repository at this point in the history
PR #64545 got a big speed-up by replacing a hot call to `all()` with
explicit iteration. This is because the implementation of `all()` is
excessively complex: it wraps the given predicate in a closure that
returns a `LoopState`, passes that closure to `try_for_each()`, which
wraps the first closure in a second closure, passes that second closure
to `try_fold()`, which does the actual iteration using the second
closure.

A sufficient smart compiler could optimize all this away; rustc is
currently not sufficiently smart.

This commit does the following.

- Changes the implementations of `all()`, `any()`, `find()` and
  `find_map()` to use the simplest possible code, rather than using
  `try_for_each()`. (I am reminded of "The Evolution of a Haskell
  Programmer".) These are both shorter and faster than the current
  implementations, and will permit the undoing of the `all()` removal in
  #64545.

- Changes `ResultShunt::next()` so it doesn't call `self.find()`,
  because that was causing infinite recursion with the new
  implementation of `find()`, which itself calls `self.next()`. (I
  honestly don't know how the old implementation of
  `ResultShunt::next()` didn't cause an infinite loop, given that it
  also called `self.next()`, albeit via `try_for_each()` and
  `try_fold()`.)

- Changes `nth()` to use `self.next()` in a while loop rather than `for
  x in self`, because using self-iteration within an iterator method
  seems dubious, and `self.next()` is used in all the other iterator
  methods.
  • Loading branch information
nnethercote committed Sep 18, 2019
1 parent 5670d04 commit 7e98fb9
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 33 deletions.
6 changes: 5 additions & 1 deletion src/libcore/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2404,7 +2404,11 @@ impl<I, T, E> Iterator for ResultShunt<'_, I, E>
type Item = T;

fn next(&mut self) -> Option<Self::Item> {
self.find(|_| true)
match self.iter.next() {
Some(Ok(v)) => Some(v),
Some(Err(e)) => { *self.error = Err(e); None },
None => None,
}
}

fn size_hint(&self) -> (usize, Option<usize>) {
Expand Down
53 changes: 21 additions & 32 deletions src/libcore/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ pub trait Iterator {
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn nth(&mut self, mut n: usize) -> Option<Self::Item> {
for x in self {
while let Some(x) = self.next() {
if n == 0 { return Some(x) }
n -= 1;
}
Expand Down Expand Up @@ -1855,18 +1855,15 @@ pub trait Iterator {
/// ```
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn all<F>(&mut self, f: F) -> bool where
fn all<F>(&mut self, mut f: F) -> bool where
Self: Sized, F: FnMut(Self::Item) -> bool
{
#[inline]
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut(T) -> LoopState<(), ()> {
move |x| {
if f(x) { LoopState::Continue(()) }
else { LoopState::Break(()) }
while let Some(x) = self.next() {
if !f(x) {
return false;
}
}

self.try_for_each(check(f)) == LoopState::Continue(())
true
}

/// Tests if any element of the iterator matches a predicate.
Expand Down Expand Up @@ -1908,19 +1905,16 @@ pub trait Iterator {
/// ```
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn any<F>(&mut self, f: F) -> bool where
fn any<F>(&mut self, mut f: F) -> bool where
Self: Sized,
F: FnMut(Self::Item) -> bool
{
#[inline]
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut(T) -> LoopState<(), ()> {
move |x| {
if f(x) { LoopState::Break(()) }
else { LoopState::Continue(()) }
while let Some(x) = self.next() {
if f(x) {
return true;
}
}

self.try_for_each(check(f)) == LoopState::Break(())
false
}

/// Searches for an element of an iterator that satisfies a predicate.
Expand Down Expand Up @@ -1967,19 +1961,16 @@ pub trait Iterator {
/// ```
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
fn find<P>(&mut self, predicate: P) -> Option<Self::Item> where
fn find<P>(&mut self, mut predicate: P) -> Option<Self::Item> where
Self: Sized,
P: FnMut(&Self::Item) -> bool,
{
#[inline]
fn check<T>(mut predicate: impl FnMut(&T) -> bool) -> impl FnMut(T) -> LoopState<(), T> {
move |x| {
if predicate(&x) { LoopState::Break(x) }
else { LoopState::Continue(()) }
while let Some(x) = self.next() {
if predicate(&x) {
return Some(x);
}
}

self.try_for_each(check(predicate)).break_value()
None
}

/// Applies function to the elements of iterator and returns
Expand All @@ -1999,19 +1990,17 @@ pub trait Iterator {
/// ```
#[inline]
#[stable(feature = "iterator_find_map", since = "1.30.0")]
fn find_map<B, F>(&mut self, f: F) -> Option<B> where
fn find_map<B, F>(&mut self, mut f: F) -> Option<B> where
Self: Sized,
F: FnMut(Self::Item) -> Option<B>,
{
#[inline]
fn check<T, B>(mut f: impl FnMut(T) -> Option<B>) -> impl FnMut(T) -> LoopState<(), B> {
move |x| match f(x) {
Some(x) => LoopState::Break(x),
None => LoopState::Continue(()),
while let Some(x) = self.next() {
if let Some(y) = f(x) {
return Some(y);
}
}
None

self.try_for_each(check(f)).break_value()
}

/// Searches for an element in an iterator, returning its index.
Expand Down

0 comments on commit 7e98fb9

Please sign in to comment.