Skip to content

Commit c2258d6

Browse files
committed
Optimise Iterator::{max, max_by, min, min_by}.
The main change in this patch is removing the use of `Option` inside the inner loops of those functions to avoid comparisons where one branch will only trigger on the first pass through the loop. The included benchmarks go from: test bench_max ... bench: 372 ns/iter (+/- 118) test bench_max_by ... bench: 428 ns/iter (+/- 33) test bench_max_by2 ... bench: 7128 ns/iter (+/- 326) to: test bench_max ... bench: 317 ns/iter (+/- 64) test bench_max_by ... bench: 356 ns/iter (+/- 270) test bench_max_by2 ... bench: 1387 ns/iter (+/- 183) Problem noticed in http://www.reddit.com/r/rust/comments/31syce/using_iterators_to_find_the_index_of_the_min_or/
1 parent d9146bf commit c2258d6

File tree

2 files changed

+88
-36
lines changed

2 files changed

+88
-36
lines changed

src/libcore/iter.rs

+57-36
Original file line numberDiff line numberDiff line change
@@ -743,12 +743,12 @@ pub trait Iterator {
743743
#[stable(feature = "rust1", since = "1.0.0")]
744744
fn max(self) -> Option<Self::Item> where Self: Sized, Self::Item: Ord
745745
{
746-
self.fold(None, |max, y| {
747-
match max {
748-
None => Some(y),
749-
Some(x) => Some(cmp::max(x, y))
750-
}
751-
})
746+
select_fold1(self,
747+
|_| (),
748+
// switch to y even if it is only equal, to preserve
749+
// stability.
750+
|_, x, _, y| *x <= *y)
751+
.map(|(_, x)| x)
752752
}
753753

754754
/// Consumes the entire iterator to return the minimum element.
@@ -766,12 +766,12 @@ pub trait Iterator {
766766
#[stable(feature = "rust1", since = "1.0.0")]
767767
fn min(self) -> Option<Self::Item> where Self: Sized, Self::Item: Ord
768768
{
769-
self.fold(None, |min, y| {
770-
match min {
771-
None => Some(y),
772-
Some(x) => Some(cmp::min(x, y))
773-
}
774-
})
769+
select_fold1(self,
770+
|_| (),
771+
// only switch to y if it is strictly smaller, to
772+
// preserve stability.
773+
|_, x, _, y| *x > *y)
774+
.map(|(_, x)| x)
775775
}
776776

777777
/// `min_max` finds the minimum and maximum elements in the iterator.
@@ -869,21 +869,16 @@ pub trait Iterator {
869869
#[inline]
870870
#[unstable(feature = "core",
871871
reason = "may want to produce an Ordering directly; see #15311")]
872-
fn max_by<B: Ord, F>(self, mut f: F) -> Option<Self::Item> where
872+
fn max_by<B: Ord, F>(self, f: F) -> Option<Self::Item> where
873873
Self: Sized,
874874
F: FnMut(&Self::Item) -> B,
875875
{
876-
self.fold(None, |max: Option<(Self::Item, B)>, y| {
877-
let y_val = f(&y);
878-
match max {
879-
None => Some((y, y_val)),
880-
Some((x, x_val)) => if y_val >= x_val {
881-
Some((y, y_val))
882-
} else {
883-
Some((x, x_val))
884-
}
885-
}
886-
}).map(|(x, _)| x)
876+
select_fold1(self,
877+
f,
878+
// switch to y even if it is only equal, to preserve
879+
// stability.
880+
|x_p, _, y_p, _| x_p <= y_p)
881+
.map(|(_, x)| x)
887882
}
888883

889884
/// Return the element that gives the minimum value from the
@@ -903,21 +898,16 @@ pub trait Iterator {
903898
#[inline]
904899
#[unstable(feature = "core",
905900
reason = "may want to produce an Ordering directly; see #15311")]
906-
fn min_by<B: Ord, F>(self, mut f: F) -> Option<Self::Item> where
901+
fn min_by<B: Ord, F>(self, f: F) -> Option<Self::Item> where
907902
Self: Sized,
908903
F: FnMut(&Self::Item) -> B,
909904
{
910-
self.fold(None, |min: Option<(Self::Item, B)>, y| {
911-
let y_val = f(&y);
912-
match min {
913-
None => Some((y, y_val)),
914-
Some((x, x_val)) => if x_val <= y_val {
915-
Some((x, x_val))
916-
} else {
917-
Some((y, y_val))
918-
}
919-
}
920-
}).map(|(x, _)| x)
905+
select_fold1(self,
906+
f,
907+
// only switch to y if it is strictly smaller, to
908+
// preserve stability.
909+
|x_p, _, y_p, _| x_p > y_p)
910+
.map(|(_, x)| x)
921911
}
922912

