Skip to content

Commit

Permalink
Auto merge of rust-lang#124032 - Voultapher:a-new-sort, r=thomcc
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), includes detailed benchmarks and analysis.

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

#### 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 be changed, this advice would be misleading for some Rust versions. By replacing them atomically, the advice 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 Jun 20, 2024
2 parents dbaf524 + 0f6922d commit 9a945fd
Show file tree
Hide file tree
Showing 18 changed files with 2,627 additions and 1,686 deletions.
214 changes: 103 additions & 111 deletions alloc/src/slice.rs

Large diffs are not rendered by default.

19 changes: 12 additions & 7 deletions 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 @@ -294,15 +294,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 core/src/slice/mod.rs

Large diffs are not rendered by default.

Loading

0 comments on commit 9a945fd

Please sign in to comment.