Skip to content

Commit 05b5797

Browse files
committed
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
2 parents a5d34e1 + 723833f commit 05b5797

File tree

5 files changed

+181
-96
lines changed

5 files changed

+181
-96
lines changed

src/liballoc/benches/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#![feature(sort_unstable)]
1818
#![feature(test)]
1919

20+
extern crate rand;
2021
extern crate test;
2122

2223
mod btree;

src/liballoc/benches/slice.rs

+35-19
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
use std::{mem, ptr};
12-
use std::__rand::{Rng, thread_rng};
11+
use std::__rand::{thread_rng};
12+
use std::mem;
13+
use std::ptr;
1314

15+
use rand::{Rng, SeedableRng, XorShiftRng};
1416
use test::{Bencher, black_box};
1517

1618
#[bench]
@@ -191,17 +193,17 @@ fn gen_descending(len: usize) -> Vec<u64> {
191193
}
192194

193195
fn gen_random(len: usize) -> Vec<u64> {
194-
let mut rng = thread_rng();
196+
let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
195197
rng.gen_iter::<u64>().take(len).collect()
196198
}
197199

198200
fn gen_random_bytes(len: usize) -> Vec<u8> {
199-
let mut rng = thread_rng();
201+
let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
200202
rng.gen_iter::<u8>().take(len).collect()
201203
}
202204

