From 3bd6e46687b0d7ea347b6bb860bb1c35dfe2e7cc Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 8 Feb 2017 12:19:50 +0100 Subject: [PATCH 1/3] Specialize `PartialOrd 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::>(); 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. --- src/libcore/slice.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 1a482b75731c1..18244cec7c78b 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -2290,6 +2290,28 @@ impl SlicePartialOrd for [A] } } +impl SlicePartialOrd for [A] + where A: Ord +{ + default fn partial_compare(&self, other: &[A]) -> Option { + let l = cmp::min(self.len(), other.len()); + + // Slice to the loop iteration range to enable bound check + // elimination in the compiler + let lhs = &self[..l]; + let rhs = &other[..l]; + + for i in 0..l { + match lhs[i].cmp(&rhs[i]) { + Ordering::Equal => (), + non_eq => return Some(non_eq), + } + } + + self.len().partial_cmp(&other.len()) + } +} + impl SlicePartialOrd for [u8] { #[inline] fn partial_compare(&self, other: &[u8]) -> Option { From ececbb26875c7aa2d9a6b20af5280a700dba11c8 Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 8 Feb 2017 16:01:32 +0100 Subject: [PATCH 2/3] Simplify by calling SliceOrd::compare --- src/libcore/slice.rs | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 18244cec7c78b..7d052c218fe55 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -2294,21 +2294,7 @@ impl SlicePartialOrd for [A] where A: Ord { default fn partial_compare(&self, other: &[A]) -> Option { - let l = cmp::min(self.len(), other.len()); - - // Slice to the loop iteration range to enable bound check - // elimination in the compiler - let lhs = &self[..l]; - let rhs = &other[..l]; - - for i in 0..l { - match lhs[i].cmp(&rhs[i]) { - Ordering::Equal => (), - non_eq => return Some(non_eq), - } - } - - self.len().partial_cmp(&other.len()) + Some(SliceOrd::compare(self, other)) } } From a344c126d03729a9d147f18dfc9cc6432bc790fd Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 8 Feb 2017 18:03:10 +0100 Subject: [PATCH 3/3] Remove unnecessary specialization for [u8] --- src/libcore/slice.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/libcore/slice.rs b/src/libcore/slice.rs index 7d052c218fe55..3e0b842557353 100644 --- a/src/libcore/slice.rs +++ b/src/libcore/slice.rs @@ -2298,13 +2298,6 @@ impl SlicePartialOrd for [A] } } -impl SlicePartialOrd for [u8] { - #[inline] - fn partial_compare(&self, other: &[u8]) -> Option { - Some(SliceOrd::compare(self, other)) - } -} - #[doc(hidden)] // intermediate trait for specialization of slice's Ord trait SliceOrd {