From ea6a1bdf6ba754f3b96e3a46f9173b17ff18ed07 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 1 Mar 2018 10:46:06 +0000 Subject: [PATCH 01/15] Compute each key only one during slice::sort_by_key --- src/liballoc/slice.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index dc40062ef13df..c57f1e7f57297 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1328,10 +1328,18 @@ impl [T] { /// ``` #[stable(feature = "slice_sort_by_key", since = "1.7.0")] #[inline] - pub fn sort_by_key(&mut self, mut f: F) + pub fn sort_by_key(&mut self, f: F) where F: FnMut(&T) -> B, B: Ord { - merge_sort(self, |a, b| f(a).lt(&f(b))); + let mut indices: Vec<_> = self.iter().map(f).enumerate().map(|(i, k)| (k, i)).collect(); + indices.sort(); + for i in 0..self.len() { + let mut index = indices[i].1; + while index < i { + index = indices[index].1; + } + self.swap(i, index); + } } /// Sorts the slice, but may not preserve the order of equal elements. From 670e69e20753e2ef33ee61b0515092cace3fd716 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 1 Mar 2018 12:15:05 +0000 Subject: [PATCH 02/15] Update documentation for sort_by_key --- src/liballoc/slice.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index c57f1e7f57297..db3b14859dddb 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1302,11 +1302,9 @@ impl [T] { /// Sorts the slice with a key extraction function. /// - /// This sort is stable (i.e. does not reorder equal elements) and `O(n log n)` worst-case. - /// - /// When applicable, unstable sorting is preferred because it is generally faster than stable - /// sorting and it doesn't allocate auxiliary memory. - /// See [`sort_unstable_by_key`](#method.sort_unstable_by_key). + /// During sorting, the key function is called only once per element. + /// This sort is stable (i.e. does not reorder equal elements) and `O(m n + n log n)` + /// worst-case, where the key function is `O(m)`. /// /// # Current implementation /// @@ -1315,8 +1313,7 @@ impl [T] { /// It is designed to be very fast in cases where the slice is nearly sorted, or consists of /// two or more sorted sequences concatenated one after another. /// - /// Also, it allocates temporary storage half the size of `self`, but for short slices a - /// non-allocating insertion sort is used instead. + /// The algorithm allocates temporary storage the size of `self`. /// /// # Examples /// From b8452cc3266f2d9567b5c07ca3044b3960e10ebf Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 1 Mar 2018 12:22:11 +0000 Subject: [PATCH 03/15] Clarify behaviour of sort_unstable_by_key with respect to sort_by_key --- src/liballoc/slice.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index db3b14859dddb..63b22bd0f6d87 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1303,6 +1303,7 @@ impl [T] { /// Sorts the slice with a key extraction function. /// /// During sorting, the key function is called only once per element. + /// /// This sort is stable (i.e. does not reorder equal elements) and `O(m n + n log n)` /// worst-case, where the key function is `O(m)`. /// @@ -1329,7 +1330,10 @@ impl [T] { where F: FnMut(&T) -> B, B: Ord { let mut indices: Vec<_> = self.iter().map(f).enumerate().map(|(i, k)| (k, i)).collect(); - indices.sort(); + // The elements of `indices` are unique, as they are indexed, so any sort will be stable + // with respect to the original slice. We use `sort_unstable` here because it requires less + // memory allocation. + indices.sort_unstable(); for i in 0..self.len() { let mut index = indices[i].1; while index < i { @@ -1414,8 +1418,11 @@ impl [T] { /// Sorts the slice with a key extraction function, but may not preserve the order of equal /// elements. /// + /// Note that, currently, the key function for `sort_unstable_by_key` is called multiple times + /// per element, unlike `sort_stable_by_key`. + /// /// This sort is unstable (i.e. may reorder equal elements), in-place (i.e. does not allocate), - /// and `O(n log n)` worst-case. + /// and `O(m n log m n)` worst-case, where the key function is `O(m)`. /// /// # Current implementation /// @@ -1425,8 +1432,8 @@ impl [T] { /// randomization to avoid degenerate cases, but with a fixed seed to always provide /// deterministic behavior. /// - /// It is typically faster than stable sorting, except in a few special cases, e.g. when the - /// slice consists of several concatenated sorted sequences. + /// Due to its key calling strategy, `sort_unstable_by_key` is likely to be slower than + /// `sort_by_key` in cases where the key function is expensive. /// /// # Examples /// From 9fbee359d7eb3a621604bc2067a375f6d4b757e5 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 1 Mar 2018 13:39:52 +0000 Subject: [PATCH 04/15] Add a test for sort_by_key --- src/liballoc/tests/slice.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/liballoc/tests/slice.rs b/src/liballoc/tests/slice.rs index d9e9d91cea88a..300f6abaa7f1b 100644 --- a/src/liballoc/tests/slice.rs +++ b/src/liballoc/tests/slice.rs @@ -425,6 +425,11 @@ fn test_sort() { v.sort_by(|a, b| b.cmp(a)); assert!(v.windows(2).all(|w| w[0] >= w[1])); + // Sort in lexicographic order. + let mut v = orig.clone(); + v.sort_by_key(|x| x.to_string()); + assert!(v.windows(2).all(|w| w[0].to_string() <= w[1].to_string())); + // Sort with many pre-sorted runs. let mut v = orig.clone(); v.sort(); From 21fde0903b394ca4765b17321a736983c43886cb Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 1 Mar 2018 16:07:58 +0000 Subject: [PATCH 05/15] Update documentation --- src/liballoc/slice.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index 63b22bd0f6d87..32ade0b017833 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1314,7 +1314,7 @@ impl [T] { /// It is designed to be very fast in cases where the slice is nearly sorted, or consists of /// two or more sorted sequences concatenated one after another. /// - /// The algorithm allocates temporary storage the size of `self`. + /// The algorithm allocates temporary storage in a `Vec<(K, usize)` the length of the slice. /// /// # Examples /// @@ -1326,8 +1326,8 @@ impl [T] { /// ``` #[stable(feature = "slice_sort_by_key", since = "1.7.0")] #[inline] - pub fn sort_by_key(&mut self, f: F) - where F: FnMut(&T) -> B, B: Ord + pub fn sort_by_key(&mut self, f: F) + where F: FnMut(&T) -> K, K: Ord { let mut indices: Vec<_> = self.iter().map(f).enumerate().map(|(i, k)| (k, i)).collect(); // The elements of `indices` are unique, as they are indexed, so any sort will be stable @@ -1418,8 +1418,8 @@ impl [T] { /// Sorts the slice with a key extraction function, but may not preserve the order of equal /// elements. /// - /// Note that, currently, the key function for `sort_unstable_by_key` is called multiple times - /// per element, unlike `sort_stable_by_key`. + /// Note that, currently, the key function for [`sort_unstable_by_key`] is called multiple times + /// per element, unlike [`sort_by_key`]. /// /// This sort is unstable (i.e. may reorder equal elements), in-place (i.e. does not allocate), /// and `O(m n log m n)` worst-case, where the key function is `O(m)`. @@ -1432,8 +1432,8 @@ impl [T] { /// randomization to avoid degenerate cases, but with a fixed seed to always provide /// deterministic behavior. /// - /// Due to its key calling strategy, `sort_unstable_by_key` is likely to be slower than - /// `sort_by_key` in cases where the key function is expensive. + /// Due to its key calling strategy, [`sort_unstable_by_key`] is likely to be slower than + /// [`sort_by_key`] in cases where the key function is expensive. /// /// # Examples /// @@ -1444,12 +1444,13 @@ impl [T] { /// assert!(v == [1, 2, -3, 4, -5]); /// ``` /// + /// [`sort_by_key`]: #method.sort_by_key + /// [`sort_unstable_by_key`]: #method.sort_unstable_by_key /// [pdqsort]: https://github.com/orlp/pdqsort #[stable(feature = "sort_unstable", since = "1.20.0")] #[inline] - pub fn sort_unstable_by_key(&mut self, f: F) - where F: FnMut(&T) -> B, - B: Ord + pub fn sort_unstable_by_key(&mut self, f: F) + where F: FnMut(&T) -> K, K: Ord { core_slice::SliceExt::sort_unstable_by_key(self, f); } From 7dcfc07d2c441e6e18c34dfe844c7bdc1c0137fe Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 2 Mar 2018 15:51:20 +0000 Subject: [PATCH 06/15] Cull the quadratic --- src/liballoc/slice.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index 32ade0b017833..3fa5e78c04f4e 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1339,6 +1339,7 @@ impl [T] { while index < i { index = indices[index].1; } + indices[i].1 = index; self.swap(i, index); } } From bdcc6f939a10bc23a434b2ef301238650841dec9 Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 2 Mar 2018 16:28:55 +0000 Subject: [PATCH 07/15] Index enumeration by minimally sized type --- src/liballoc/slice.rs | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index 3fa5e78c04f4e..fbd8d87930842 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -102,6 +102,7 @@ use core::mem::size_of; use core::mem; use core::ptr; use core::slice as core_slice; +use core::{u8, u16, u32}; use borrow::{Borrow, BorrowMut, ToOwned}; use boxed::Box; @@ -1329,19 +1330,31 @@ impl [T] { pub fn sort_by_key(&mut self, f: F) where F: FnMut(&T) -> K, K: Ord { - let mut indices: Vec<_> = self.iter().map(f).enumerate().map(|(i, k)| (k, i)).collect(); - // The elements of `indices` are unique, as they are indexed, so any sort will be stable - // with respect to the original slice. We use `sort_unstable` here because it requires less - // memory allocation. - indices.sort_unstable(); - for i in 0..self.len() { - let mut index = indices[i].1; - while index < i { - index = indices[index].1; - } - indices[i].1 = index; - self.swap(i, index); + // Helper macro for indexing our vector by the smallest possible type, to reduce allocation. + macro_rules! sort_by_key { + ($t:ty, $slice:ident, $f:ident) => ({ + let mut indices: Vec<_> = + $slice.iter().map($f).enumerate().map(|(i, k)| (k, i as $t)).collect(); + // The elements of `indices` are unique, as they are indexed, so any sort will be + // stable with respect to the original slice. We use `sort_unstable` here because it + // requires less memory allocation. + indices.sort_unstable(); + for i in 0..$slice.len() { + let mut index = indices[i].1; + while (index as usize) < i { + index = indices[index as usize].1; + } + indices[i].1 = index; + $slice.swap(i, index as usize); + } + }) } + + let len = self.len(); + if len <= ( u8::MAX as usize) { return sort_by_key!( u8, self, f) } + if len <= (u16::MAX as usize) { return sort_by_key!(u16, self, f) } + if len <= (u32::MAX as usize) { return sort_by_key!(u32, self, f) } + sort_by_key!(usize, self, f) } /// Sorts the slice, but may not preserve the order of equal elements. From f41a26f2040dfa752825a7d1fdfbd5a8ae3310cf Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 16 Mar 2018 14:37:17 +0000 Subject: [PATCH 08/15] Add sort_by_cached_key method --- src/liballoc/slice.rs | 61 ++++++++++++++++++++++++++++++------- src/liballoc/tests/slice.rs | 9 ++++-- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index fbd8d87930842..306c467f048cb 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1301,6 +1301,45 @@ impl [T] { merge_sort(self, |a, b| compare(a, b) == Less); } + /// Sorts the slice with a key extraction function. + /// + /// This sort is stable (i.e. does not reorder equal elements) and `O(m n log m n)` + /// worst-case, where the key function is `O(m)`. + /// + /// For expensive key functions (e.g. functions that are not simple property accesses or + /// basic operations), [`sort_by_cached_key`](#method.sort_by_cached_key) is likely to be + /// significantly faster, as it does not recompute element keys. + /// + /// When applicable, unstable sorting is preferred because it is generally faster than stable + /// sorting and it doesn't allocate auxiliary memory. + /// See [`sort_unstable_by_key`](#method.sort_unstable_by_key). + /// + /// # Current implementation + /// + /// The current algorithm is an adaptive, iterative merge sort inspired by + /// [timsort](https://en.wikipedia.org/wiki/Timsort). + /// It is designed to be very fast in cases where the slice is nearly sorted, or consists of + /// two or more sorted sequences concatenated one after another. + /// + /// Also, it allocates temporary storage half the size of `self`, but for short slices a + /// non-allocating insertion sort is used instead. + /// + /// # Examples + /// + /// ``` + /// let mut v = [-5i32, 4, 1, -3, 2]; + /// + /// v.sort_by_key(|k| k.abs()); + /// assert!(v == [1, 2, -3, 4, -5]); + /// ``` + #[stable(feature = "slice_sort_by_key", since = "1.7.0")] + #[inline] + pub fn sort_by_key(&mut self, mut f: F) + where F: FnMut(&T) -> K, K: Ord + { + merge_sort(self, |a, b| f(a).lt(&f(b))); + } + /// Sorts the slice with a key extraction function. /// /// During sorting, the key function is called only once per element. @@ -1308,6 +1347,10 @@ impl [T] { /// This sort is stable (i.e. does not reorder equal elements) and `O(m n + n log n)` /// worst-case, where the key function is `O(m)`. /// + /// For simple key functions (e.g. functions that are property accesses or + /// basic operations), [`sort_by_key`](#method.sort_by_key) is likely to be + /// faster. + /// /// # Current implementation /// /// The current algorithm is an adaptive, iterative merge sort inspired by @@ -1315,19 +1358,19 @@ impl [T] { /// It is designed to be very fast in cases where the slice is nearly sorted, or consists of /// two or more sorted sequences concatenated one after another. /// - /// The algorithm allocates temporary storage in a `Vec<(K, usize)` the length of the slice. + /// The algorithm allocates temporary storage in a `Vec<(K, usize)>` the length of the slice. /// /// # Examples /// /// ``` /// let mut v = [-5i32, 4, 1, -3, 2]; /// - /// v.sort_by_key(|k| k.abs()); + /// v.sort_by_cached_key(|k| k.abs()); /// assert!(v == [1, 2, -3, 4, -5]); /// ``` - #[stable(feature = "slice_sort_by_key", since = "1.7.0")] + #[unstable(feature = "slice_sort_by_uncached_key", issue = "34447")] #[inline] - pub fn sort_by_key(&mut self, f: F) + pub fn sort_by_cached_key(&mut self, f: F) where F: FnMut(&T) -> K, K: Ord { // Helper macro for indexing our vector by the smallest possible type, to reduce allocation. @@ -1432,9 +1475,6 @@ impl [T] { /// Sorts the slice with a key extraction function, but may not preserve the order of equal /// elements. /// - /// Note that, currently, the key function for [`sort_unstable_by_key`] is called multiple times - /// per element, unlike [`sort_by_key`]. - /// /// This sort is unstable (i.e. may reorder equal elements), in-place (i.e. does not allocate), /// and `O(m n log m n)` worst-case, where the key function is `O(m)`. /// @@ -1446,8 +1486,9 @@ impl [T] { /// randomization to avoid degenerate cases, but with a fixed seed to always provide /// deterministic behavior. /// - /// Due to its key calling strategy, [`sort_unstable_by_key`] is likely to be slower than - /// [`sort_by_key`] in cases where the key function is expensive. + /// Due to its key calling strategy, [`sort_unstable_by_key`](#method.sort_unstable_by_key) + /// is likely to be slower than [`sort_by_cached_key`](#method.sort_by_uncached_key) in + /// cases where the key function is expensive. /// /// # Examples /// @@ -1458,8 +1499,6 @@ impl [T] { /// assert!(v == [1, 2, -3, 4, -5]); /// ``` /// - /// [`sort_by_key`]: #method.sort_by_key - /// [`sort_unstable_by_key`]: #method.sort_unstable_by_key /// [pdqsort]: https://github.com/orlp/pdqsort #[stable(feature = "sort_unstable", since = "1.20.0")] #[inline] diff --git a/src/liballoc/tests/slice.rs b/src/liballoc/tests/slice.rs index 300f6abaa7f1b..7d4dac1c5ec99 100644 --- a/src/liballoc/tests/slice.rs +++ b/src/liballoc/tests/slice.rs @@ -426,9 +426,12 @@ fn test_sort() { assert!(v.windows(2).all(|w| w[0] >= w[1])); // Sort in lexicographic order. - let mut v = orig.clone(); - v.sort_by_key(|x| x.to_string()); - assert!(v.windows(2).all(|w| w[0].to_string() <= w[1].to_string())); + let mut v1 = orig.clone(); + let mut v2 = orig.clone(); + v1.sort_by_key(|x| x.to_string()); + v2.sort_by_cached_key(|x| x.to_string()); + assert!(v1.windows(2).all(|w| w[0].to_string() <= w[1].to_string())); + assert!(v1 == v2); // Sort with many pre-sorted runs. let mut v = orig.clone(); From b430cba343743783cee517bf93c7f255827cccc5 Mon Sep 17 00:00:00 2001 From: varkor Date: Fri, 16 Mar 2018 16:35:27 +0000 Subject: [PATCH 09/15] Fix use of unstable feature in test --- src/liballoc/lib.rs | 1 + src/liballoc/slice.rs | 4 ++-- src/liballoc/tests/lib.rs | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index b93e128d50819..253b8acc16c42 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -113,6 +113,7 @@ #![feature(slice_get_slice)] #![feature(slice_patterns)] #![feature(slice_rsplit)] +#![feature(slice_sort_by_cached_key)] #![feature(specialization)] #![feature(staged_api)] #![feature(str_internals)] diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index 306c467f048cb..db8900664476e 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1368,7 +1368,7 @@ impl [T] { /// v.sort_by_cached_key(|k| k.abs()); /// assert!(v == [1, 2, -3, 4, -5]); /// ``` - #[unstable(feature = "slice_sort_by_uncached_key", issue = "34447")] + #[unstable(feature = "slice_sort_by_cached_key", issue = "34447")] #[inline] pub fn sort_by_cached_key(&mut self, f: F) where F: FnMut(&T) -> K, K: Ord @@ -1487,7 +1487,7 @@ impl [T] { /// deterministic behavior. /// /// Due to its key calling strategy, [`sort_unstable_by_key`](#method.sort_unstable_by_key) - /// is likely to be slower than [`sort_by_cached_key`](#method.sort_by_uncached_key) in + /// is likely to be slower than [`sort_by_cached_key`](#method.sort_by_cached_key) in /// cases where the key function is expensive. /// /// # Examples diff --git a/src/liballoc/tests/lib.rs b/src/liballoc/tests/lib.rs index 285cba0270c03..27f3028a51343 100644 --- a/src/liballoc/tests/lib.rs +++ b/src/liballoc/tests/lib.rs @@ -23,6 +23,7 @@ #![feature(pattern)] #![feature(placement_in_syntax)] #![feature(rand)] +#![feature(slice_sort_by_cached_key)] #![feature(splice)] #![feature(str_escape)] #![feature(string_retain)] From ca3bed0c66d27fbf30eb48ae3eb5af235669364d Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 17 Mar 2018 20:18:08 +0000 Subject: [PATCH 10/15] Improve and fix documentation for sort_by_cached_key --- src/liballoc/slice.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index db8900664476e..bef50a733cc76 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1353,21 +1353,25 @@ impl [T] { /// /// # Current implementation /// - /// The current algorithm is an adaptive, iterative merge sort inspired by - /// [timsort](https://en.wikipedia.org/wiki/Timsort). - /// It is designed to be very fast in cases where the slice is nearly sorted, or consists of - /// two or more sorted sequences concatenated one after another. + /// The current algorithm is based on [pattern-defeating quicksort][pdqsort] by Orson Peters, + /// which combines the fast average case of randomized quicksort with the fast worst case of + /// heapsort, while achieving linear time on slices with certain patterns. It uses some + /// randomization to avoid degenerate cases, but with a fixed seed to always provide + /// deterministic behavior. /// - /// The algorithm allocates temporary storage in a `Vec<(K, usize)>` the length of the slice. + /// In the worst case, the algorithm allocates temporary storage in a `Vec<(K, usize)>` the + /// length of the slice. /// /// # Examples /// /// ``` - /// let mut v = [-5i32, 4, 1, -3, 2]; + /// let mut v = [-5i32, 4, 32, -3, 2]; /// - /// v.sort_by_cached_key(|k| k.abs()); - /// assert!(v == [1, 2, -3, 4, -5]); + /// v.sort_by_cached_key(|k| k.to_string()); + /// assert!(v == [-3, -5, 2, 32, 4]); /// ``` + /// + /// [pdqsort]: https://github.com/orlp/pdqsort #[unstable(feature = "slice_sort_by_cached_key", issue = "34447")] #[inline] pub fn sort_by_cached_key(&mut self, f: F) From 9896b38f01c068abfe7170cb9ae2bfadb4aebbc4 Mon Sep 17 00:00:00 2001 From: varkor Date: Sat, 17 Mar 2018 20:43:46 +0000 Subject: [PATCH 11/15] Clarify time complexity --- src/liballoc/lib.rs | 1 - src/liballoc/slice.rs | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 253b8acc16c42..b93e128d50819 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -113,7 +113,6 @@ #![feature(slice_get_slice)] #![feature(slice_patterns)] #![feature(slice_rsplit)] -#![feature(slice_sort_by_cached_key)] #![feature(specialization)] #![feature(staged_api)] #![feature(str_internals)] diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index bef50a733cc76..cd212aa15baeb 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1303,7 +1303,7 @@ impl [T] { /// Sorts the slice with a key extraction function. /// - /// This sort is stable (i.e. does not reorder equal elements) and `O(m n log m n)` + /// This sort is stable (i.e. does not reorder equal elements) and `O(m n log(m n))` /// worst-case, where the key function is `O(m)`. /// /// For expensive key functions (e.g. functions that are not simple property accesses or @@ -1365,6 +1365,7 @@ impl [T] { /// # Examples /// /// ``` + /// #![feature(slice_sort_by_cached_key)] /// let mut v = [-5i32, 4, 32, -3, 2]; /// /// v.sort_by_cached_key(|k| k.to_string()); @@ -1480,7 +1481,7 @@ impl [T] { /// elements. /// /// This sort is unstable (i.e. may reorder equal elements), in-place (i.e. does not allocate), - /// and `O(m n log m n)` worst-case, where the key function is `O(m)`. + /// and `O(m n log(m n))` worst-case, where the key function is `O(m)`. /// /// # Current implementation /// From 81edd1796b463776d111cd4fe48e866dd716dfab Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 18 Mar 2018 11:11:17 +0000 Subject: [PATCH 12/15] Check that the size optimisation is not redundant --- src/liballoc/slice.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index cd212aa15baeb..2b4ce9fe49c8e 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1384,8 +1384,8 @@ impl [T] { let mut indices: Vec<_> = $slice.iter().map($f).enumerate().map(|(i, k)| (k, i as $t)).collect(); // The elements of `indices` are unique, as they are indexed, so any sort will be - // stable with respect to the original slice. We use `sort_unstable` here because it - // requires less memory allocation. + // stable with respect to the original slice. We use `sort_unstable` here because + // it requires less memory allocation. indices.sort_unstable(); for i in 0..$slice.len() { let mut index = indices[i].1; @@ -1398,10 +1398,15 @@ impl [T] { }) } + let sz_u8 = mem::size_of::<(K, u8)>(); + let sz_u16 = mem::size_of::<(K, u16)>(); + let sz_u32 = mem::size_of::<(K, u32)>(); + let sz_usize = mem::size_of::<(K, usize)>(); + let len = self.len(); - if len <= ( u8::MAX as usize) { return sort_by_key!( u8, self, f) } - if len <= (u16::MAX as usize) { return sort_by_key!(u16, self, f) } - if len <= (u32::MAX as usize) { return sort_by_key!(u32, self, f) } + if sz_u8 < sz_u16 && len <= ( u8::MAX as usize) { return sort_by_key!( u8, self, f) } + if sz_u16 < sz_u32 && len <= (u16::MAX as usize) { return sort_by_key!(u16, self, f) } + if sz_u32 < sz_usize && len <= (u32::MAX as usize) { return sort_by_key!(u32, self, f) } sort_by_key!(usize, self, f) } From 785e3c38fe6c49e39aec145c81e463ceb60d179e Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 18 Mar 2018 12:37:06 +0000 Subject: [PATCH 13/15] Add lexicographic sorting benchmark --- src/liballoc/benches/lib.rs | 1 + src/liballoc/benches/slice.rs | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/liballoc/benches/lib.rs b/src/liballoc/benches/lib.rs index 2de0ffb4b2611..9cb5f558574c1 100644 --- a/src/liballoc/benches/lib.rs +++ b/src/liballoc/benches/lib.rs @@ -13,6 +13,7 @@ #![feature(i128_type)] #![feature(rand)] #![feature(repr_simd)] +#![feature(slice_sort_by_cached_key)] #![feature(test)] extern crate rand; diff --git a/src/liballoc/benches/slice.rs b/src/liballoc/benches/slice.rs index ee5182a1d4663..a699ff9c0a76e 100644 --- a/src/liballoc/benches/slice.rs +++ b/src/liballoc/benches/slice.rs @@ -284,6 +284,17 @@ macro_rules! sort_expensive { } } +macro_rules! sort_lexicographic { + ($f:ident, $name:ident, $gen:expr, $len:expr) => { + #[bench] + fn $name(b: &mut Bencher) { + let v = $gen($len); + b.iter(|| v.clone().$f(|x| x.to_string())); + b.bytes = $len * mem::size_of_val(&$gen(1)[0]) as u64; + } + } +} + sort!(sort, sort_small_ascending, gen_ascending, 10); sort!(sort, sort_small_descending, gen_descending, 10); sort!(sort, sort_small_random, gen_random, 10); @@ -312,6 +323,10 @@ sort!(sort_unstable, sort_unstable_large_big, gen_big_random, 10000); sort_strings!(sort_unstable, sort_unstable_large_strings, gen_strings, 10000); sort_expensive!(sort_unstable_by, sort_unstable_large_expensive, gen_random, 10000); +sort_lexicographic!(sort_by_key, sort_by_key_lexicographic, gen_random, 10000); +sort_lexicographic!(sort_unstable_by_key, sort_unstable_by_key_lexicographic, gen_random, 10000); +sort_lexicographic!(sort_by_cached_key, sort_by_cached_key_lexicographic, gen_random, 10000); + macro_rules! reverse { ($name:ident, $ty:ty, $f:expr) => { #[bench] From eca1e18cd7021f01757640c0c5ef63717870686c Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 19 Mar 2018 00:11:47 +0000 Subject: [PATCH 14/15] Add stability test for sort_by_cached_key --- src/liballoc/tests/slice.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/liballoc/tests/slice.rs b/src/liballoc/tests/slice.rs index 7d4dac1c5ec99..66c7dd75c8740 100644 --- a/src/liballoc/tests/slice.rs +++ b/src/liballoc/tests/slice.rs @@ -485,7 +485,7 @@ fn test_sort_stability() { // the second item represents which occurrence of that // number this element is, i.e. the second elements // will occur in sorted order. - let mut v: Vec<_> = (0..len) + let mut orig: Vec<_> = (0..len) .map(|_| { let n = thread_rng().gen::() % 10; counts[n] += 1; @@ -493,16 +493,21 @@ fn test_sort_stability() { }) .collect(); - // only sort on the first element, so an unstable sort + let mut v = orig.clone(); + // Only sort on the first element, so an unstable sort // may mix up the counts. v.sort_by(|&(a, _), &(b, _)| a.cmp(&b)); - // this comparison includes the count (the second item + // This comparison includes the count (the second item // of the tuple), so elements with equal first items // will need to be ordered with increasing // counts... i.e. exactly asserting that this sort is // stable. assert!(v.windows(2).all(|w| w[0] <= w[1])); + + let mut v = orig.clone(); + v.sort_by_cached_key(|&(x, _)| x); + assert!(v.windows(2).all(|w| w[0] <= w[1])); } } } From 9c7b69e17909ceb090a1c4b8882a4e0924a2a755 Mon Sep 17 00:00:00 2001 From: varkor Date: Mon, 26 Mar 2018 22:24:03 +0100 Subject: [PATCH 15/15] Remove mentions of unstable sort_by_cached key from stable documentation --- src/liballoc/slice.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index 2b4ce9fe49c8e..68f2313843c31 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -1306,10 +1306,6 @@ impl [T] { /// This sort is stable (i.e. does not reorder equal elements) and `O(m n log(m n))` /// worst-case, where the key function is `O(m)`. /// - /// For expensive key functions (e.g. functions that are not simple property accesses or - /// basic operations), [`sort_by_cached_key`](#method.sort_by_cached_key) is likely to be - /// significantly faster, as it does not recompute element keys. - /// /// When applicable, unstable sorting is preferred because it is generally faster than stable /// sorting and it doesn't allocate auxiliary memory. /// See [`sort_unstable_by_key`](#method.sort_unstable_by_key). @@ -1496,10 +1492,6 @@ impl [T] { /// randomization to avoid degenerate cases, but with a fixed seed to always provide /// deterministic behavior. /// - /// Due to its key calling strategy, [`sort_unstable_by_key`](#method.sort_unstable_by_key) - /// is likely to be slower than [`sort_by_cached_key`](#method.sort_by_cached_key) in - /// cases where the key function is expensive. - /// /// # Examples /// /// ```