Added a dedicated sort to core:sort with inline cmp for high performance - #7425
Open
TheRadischen wants to merge 13 commits into
Open
Added a dedicated sort to core:sort with inline cmp for high performance#7425TheRadischen wants to merge 13 commits into
TheRadischen wants to merge 13 commits into
Conversation
Contributor
|
You should fix typos in prtition, and lumoto -> lomuto. :) I wonder if changing this if left < right {
loop(arr[:left], data, 0)
arr = arr[left + 1:]
last_piv = -1
} else {
loop(arr[left + 1:], data, -1)
arr = arr[:left]
last_piv = 0
}to if left < right {
loop(arr[:left], data, last_piv) // arr[:left] is same base as arr, so arr[-1] translates directly to the subarray
arr = arr[left + 1:]
last_piv = -1
} else {
loop(arr[left + 1:], data, -1)
arr = arr[:left]
// last_piv = 0 // keep last_piv here by the above logic
}would save comparisons for data with many duplicates. |
Contributor
Author
|
thanks for the corrections, now we just check if we are in the leftmost partition, where we use normal insertion sort and cant compare pivots |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
slice.sort is versatile, but slow. #7423 doesnt change that either
a dedicated sort for when performance matters would be usefull
#6739 tried to start that, but that went nowhere. this implementation is ~2x faster
Implementation
branchless quicksort + insertion sort
uses median3 rekursively to find the pivot
partitioning is with gapped lumoto for small types and hoare for types > 80 byte
unrolled recursion into bigger partition to keep stack usage low
everything is in-place exept temp variables
main improvements can be found with the smallsort, but there is not much that can be done without using extra memory
because it uses a compile time comparison proc, it generates a new function each time it is called with any change, so i added a warning to each proc.
the name is explicit about what it does: sort_inlined to inicate that the comparison proc gets inlined for more performance, but also more code gen
// WARNING: each call generates a new quicksort, only use in performance critical path
Written with simplicity in mind
Should i add more documentation?
API
sort_inlinedsort_inlined_bysort_inlined_with_indicessort_inlined_by_with_indicessort_inlined_by_with_datasort_inlined_by_with_indices_with_datasort_inlined_by_cmpsort_inlined_by_cmp_with_dataBenchmarks
Within 20% of optimized sorts like ipnsort but significantly simpler and smaller
Bench code: https://github.com/TheRadischen/tests/blob/main/inline/test_inline.odin