From 7e98fb944c7ea9548cfc6cb4f273f2537f681275 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 18 Sep 2019 17:24:23 +1000 Subject: [PATCH] Simplify some `Iterator` methods. 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. --- src/libcore/iter/adapters/mod.rs | 6 +++- src/libcore/iter/traits/iterator.rs | 53 ++++++++++++----------------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/libcore/iter/adapters/mod.rs b/src/libcore/iter/adapters/mod.rs index 3b8edc2ad6177..c87834fb0226a 100644 --- a/src/libcore/iter/adapters/mod.rs +++ b/src/libcore/iter/adapters/mod.rs @@ -2404,7 +2404,11 @@ impl Iterator for ResultShunt<'_, I, E> type Item = T; fn next(&mut self) -> Option { - 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) { diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index c09df3f7f22cb..da92b5e382fa7 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -319,7 +319,7 @@ pub trait Iterator { #[inline] #[stable(feature = "rust1", since = "1.0.0")] fn nth(&mut self, mut n: usize) -> Option { - for x in self { + while let Some(x) = self.next() { if n == 0 { return Some(x) } n -= 1; } @@ -1855,18 +1855,15 @@ pub trait Iterator { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] - fn all(&mut self, f: F) -> bool where + fn all(&mut self, mut f: F) -> bool where Self: Sized, F: FnMut(Self::Item) -> bool { - #[inline] - fn check(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. @@ -1908,19 +1905,16 @@ pub trait Iterator { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] - fn any(&mut self, f: F) -> bool where + fn any(&mut self, mut f: F) -> bool where Self: Sized, F: FnMut(Self::Item) -> bool { - #[inline] - fn check(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. @@ -1967,19 +1961,16 @@ pub trait Iterator { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] - fn find

(&mut self, predicate: P) -> Option where + fn find

(&mut self, mut predicate: P) -> Option where Self: Sized, P: FnMut(&Self::Item) -> bool, { - #[inline] - fn check(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 @@ -1999,19 +1990,17 @@ pub trait Iterator { /// ``` #[inline] #[stable(feature = "iterator_find_map", since = "1.30.0")] - fn find_map(&mut self, f: F) -> Option where + fn find_map(&mut self, mut f: F) -> Option where Self: Sized, F: FnMut(Self::Item) -> Option, { - #[inline] - fn check(mut f: impl FnMut(T) -> Option) -> 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.