Skip to content

Commit c9d9df5

Browse files
authored
Rollup merge of rust-lang#59262 - timvermeulen:iterator_cmp_dedup, r=scottmcm
Remove duplicated code from Iterator::{ne, lt, le, gt, ge} This PR delegates `Iterator::ne` to `Iterator::eq` and `Iterator::{lt, le, gt, ge}` to `Iterator::partial_cmp`. Oddly enough, this change actually simplifies the generated assembly [in some cases](https://rust.godbolt.org/z/riBtNe), although I don't understand assembly well enough to see if the longer assembly is doing something clever. I also added two extremely simple benchmarks: ``` // before test iter::bench_lt ... bench: 98,404 ns/iter (+/- 21,008) test iter::bench_partial_cmp ... bench: 62,437 ns/iter (+/- 5,009) // after test iter::bench_lt ... bench: 61,757 ns/iter (+/- 8,770) test iter::bench_partial_cmp ... bench: 62,151 ns/iter (+/- 13,753) ``` I have no idea why the current `lt`/`le`/`gt`/`ge` implementations don't seem to be compiled optimally, but simply having them call `partial_cmp` seems to be an improvement. See rust-lang#44729 for a previous discussion.
2 parents f694222 + 075b269 commit c9d9df5

File tree

2 files changed

+24
-98
lines changed

2 files changed

+24
-98
lines changed

src/libcore/benches/iter.rs

+10
Original file line numberDiff line numberDiff line change
@@ -334,3 +334,13 @@ fn bench_filter_chain_ref_count(b: &mut Bencher) {
334334
(0i64..1000000).chain(0..1000000).map(black_box).by_ref().filter(|x| x % 3 == 0).count()
335335
})
336336
}
337+
338+
#[bench]
339+
fn bench_partial_cmp(b: &mut Bencher) {
340+
b.iter(|| (0..100000).map(black_box).partial_cmp((0..100000).map(black_box)))
341+
}
342+
343+
#[bench]
344+
fn bench_lt(b: &mut Bencher) {
345+
b.iter(|| (0..100000).map(black_box).lt((0..100000).map(black_box)))
346+
}

src/libcore/iter/traits/iterator.rs