923913
/// Change the direction of the iterator
@@ -1024,6 +1014,37 @@ pub trait Iterator {
10241014
}
10251015
}
10261016

1017+
/// Select an element from an iterator based on the given projection
1018+
/// and "comparison" function.
1019+
///
1020+
/// This is an idiosyncratic helper to try to factor out the
1021+
/// commonalities of {max,min}{,_by}. In particular, this avoids
1022+
/// having to implement optimisations several times.
1023+
#[inline]
1024+
fn select_fold1<I,B, FProj, FCmp>(mut it: I,
1025+
mut f_proj: FProj,
1026+
mut f_cmp: FCmp) -> Option<(B, I::Item)>
1027+
where I: Iterator,
1028+
FProj: FnMut(&I::Item) -> B,
1029+
FCmp: FnMut(&B, &I::Item, &B, &I::Item) -> bool
1030+
{
1031+
// start with the first element as our selection. This avoids
1032+
// having to use `Option`s inside the loop, translating to a
1033+
// sizeable performance gain (6x in one case).
1034+
it.next().map(|mut sel| {
1035+
let mut sel_p = f_proj(&sel);
1036+
1037+
for x in it {
1038+
let x_p = f_proj(&x);
1039+
if f_cmp(&sel_p, &sel, &x_p, &x) {
1040+
sel = x;
1041+
sel_p = x_p;
1042+
}
1043+
}
1044+
(sel_p, sel)
1045+
})
1046+
}
1047+
10271048
#[stable(feature = "rust1", since = "1.0.0")]
10281049
impl<'a, I: Iterator + ?Sized> Iterator for &'a mut I {
10291050
type Item = I::Item;

src/libcoretest/iter.rs

+31
Original file line numberDiff line numberDiff line change
@@ -901,3 +901,34 @@ fn bench_multiple_take(b: &mut Bencher) {
901901
}
902902
});
903903
}
904+
905+
fn scatter(x: i32) -> i32 { (x * 31) % 127 }
906+
907+
#[bench]
908+
fn bench_max_by(b: &mut Bencher) {
909+
b.iter(|| {
910+
let it = 0..100;
911+
it.max_by(|&x| scatter(x))
912+
})
913+
}
914+
915+
// http://www.reddit.com/r/rust/comments/31syce/using_iterators_to_find_the_index_of_the_min_or/
916+
#[bench]
917+
fn bench_max_by2(b: &mut Bencher) {
918+
fn max_index_iter(array: &[i32]) -> usize {
919+
array.iter().enumerate().max_by(|&(_, item)| item).unwrap().0
920+
}
921+
922+
let mut data = vec![0i32; 1638];
923+
data[514] = 9999;
924+
925+
b.iter(|| max_index_iter(&data));
926+
}
927+
928+
#[bench]
929+
fn bench_max(b: &mut Bencher) {
930+
b.iter(|| {
931+
let it = 0..100;
932+
it.map(scatter).max()
933+
})
934+
}

0 commit comments

Comments
 (0)