Skip to content

Commit

Permalink
Rollup merge of rust-lang#59262 - timvermeulen:iterator_cmp_dedup, r=…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Centril committed Apr 2, 2019
2 parents f694222 + 075b269 commit c9d9df5
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 98 deletions.
10 changes: 10 additions & 0 deletions src/libcore/benches/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,3 +334,13 @@ fn bench_filter_chain_ref_count(b: &mut Bencher) {
(0i64..1000000).chain(0..1000000).map(black_box).by_ref().filter(|x| x % 3 == 0).count()
})
}

#[bench]
fn bench_partial_cmp(b: &mut Bencher) {
b.iter(|| (0..100000).map(black_box).partial_cmp((0..100000).map(black_box)))
}

#[bench]
fn bench_lt(b: &mut Bencher) {
b.iter(|| (0..100000).map(black_box).lt((0..100000).map(black_box)))
}
112 changes: 14 additions & 98 deletions src/libcore/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2435,145 +2435,61 @@ pub trait Iterator {
/// Determines if the elements of this `Iterator` are unequal to those of
/// another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn ne<I>(mut self, other: I) -> bool where
fn ne<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialEq<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_some(),
Some(val) => val,
};

let y = match other.next() {
None => return true,
Some(val) => val,
};

if x != y { return true }
}
!self.eq(other)
}

/// Determines if the elements of this `Iterator` are lexicographically
/// less than those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn lt<I>(mut self, other: I) -> bool where
fn lt<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_some(),
Some(val) => val,
};

let y = match other.next() {
None => return false,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return true,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return false,
None => return false,
}
}
self.partial_cmp(other) == Some(Ordering::Less)
}

/// Determines if the elements of this `Iterator` are lexicographically
/// less or equal to those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn le<I>(mut self, other: I) -> bool where
fn le<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => { other.next(); return true; },
Some(val) => val,
};

let y = match other.next() {
None => return false,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return true,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return false,
None => return false,
}
match self.partial_cmp(other) {
Some(Ordering::Less) | Some(Ordering::Equal) => true,
_ => false,
}
}

/// Determines if the elements of this `Iterator` are lexicographically
/// greater than those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn gt<I>(mut self, other: I) -> bool where
fn gt<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => { other.next(); return false; },
Some(val) => val,
};

let y = match other.next() {
None => return true,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return false,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return true,
None => return false,
}
}
self.partial_cmp(other) == Some(Ordering::Greater)
}

/// Determines if the elements of this `Iterator` are lexicographically
/// greater than or equal to those of another.
#[stable(feature = "iter_order", since = "1.5.0")]
fn ge<I>(mut self, other: I) -> bool where
fn ge<I>(self, other: I) -> bool where
I: IntoIterator,
Self::Item: PartialOrd<I::Item>,
Self: Sized,
{
let mut other = other.into_iter();

loop {
let x = match self.next() {
None => return other.next().is_none(),
Some(val) => val,
};

let y = match other.next() {
None => return true,
Some(val) => val,
};

match x.partial_cmp(&y) {
Some(Ordering::Less) => return false,
Some(Ordering::Equal) => (),
Some(Ordering::Greater) => return true,
None => return false,
}
match self.partial_cmp(other) {
Some(Ordering::Greater) | Some(Ordering::Equal) => true,
_ => false,
}
}

Expand Down

0 comments on commit c9d9df5

Please sign in to comment.