+14-98
Original file line numberDiff line numberDiff line change
@@ -2435,145 +2435,61 @@ pub trait Iterator {
24352435
/// Determines if the elements of this `Iterator` are unequal to those of
24362436
/// another.
24372437
#[stable(feature = "iter_order", since = "1.5.0")]
2438-
fn ne<I>(mut self, other: I) -> bool where
2438+
fn ne<I>(self, other: I) -> bool where
24392439
I: IntoIterator,
24402440
Self::Item: PartialEq<I::Item>,
24412441
Self: Sized,
24422442
{
2443-
let mut other = other.into_iter();
2444-
2445-
loop {
2446-
let x = match self.next() {
2447-
None => return other.next().is_some(),
2448-
Some(val) => val,
2449-
};
2450-
2451-
let y = match other.next() {
2452-
None => return true,
2453-
Some(val) => val,
2454-
};
2455-
2456-
if x != y { return true }
2457-
}
2443+
!self.eq(other)
24582444
}
24592445

24602446
/// Determines if the elements of this `Iterator` are lexicographically
24612447
/// less than those of another.
24622448
#[stable(feature = "iter_order", since = "1.5.0")]
2463-
fn lt<I>(mut self, other: I) -> bool where
2449+
fn lt<I>(self, other: I) -> bool where
24642450
I: IntoIterator,
24652451
Self::Item: PartialOrd<I::Item>,
24662452
Self: Sized,
24672453
{
2468-
let mut other = other.into_iter();
2469-
2470-
loop {
2471-
let x = match self.next() {
2472-
None => return other.next().is_some(),
2473-
Some(val) => val,
2474-
};
2475-
2476-
let y = match other.next() {
2477-
None => return false,
2478-
Some(val) => val,
2479-
};
2480-
2481-
match x.partial_cmp(&y) {
2482-
Some(Ordering::Less) => return true,
2483-
Some(Ordering::Equal) => (),
2484-
Some(Ordering::Greater) => return false,
2485-
None => return false,
2486-
}
2487-
}
2454+
self.partial_cmp(other) == Some(Ordering::Less)
24882455
}
24892456

24902457
/// Determines if the elements of this `Iterator` are lexicographically
24912458
/// less or equal to those of another.
24922459
#[stable(feature = "iter_order", since = "1.5.0")]
2493-
fn le<I>(mut self, other: I) -> bool where
2460+
fn le<I>(self, other: I) -> bool where
24942461
I: IntoIterator,
24952462
Self::Item: PartialOrd<I::Item>,
24962463
Self: Sized,
24972464
{
2498-
let mut other = other.into_iter();
2499-
2500-
loop {
2501-
let x = match self.next() {
2502-
None => { other.next(); return true; },
2503-
Some(val) => val,
2504-
};
2505-
2506-
let y = match other.next() {
2507-
None => return false,
2508-
Some(val) => val,
2509-
};
2510-
2511-
match x.partial_cmp(&y) {
2512-
Some(Ordering::Less) => return true,
2513-
Some(Ordering::Equal) => (),
2514-
Some(Ordering::Greater) => return false,
2515-
None => return false,
2516-
}
2465+
match self.partial_cmp(other) {
2466+
Some(Ordering::Less) | Some(Ordering::Equal) => true,
2467+
_ => false,
25172468
}
25182469
}
25192470

25202471
/// Determines if the elements of this `Iterator` are lexicographically
25212472
/// greater than those of another.
25222473
#[stable(feature = "iter_order", since = "1.5.0")]
2523-
fn gt<I>(mut self, other: I) -> bool where
2474+
fn gt<I>(self, other: I) -> bool where
25242475
I: IntoIterator,
25252476
Self::Item: PartialOrd<I::Item>,
25262477
Self: Sized,
25272478
{
2528-
let mut other = other.into_iter();
2529-
2530-
loop {
2531-
let x = match self.next() {
2532-
None => { other.next(); return false; },
2533-
Some(val) => val,
2534-
};
2535-
2536-
let y = match other.next() {
2537-
None => return true,
2538-
Some(val) => val,
2539-
};
2540-
2541-
match x.partial_cmp(&y) {
2542-
Some(Ordering::Less) => return false,
2543-
Some(Ordering::Equal) => (),
2544-
Some(Ordering::Greater) => return true,
2545-
None => return false,
2546-
}
2547-
}
2479+
self.partial_cmp(other) == Some(Ordering::Greater)
25482480
}
25492481

25502482
/// Determines if the elements of this `Iterator` are lexicographically
25512483
/// greater than or equal to those of another.
25522484
#[stable(feature = "iter_order", since = "1.5.0")]
2553-
fn ge<I>(mut self, other: I) -> bool where
2485+
fn ge<I>(self, other: I) -> bool where
25542486
I: IntoIterator,
25552487
Self::Item: PartialOrd<I::Item>,
25562488
Self: Sized,
25572489
{
2558-
let mut other = other.into_iter();
2559-
2560-
loop {
2561-
let x = match self.next() {
2562-
None => return other.next().is_none(),
2563-
Some(val) => val,
2564-
};
2565-
2566-
let y = match other.next() {
2567-
None => return true,
2568-
Some(val) => val,
2569-
};
2570-
2571-
match x.partial_cmp(&y) {
2572-
Some(Ordering::Less) => return false,
2573-
Some(Ordering::Equal) => (),
2574-
Some(Ordering::Greater) => return true,
2575-
None => return false,
2576-
}
2490+
match self.partial_cmp(other) {
2491+
Some(Ordering::Greater) | Some(Ordering::Equal) => true,
2492+
_ => false,
25772493
}
25782494
}
25792495

0 commit comments

Comments
 (0)