perf(arrow-ord): Avoid full index materialization for small-limit lexsorts#9991
Open
pchintar wants to merge 1 commit into
Open
perf(arrow-ord): Avoid full index materialization for small-limit lexsorts#9991pchintar wants to merge 1 commit into
pchintar wants to merge 1 commit into
Conversation
b3b7fe3 to
eb9a67b
Compare
eb9a67b to
c3fdb35
Compare
Contributor
Author
|
@alamb & @etseidl I investigated this integration failure locally since the failure appeared unrelated to the changes in my PR. My PR only modifies
I also checked the local integration corpus and could not find a I also noticed the same |
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.
Which issue does this PR close?
Rationale for this change
In
arrow-ord/src/sort.rs,lexsort_to_indicescurrently materializes row indices for the full input before applying the requested lexsort limit.For example, with:
the current implementation still allocates and initializes indices for all 4096 rows:
even though only the first 10 sorted indices are returned.
This PR reduces allocation and sorting work for small-limit lexsorts by avoiding full index materialization when the requested limit is a small fraction(limit < 1/10 th of row count) of the input size.
What changes are included in this PR?
This PR adds a bounded top-k heap path for small-limit lexsorts in
arrow-ord/src/sort.rs.The new path:
The existing partial-sort implementation remains unchanged for larger limits.
To support the bounded heap implementation, this PR also adds:
lexsort_topk_fixedlexsort_topksift_up_worst_heapsift_down_worst_heapAre these changes tested?
Yes.
Existing tests pass:
My local Benchmark results from:
show improvements for small-limit lexsorts such as limits 10, 100.
Are there any user-facing changes?
No.