203205
fn gen_mostly_ascending(len: usize) -> Vec<u64> {
204-
let mut rng = thread_rng();
206+
let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
205207
let mut v = gen_ascending(len);
206208
for _ in (0usize..).take_while(|x| x * x <= len) {
207209
let x = rng.gen::<usize>() % len;
@@ -212,7 +214,7 @@ fn gen_mostly_ascending(len: usize) -> Vec<u64> {
212214
}
213215

214216
fn gen_mostly_descending(len: usize) -> Vec<u64> {
215-
let mut rng = thread_rng();
217+
let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
216218
let mut v = gen_descending(len);
217219
for _ in (0usize..).take_while(|x| x * x <= len) {
218220
let x = rng.gen::<usize>() % len;
@@ -223,7 +225,7 @@ fn gen_mostly_descending(len: usize) -> Vec<u64> {
223225
}
224226

225227
fn gen_strings(len: usize) -> Vec<String> {
226-
let mut rng = thread_rng();
228+
let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
227229
let mut v = vec![];
228230
for _ in 0..len {
229231
let n = rng.gen::<usize>() % 20 + 1;
@@ -233,26 +235,40 @@ fn gen_strings(len: usize) -> Vec<String> {
233235
}
234236

235237
fn gen_big_random(len: usize) -> Vec<[u64; 16]> {
236-
let mut rng = thread_rng();
238+
let mut rng = XorShiftRng::from_seed([0, 1, 2, 3]);
237239
rng.gen_iter().map(|x| [x; 16]).take(len).collect()
238240
}
239241

240242
macro_rules! sort {
241243
($f:ident, $name:ident, $gen:expr, $len:expr) => {
242244
#[bench]
243245
fn $name(b: &mut Bencher) {
244-
b.iter(|| $gen($len).$f());
246+
let v = $gen($len);
247+
b.iter(|| v.clone().$f());
245248
b.bytes = $len * mem::size_of_val(&$gen(1)[0]) as u64;
246249
}
247250
}
248251
}
249252

253+
macro_rules! sort_strings {
254+
($f:ident, $name:ident, $gen:expr, $len:expr) => {
255+
#[bench]
256+
fn $name(b: &mut Bencher) {
257+
let v = $gen($len);
258+
let v = v.iter().map(|s| &**s).collect::<Vec<&str>>();
259+
b.iter(|| v.clone().$f());
260+
b.bytes = $len * mem::size_of::<&str>() as u64;
261+
}
262+
}
263+
}
264+
250265
macro_rules! sort_expensive {
251266
($f:ident, $name:ident, $gen:expr, $len:expr) => {
252267
#[bench]
253268
fn $name(b: &mut Bencher) {
269+
let v = $gen($len);
254270
b.iter(|| {
255-
let mut v = $gen($len);
271+
let mut v = v.clone();
256272
let mut count = 0;
257273
v.$f(|a: &u64, b: &u64| {
258274
count += 1;
@@ -263,38 +279,38 @@ macro_rules! sort_expensive {
263279
});
264280
black_box(count);
265281
});
266-
b.bytes = $len as u64 * mem::size_of::<u64>() as u64;
282+
b.bytes = $len * mem::size_of_val(&$gen(1)[0]) as u64;
267283
}
268284
}
269285
}
270286

271287
sort!(sort, sort_small_ascending, gen_ascending, 10);
272288
sort!(sort, sort_small_descending, gen_descending, 10);
273289
sort!(sort, sort_small_random, gen_random, 10);
274-
sort!(sort, sort_small_big_random, gen_big_random, 10);
290+
sort!(sort, sort_small_big, gen_big_random, 10);
275291
sort!(sort, sort_medium_random, gen_random, 100);
276292
sort!(sort, sort_large_ascending, gen_ascending, 10000);
277293
sort!(sort, sort_large_descending, gen_descending, 10000);
278294
sort!(sort, sort_large_mostly_ascending, gen_mostly_ascending, 10000);
279295
sort!(sort, sort_large_mostly_descending, gen_mostly_descending, 10000);
280296
sort!(sort, sort_large_random, gen_random, 10000);
281-
sort!(sort, sort_large_big_random, gen_big_random, 10000);
282-
sort!(sort, sort_large_strings, gen_strings, 10000);
283-
sort_expensive!(sort_by, sort_large_random_expensive, gen_random, 10000);
297+
sort!(sort, sort_large_big, gen_big_random, 10000);
298+
sort_strings!(sort, sort_large_strings, gen_strings, 10000);
299+
sort_expensive!(sort_by, sort_large_expensive, gen_random, 10000);
284300

285301
sort!(sort_unstable, sort_unstable_small_ascending, gen_ascending, 10);
286302
sort!(sort_unstable, sort_unstable_small_descending, gen_descending, 10);
287303
sort!(sort_unstable, sort_unstable_small_random, gen_random, 10);
288-
sort!(sort_unstable, sort_unstable_small_big_random, gen_big_random, 10);
304+
sort!(sort_unstable, sort_unstable_small_big, gen_big_random, 10);
289305
sort!(sort_unstable, sort_unstable_medium_random, gen_random, 100);
290306
sort!(sort_unstable, sort_unstable_large_ascending, gen_ascending, 10000);
291307
sort!(sort_unstable, sort_unstable_large_descending, gen_descending, 10000);
292308
sort!(sort_unstable, sort_unstable_large_mostly_ascending, gen_mostly_ascending, 10000);
293309
sort!(sort_unstable, sort_unstable_large_mostly_descending, gen_mostly_descending, 10000);
294310
sort!(sort_unstable, sort_unstable_large_random, gen_random, 10000);
295-
sort!(sort_unstable, sort_unstable_large_big_random, gen_big_random, 10000);
296-
sort!(sort_unstable, sort_unstable_large_strings, gen_strings, 10000);
297-
sort_expensive!(sort_unstable_by, sort_unstable_large_random_expensive, gen_random, 10000);
311+
sort!(sort_unstable, sort_unstable_large_big, gen_big_random, 10000);
312+
sort_strings!(sort_unstable, sort_unstable_large_strings, gen_strings, 10000);
313+
sort_expensive!(sort_unstable_by, sort_unstable_large_expensive, gen_random, 10000);
298314

299315
macro_rules! reverse {
300316
($name:ident, $ty:ty, $f:expr) => {

src/liballoc/slice.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1794,7 +1794,7 @@ unsafe fn merge<T, F>(v: &mut [T], mid: usize, buf: *mut T, is_less: &mut F)
17941794

17951795
impl<T> Drop for MergeHole<T> {
17961796
fn drop(&mut self) {
1797-
// `T` is not a zero-sized type, so it's okay to divide by it's size.
1797+
// `T` is not a zero-sized type, so it's okay to divide by its size.
17981798
let len = (self.end as usize - self.start as usize) / mem::size_of::<T>();
17991799
unsafe { ptr::copy_nonoverlapping(self.start, self.dest, len); }
18001800
}
@@ -1908,7 +1908,7 @@ fn merge_sort<T, F>(v: &mut [T], mut is_less: F)
19081908
// if `Some(r)` is returned, that means `runs[r]` and `runs[r + 1]` must be merged next. If the
19091909
// algorithm should continue building a new run instead, `None` is returned.
19101910
//
1911-
// TimSort is infamous for it's buggy implementations, as described here:
1911+
// TimSort is infamous for its buggy implementations, as described here:
19121912
// http://envisage-project.eu/timsort-specification-and-verification/
19131913
//
19141914
// The gist of the story is: we must enforce the invariants on the top four runs on the stack.

src/liballoc/tests/slice.rs

+38-12
Original file line numberDiff line numberDiff line change
@@ -396,18 +396,44 @@ fn test_sort() {
396396
let mut rng = thread_rng();
397397

398398
for len in (2..25).chain(500..510) {
399-
for _ in 0..100 {
400-
let mut v: Vec<_> = rng.gen_iter::<i32>().take(len).collect();
401-
let mut v1 = v.clone();
402-
403-
v.sort();
404-
assert!(v.windows(2).all(|w| w[0] <= w[1]));
405-
406-
v1.sort_by(|a, b| a.cmp(b));
407-
assert!(v1.windows(2).all(|w| w[0] <= w[1]));
408-
409-
v1.sort_by(|a, b| b.cmp(a));
410-
assert!(v1.windows(2).all(|w| w[0] >= w[1]));
399+
for &modulus in &[5, 10, 100, 1000] {
400+
for _ in 0..10 {
401+
let orig: Vec<_> = rng.gen_iter::<i32>()
402+
.map(|x| x % modulus)
403+
.take(len)
404+
.collect();
405+
406+
// Sort in default order.
407+
let mut v = orig.clone();
408+
v.sort();
409+
assert!(v.windows(2).all(|w| w[0] <= w[1]));
410+
411+
// Sort in ascending order.
412+
let mut v = orig.clone();
413+
v.sort_by(|a, b| a.cmp(b));
414+
assert!(v.windows(2).all(|w| w[0] <= w[1]));
415+
416+
// Sort in descending order.
417+
let mut v = orig.clone();
418+
v.sort_by(|a, b| b.cmp(a));
419+
assert!(v.windows(2).all(|w| w[0] >= w[1]));
420+
421+
// Sort with many pre-sorted runs.
422+
let mut v = orig.clone();
423+
v.sort();
424+
v.reverse();
425+
for _ in 0..5 {
426+
let a = rng.gen::<usize>() % len;
427+
let b = rng.gen::<usize>() % len;
428+
if a < b {
429+
v[a..b].reverse();
430+
} else {
431+
v.swap(a, b);
432+
}
433+
}
434+
v.sort();
435+
assert!(v.windows(2).all(|w| w[0] <= w[1]));
436+
}
411437
}
412438
}
413439

0 commit comments

Comments
 (0)