Skip to content

Commit

Permalink
Auto merge of #124032 - Voultapher:a-new-sort, r=<try>
Browse files Browse the repository at this point in the history
Replace sort implementations

This PR replaces the sort implementations with tailor-made ones that strike a balance of run-time, compile-time and binary-size, yielding run-time and compile-time improvements. Regressing binary-size for `slice::sort` while improving it for `slice::sort_unstable`. All while upholding the existing soft and hard safety guarantees, and even extending the soft guarantees, detecting strict weak ordering violations with a high chance and reporting it to users via a panic.

* `slice::sort` -> driftsort [design document](https://github.com/Voultapher/sort-research-rs/blob/main/writeup/driftsort_introduction/text.md)

* `slice::sort_unstable` -> ipnsort [design document](https://github.com/Voultapher/sort-research-rs/blob/main/writeup/ipnsort_introduction/text.md)

#### Why should we change the sort implementations?

In the [2023 Rust survey](https://blog.rust-lang.org/2024/02/19/2023-Rust-Annual-Survey-2023-results.html#challenges), one of the questions was: "In your opinion, how should work on the following aspects of Rust be prioritized?". The second place was "Runtime performance" and the third one "Compile Times". This PR aims to improve both.

#### Why is this one big PR and not multiple?

* The current documentation gives performance recommendations for `slice::sort` and `slice::sort_unstable`. If for example only one of them were to changed, this advise may be misleading for some Rust versions. By replacing them atomically, the advise remains largely unchanged, and users don't have to change their code.
* driftsort and ipnsort share a substantial part of their implementations.
* The implementation of `select_nth_unstable` uses internals of `slice::sort_unstable`, which makes it impractical to split changes.

---

This PR is a collaboration with `@orlp.`
  • Loading branch information
bors committed Apr 17, 2024
2 parents 6c6b302 + eeee5de commit a4132fa
Show file tree
Hide file tree
Showing 19 changed files with 2,612 additions and 1,693 deletions.
214 changes: 103 additions & 111 deletions library/alloc/src/slice.rs

Large diffs are not rendered by default.

22 changes: 13 additions & 9 deletions library/alloc/src/slice/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ macro_rules! do_test {
}

let v = $input.to_owned();
let _ = std::panic::catch_unwind(move || {
let _ = panic::catch_unwind(move || {
let mut v = v;
let mut panic_countdown = panic_countdown;
v.$func(|a, b| {
Expand Down Expand Up @@ -197,8 +197,7 @@ fn panic_safe() {

let mut rng = test_rng();

// Miri is too slow (but still need to `chain` to make the types match)
let lens = if cfg!(miri) { (1..10).chain(0..0) } else { (1..20).chain(70..MAX_LEN) };
let lens = if cfg!(miri) { (1..10).chain(30..36) } else { (1..20).chain(70..MAX_LEN) };
let moduli: &[u32] = if cfg!(miri) { &[5] } else { &[5, 20, 50] };

for len in lens {
Expand Down Expand Up @@ -294,15 +293,20 @@ fn test_sort() {
}
}

// Sort using a completely random comparison function.
// This will reorder the elements *somehow*, but won't panic.
let mut v = [0; 500];
for i in 0..v.len() {
const ORD_VIOLATION_MAX_LEN: usize = 500;
let mut v = [0; ORD_VIOLATION_MAX_LEN];
for i in 0..ORD_VIOLATION_MAX_LEN {
v[i] = i as i32;
}
v.sort_by(|_, _| *[Less, Equal, Greater].choose(&mut rng).unwrap());

// Sort using a completely random comparison function. This will reorder the elements *somehow*,
// it may panic but the original elements must still be present.
let _ = panic::catch_unwind(move || {
v.sort_by(|_, _| *[Less, Equal, Greater].choose(&mut rng).unwrap());
});

v.sort();
for i in 0..v.len() {
for i in 0..ORD_VIOLATION_MAX_LEN {
assert_eq!(v[i], i as i32);
}

Expand Down
193 changes: 109 additions & 84 deletions library/core/src/slice/mod.rs

Large diffs are not rendered by default.

Loading

0 comments on commit a4132fa

Please sign in to comment.