Auto merge of #42882 - stjepang:improve-sort-tests-and-benches, r=alexcrichton

Improve tests and benchmarks for slice::sort and slice::sort_unstable

This PR just hardens the tests and improves benchmarks.
More specifically:

1. Benchmarks don't generate vectors in `Bencher::iter` loops, but simply clone pregenerated vectors.
2. Benchmark `*_strings` doesn't allocate Strings in `Bencher::iter` loops, but merely clones a `Vec<&str>`.
3. Benchmarks use seeded `XorShiftRng` to be more consistent.
4. Additional tests for `slice::sort` are added, which test sorting on slices with several ascending/descending runs. The implementation identifies such runs so it's a good idea to test that scenario a bit.
5. More checks are added to `run-pass/vector-sort-panic-safe.rs`. Sort algorithms copy elements around a lot (merge sort uses an auxilliary buffer and pdqsort copies the pivot onto the stack before partitioning, then writes it back into the slice). If elements that are being sorted are internally mutable and comparison function mutates them, it is important to make sure that sort algorithms always use the latest "versions" of elements. New checks verify that this is true for both `slice::sort` and `slice::sort_unstable`.

As a side note, all of those improvements were made as part of the parallel sorts PR in Rayon (nikomatsakis/rayon#379) and now I'm backporting them into libcore/libstd.

r? @alexcrichton

4 files changed, 76 insertions, 33 deletions

diff --git a/src/liballoc/benches/lib.rs b/src/liballoc/benches/lib.rs
index 958020d0b0e..5f274eec87d 100644
--- a/src/liballoc/benches/lib.rs
+++ b/src/liballoc/benches/lib.rs
@@ -17,6 +17,7 @@
 #![feature(sort_unstable)]
 #![feature(test)]
 
+extern crate rand;
 extern crate test;
 
 mod btree;
diff --git a/src/liballoc/benches/slice.rs b/src/liballoc/benches/slice.rs
index aa5a438b35e..d99270e7f31 100644
--- a/src/liballoc/benches/slice.rs
+++ b/src/liballoc/benches/slice.rs
@@ -8,9 +8,11 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{mem, ptr};
-use std::__rand::{Rng, thread_rng};
+use std::__rand::{thread_rng};
+use std::mem;
+use std::ptr;
 
+use rand::{Rng, SeedableRng, XorShiftRng};
 use test::{Bencher, black_box};
 
 #[bench]
@@ -191,17 +193,17 @@ fn gen_descending(len: usize) -> Vec<u64> {
 }
 
 fn gen_random(len: usize) -> Vec<u64> {
-    let mut rng = thread_rng();
+    let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
     rng.gen_iter::<u64>().take(len).collect()
 }
 
 fn gen_random_bytes(len: usize) -> Vec<u8> {
-    let mut rng = thread_rng();
+    let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
     rng.gen_iter::<u8>().take(len).collect()
 }
 
 fn gen_mostly_ascending(len: usize) -> Vec<u64> {
-    let mut rng = thread_rng();
+    let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
     let mut v = gen_ascending(len);
     for _ in (0usize..).take_while(|x| x * x <= len) {
         let x = rng.gen::<usize>() % len;
@@ -212,7 +214,7 @@ fn gen_mostly_ascending(len: usize) -> Vec<u64> {
 }
 
 fn gen_mostly_descending(len: usize) -> Vec<u64> {
-    let mut rng = thread_rng();
+    let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
     let mut v = gen_descending(len);
     for _ in (0usize..).take_while(|x| x * x <= len) {
         let x = rng.gen::<usize>() % len;
@@ -223,7 +225,7 @@ fn gen_mostly_descending(len: usize) -> Vec<u64> {
 }
 
 fn gen_strings(len: usize) -> Vec<String> {
-    let mut rng = thread_rng();
+    let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
     let mut v = vec![];
     for _ in 0..len {
         let n = rng.gen::<usize>() % 20 + 1;
@@ -233,7 +235,7 @@ fn gen_strings(len: usize) -> Vec<String> {
 }
 
 fn gen_big_random(len: usize) -> Vec<[u64; 16]> {
-    let mut rng = thread_rng();
+    let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
     rng.gen_iter().map(|x| [x; 16]).take(len).collect()
 }
 
@@ -241,18 +243,32 @@ macro_rules! sort {
     ($f:ident, $name:ident, $gen:expr, $len:expr) => {
         #[bench]
         fn $name(b: &mut Bencher) {
-            b.iter(|| $gen($len).$f());
+            let v = $gen($len);
+            b.iter(|| v.clone().$f());
             b.bytes = $len * mem::size_of_val(&$gen(1)[0]) as u64;
         }
     }
 }
 
+macro_rules! sort_strings {
+    ($f:ident, $name:ident, $gen:expr, $len:expr) => {
+        #[bench]
+        fn $name(b: &mut Bencher) {
+            let v = $gen($len);
+            let v = v.iter().map(|s| &**s).collect::<Vec<&str>>();
+            b.iter(|| v.clone().$f());
+            b.bytes = $len * mem::size_of::<&str>() as u64;
+        }
+    }
+}
+
 macro_rules! sort_expensive {
     ($f:ident, $name:ident, $gen:expr, $len:expr) => {
         #[bench]
         fn $name(b: &mut Bencher) {
+            let v = $gen($len);
             b.iter(|| {
-                let mut v = $gen($len);
+                let mut v = v.clone();
                 let mut count = 0;
                 v.$f(|a: &u64, b: &u64| {
                     count += 1;
@@ -263,7 +279,7 @@ macro_rules! sort_expensive {
                 });
                 black_box(count);
             });
-            b.bytes = $len as u64 * mem::size_of::<u64>() as u64;
+            b.bytes = $len * mem::size_of_val(&$gen(1)[0]) as u64;
         }
     }
 }
@@ -271,30 +287,30 @@ macro_rules! sort_expensive {
 sort!(sort, sort_small_ascending, gen_ascending, 10);
 sort!(sort, sort_small_descending, gen_descending, 10);
 sort!(sort, sort_small_random, gen_random, 10);
-sort!(sort, sort_small_big_random, gen_big_random, 10);
+sort!(sort, sort_small_big, gen_big_random, 10);
 sort!(sort, sort_medium_random, gen_random, 100);
 sort!(sort, sort_large_ascending, gen_ascending, 10000);
 sort!(sort, sort_large_descending, gen_descending, 10000);
 sort!(sort, sort_large_mostly_ascending, gen_mostly_ascending, 10000);
 sort!(sort, sort_large_mostly_descending, gen_mostly_descending, 10000);
 sort!(sort, sort_large_random, gen_random, 10000);
-sort!(sort, sort_large_big_random, gen_big_random, 10000);
-sort!(sort, sort_large_strings, gen_strings, 10000);
-sort_expensive!(sort_by, sort_large_random_expensive, gen_random, 10000);
+sort!(sort, sort_large_big, gen_big_random, 10000);
+sort_strings!(sort, sort_large_strings, gen_strings, 10000);
+sort_expensive!(sort_by, sort_large_expensive, gen_random, 10000);
 
 sort!(sort_unstable, sort_unstable_small_ascending, gen_ascending, 10);
 sort!(sort_unstable, sort_unstable_small_descending, gen_descending, 10);
 sort!(sort_unstable, sort_unstable_small_random, gen_random, 10);
-sort!(sort_unstable, sort_unstable_small_big_random, gen_big_random, 10);
+sort!(sort_unstable, sort_unstable_small_big, gen_big_random, 10);
 sort!(sort_unstable, sort_unstable_medium_random, gen_random, 100);
 sort!(sort_unstable, sort_unstable_large_ascending, gen_ascending, 10000);
 sort!(sort_unstable, sort_unstable_large_descending, gen_descending, 10000);
 sort!(sort_unstable, sort_unstable_large_mostly_ascending, gen_mostly_ascending, 10000);
 sort!(sort_unstable, sort_unstable_large_mostly_descending, gen_mostly_descending, 10000);
 sort!(sort_unstable, sort_unstable_large_random, gen_random, 10000);
-sort!(sort_unstable, sort_unstable_large_big_random, gen_big_random, 10000);
-sort!(sort_unstable, sort_unstable_large_strings, gen_strings, 10000);
-sort_expensive!(sort_unstable_by, sort_unstable_large_random_expensive, gen_random, 10000);
+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);
 
 macro_rules! reverse {
     ($name:ident, $ty:ty, $f:expr) => {
diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs
index 88876999d76..fe8893904eb 100644
--- a/src/liballoc/slice.rs
+++ b/src/liballoc/slice.rs
@@ -1794,7 +1794,7 @@ unsafe fn merge<T, F>(v: &mut [T], mid: usize, buf: *mut T, is_less: &mut F)
 
     impl<T> Drop for MergeHole<T> {
         fn drop(&mut self) {
-            // `T` is not a zero-sized type, so it's okay to divide by it's size.
+            // `T` is not a zero-sized type, so it's okay to divide by its size.
             let len = (self.end as usize - self.start as usize) / mem::size_of::<T>();
             unsafe { ptr::copy_nonoverlapping(self.start, self.dest, len); }
         }
@@ -1908,7 +1908,7 @@ fn merge_sort<T, F>(v: &mut [T], mut is_less: F)
     // if `Some(r)` is returned, that means `runs[r]` and `runs[r + 1]` must be merged next. If the
     // algorithm should continue building a new run instead, `None` is returned.
     //
-    // TimSort is infamous for it's buggy implementations, as described here:
+    // TimSort is infamous for its buggy implementations, as described here:
     // http://envisage-project.eu/timsort-specification-and-verification/
     //
     // The gist of the story is: we must enforce the invariants on the top four runs on the stack.
diff --git a/src/liballoc/tests/slice.rs b/src/liballoc/tests/slice.rs
index 7fa65a2144e..c53bf15f1bf 100644
--- a/src/liballoc/tests/slice.rs
+++ b/src/liballoc/tests/slice.rs
@@ -396,18 +396,44 @@ fn test_sort() {
     let mut rng = thread_rng();
 
     for len in (2..25).chain(500..510) {
-        for _ in 0..100 {
-            let mut v: Vec<_> = rng.gen_iter::<i32>().take(len).collect();
-            let mut v1 = v.clone();
-
-            v.sort();
-            assert!(v.windows(2).all(|w| w[0] <= w[1]));
-
-            v1.sort_by(|a, b| a.cmp(b));
-            assert!(v1.windows(2).all(|w| w[0] <= w[1]));
-
-            v1.sort_by(|a, b| b.cmp(a));
-            assert!(v1.windows(2).all(|w| w[0] >= w[1]));
+        for &modulus in &[5, 10, 100, 1000] {
+            for _ in 0..10 {
+                let orig: Vec<_> = rng.gen_iter::<i32>()
+                    .map(|x| x % modulus)
+                    .take(len)
+                    .collect();
+
+                // Sort in default order.
+                let mut v = orig.clone();
+                v.sort();
+                assert!(v.windows(2).all(|w| w[0] <= w[1]));
+
+                // Sort in ascending order.
+                let mut v = orig.clone();
+                v.sort_by(|a, b| a.cmp(b));
+                assert!(v.windows(2).all(|w| w[0] <= w[1]));
+
+                // Sort in descending order.
+                let mut v = orig.clone();
+                v.sort_by(|a, b| b.cmp(a));
+                assert!(v.windows(2).all(|w| w[0] >= w[1]));
+
+                // Sort with many pre-sorted runs.
+                let mut v = orig.clone();
+                v.sort();
+                v.reverse();
+                for _ in 0..5 {
+                    let a = rng.gen::<usize>() % len;
+                    let b = rng.gen::<usize>() % len;
+                    if a < b {
+                        v[a..b].reverse();
+                    } else {
+                        v.swap(a, b);
+                    }
+                }
+                v.sort();
+                assert!(v.windows(2).all(|w| w[0] <= w[1]));
+            }
         }
     }