Unify stable and unstable sort implementations in same core module #104672

Voultapher · 2022-11-21T12:48:00Z

This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access Vec. The two Vec used by merge sort, buf and runs, are modelled as custom types that implement the very limited required Vec interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as insert_head.

This is in preparation of #100856 and #104116. It only moves code, it doesn't change any of the sort related logic. This unlocks the ability to share insert_head, insert_tail, swap_if_less merge and more.

Tagging @Mark-Simulacrum I hope this allows progress on #100856, by moving merge_sort here I hope future changes will be easier to review.

This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access `Vec`. The two `Vec` used by merge sort, `buf` and `runs`, are modelled as custom types that implement the very limited required `Vec` interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as `insert_head`.

rustbot · 2022-11-21T12:48:07Z

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2022-11-21T12:48:10Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

There were several unsafe blocks in the existing implementation that were not documented with a SAFETY comment.

Voultapher · 2022-11-21T19:14:21Z

As a nice little side-effect, I'm seeing a small binary size decrease (eg. 363 -> 361kb) with this change for sort instantiations.

Voultapher · 2022-11-29T19:25:17Z

@joshtriplett friendly ping, is there something blocking review of this PR?

thomcc · 2022-12-09T06:24:38Z

r? @thomcc

thomcc · 2022-12-14T02:01:42Z

It only moves code, it doesn't change any of the sort related logic

This seems to be true, so I don't have any comments really.

I will say that this will be a problem if we don't perform the other sort-related changes, and it may constrain future sort implementations to share components in this way, but that's probably fine.

@bors r+

bors · 2022-12-14T02:01:44Z

📌 Commit 4b5844f has been approved by thomcc

It is now in the queue for this repository.

thomcc · 2022-12-14T02:03:22Z

Ah, actually, I need some more time to think about a couple parts of this, sorry for the false start.

@bors r-

Voultapher · 2022-12-20T20:44:49Z

Can I somehow help? This PR seems stuck again :(

thomcc · 2023-01-19T22:46:08Z

Yeah, this seems fine.

@bors r+

bors · 2023-01-19T22:46:09Z

📌 Commit 4b5844f has been approved by thomcc

It is now in the queue for this repository.

…homcc Unify stable and unstable sort implementations in same core module This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access `Vec`. The two `Vec` used by merge sort, `buf` and `runs`, are modelled as custom types that implement the very limited required `Vec` interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as `insert_head`. This is in preparation of rust-lang#100856 and rust-lang#104116. It only moves code, it *doesn't* change any of the sort related logic. This unlocks the ability to share `insert_head`, `insert_tail`, `swap_if_less` `merge` and more. Tagging ``@Mark-Simulacrum`` I hope this allows progress on rust-lang#100856, by moving `merge_sort` here I hope future changes will be easier to review.

…homcc Unify stable and unstable sort implementations in same core module This moves the stable sort implementation to the core::slice::sort module. By virtue of being in core it can't access `Vec`. The two `Vec` used by merge sort, `buf` and `runs`, are modelled as custom types that implement the very limited required `Vec` interface with the help of provided allocation and free functions. This is done to allow future re-use of functions and logic between stable and unstable sort. Such as `insert_head`. This is in preparation of rust-lang#100856 and rust-lang#104116. It only moves code, it *doesn't* change any of the sort related logic. This unlocks the ability to share `insert_head`, `insert_tail`, `swap_if_less` `merge` and more. Tagging ```@Mark-Simulacrum``` I hope this allows progress on rust-lang#100856, by moving `merge_sort` here I hope future changes will be easier to review.

…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#104154 (Change `bindings_with_variant_name` to deny-by-default) - rust-lang#104347 (diagnostics: suggest changing `s@self::{macro}`@::macro`` for exported) - rust-lang#104672 (Unify stable and unstable sort implementations in same core module) - rust-lang#107048 (check for x version updates) - rust-lang#107061 (Implement some more new solver candidates and fix some bugs) - rust-lang#107095 (rustdoc: remove redundant CSS selector `.sidebar .current`) - rust-lang#107112 (Fix typo in opaque_types.rs) - rust-lang#107124 (fix check macro expansion) - rust-lang#107131 (rustdoc: use CSS inline layout for radio line instead of flexbox) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned joshtriplett Nov 21, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 21, 2022

Voultapher added 2 commits November 21, 2022 14:20

Remove debug unused

1ec59cd

Document all unsafe blocks

4b5844f

There were several unsafe blocks in the existing implementation that were not documented with a SAFETY comment.

rustbot assigned thomcc and unassigned joshtriplett Dec 9, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 14, 2022

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 14, 2022

thomcc added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 18, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2023

compiler-errors mentioned this pull request Jan 20, 2023

Rollup of 8 pull requests #107132

Closed

matthiaskrgr mentioned this pull request Jan 20, 2023

Rollup of 10 pull requests #107134

Closed

compiler-errors mentioned this pull request Jan 21, 2023

Rollup of 9 pull requests #107143

Merged

bors merged commit 68b390a into rust-lang:master Jan 21, 2023

rustbot added this to the 1.68.0 milestone Jan 21, 2023

Voultapher deleted the unify-sort-modules branch January 21, 2023 09:06

cuviper mentioned this pull request Apr 5, 2024

Sync par_sort* with the standard library rayon-rs/rayon#1156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify stable and unstable sort implementations in same core module #104672

Unify stable and unstable sort implementations in same core module #104672

Voultapher commented Nov 21, 2022

rustbot commented Nov 21, 2022

rustbot commented Nov 21, 2022

Voultapher commented Nov 21, 2022 •

edited

Loading

Voultapher commented Nov 29, 2022

thomcc commented Dec 9, 2022

thomcc commented Dec 14, 2022

bors commented Dec 14, 2022

thomcc commented Dec 14, 2022

Voultapher commented Dec 20, 2022

thomcc commented Jan 19, 2023

bors commented Jan 19, 2023

Unify stable and unstable sort implementations in same core module #104672

Unify stable and unstable sort implementations in same core module #104672

Conversation

Voultapher commented Nov 21, 2022

rustbot commented Nov 21, 2022

rustbot commented Nov 21, 2022

Voultapher commented Nov 21, 2022 • edited Loading

Voultapher commented Nov 29, 2022

thomcc commented Dec 9, 2022

thomcc commented Dec 14, 2022

bors commented Dec 14, 2022

thomcc commented Dec 14, 2022

Voultapher commented Dec 20, 2022

thomcc commented Jan 19, 2023

bors commented Jan 19, 2023

Voultapher commented Nov 21, 2022 •

edited

Loading