Skip to content

Commit

Permalink
Auto merge of #39642 - stjepang:specialize-slice-partialord, r=alexcr…
Browse files Browse the repository at this point in the history
…ichton

Specialize `PartialOrd<A> for [A] where A: Ord`

This way we can call `cmp` instead of `partial_cmp` in the loop, removing some burden of optimizing `Option`s away from the compiler.

PR #39538 introduced a regression where sorting slices suddenly became slower, since `slice1.lt(slice2)` was much slower than `slice1.cmp(slice2) == Less`. This problem is now fixed.

To verify, I benchmarked this simple program:
```rust
fn main() {
    let mut v = (0..2_000_000).map(|x| x * x * x * 18913515181).map(|x| vec![x, x ^ 3137831591]).collect::<Vec<_>>();
    v.sort();
}
```

Before this PR, it would take 0.95 sec, and now it takes 0.58 sec.
I also tried changing the `is_less` lambda to use `cmp` and `partial_cmp`. Now all three versions (`lt`, `cmp`, `partial_cmp`) are equally performant for sorting slices - all of them take 0.58 sec on the
benchmark.

Tangentially, as soon as we get `default impl`, it might be a good idea to implement a blanket default impl for `lt`, `gt`, `le`, `ge` in terms of `cmp` whenever possible. Today, those four functions by default are only implemented in terms of `partial_cmp`.

r? @alexcrichton
  • Loading branch information
bors committed Feb 11, 2017
2 parents 064a0ee + a344c12 commit f140a6c
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/libcore/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2290,9 +2290,10 @@ impl<A> SlicePartialOrd<A> for [A]
}
}

impl SlicePartialOrd<u8> for [u8] {
#[inline]
fn partial_compare(&self, other: &[u8]) -> Option<Ordering> {
impl<A> SlicePartialOrd<A> for [A]
where A: Ord
{
default fn partial_compare(&self, other: &[A]) -> Option<Ordering> {
Some(SliceOrd::compare(self, other))
}
}
Expand Down

0 comments on commit f140a6c

Please sign in to comment.