-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-34561: Switch to Munro & Wild "powersort" merge strategy. #28108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
e311b34
bpo-34561: Switch to Munro & Wild "powersort" merge strategy.
tim-one 1c6dfcf
Various changes. Most significantly, got rid of the new struct
tim-one b253852
Close to done! Some code fiddling, changed comments to match the
tim-one cbd2e6d
Typo repair.
tim-one c6b1806
📜🤖 Added by blurb_it.
blurb-it[bot] 560a3f1
Update Misc/NEWS.d/next/Core and Builtins/2021-09-01-19-21-48.bpo-345…
tim-one 70fd24e
Comment changes to address good points raised in review.
tim-one 53e85de
Rewrote merge_collapse(), and renamed it to found_new_run(), to
tim-one 56f8ccf
The starting-index argument to found_new_run wasn't actually used,
tim-one File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
1 change: 1 addition & 0 deletions
1
Misc/NEWS.d/next/Core and Builtins/2021-09-01-19-21-48.bpo-34561.uMAVA-.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
List sorting now uses the merge-ordering strategy from Munro and Wild's ``powersort()``. Unlike the former strategy, this is provably near-optimal in the entropy of the distribution of run lengths. Most uses of ``list.sort()`` probably won't see a significant time difference, but may see significant improvements in cases where the former strategy was exceptionally poor. However, as these are all fast linear-time approximations to a problem that's inherently at best quadratic-time to solve truly optimally, it's also possible to contrive cases where the former strategy did better. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bit of a nitpick, but I found this confusing when reading the code and trying to follow along with the comments: the math a and b from the comments having the same names as the code a and b, despite those storing 2*math a (and b respectively).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added new comments to explain it. Does that help enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They definitely help, but what tripped me up is really the reuse of the letter "a" to mean both "a" (in the comment) and "2*a" (in the code). so maybe the comment or the code could instead talk about x and y instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, but I'm disinclined to make more changes. The overwhelmingly important concept here is "midpoint", and I don't want to obscure it under layers of pedantic name proliferation. In my mind, the midpoints are named "a" and "b", period, and the shift in perspective by 1 bit position is a triviality much better overlooked than emphasized 😉