perf: Bring over apache/arrow-rs/9683 radix sort, integrate into ExternalSorter#21525
perf: Bring over apache/arrow-rs/9683 radix sort, integrate into ExternalSorter#21525mbutrovich wants to merge 1 commit intoapache:mainfrom
Conversation
…to choose between sort implementations.
|
run benchmark sort |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing arrow_rs_9683 (e1e1d09) to 8d91fb0 (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
Benchmark results show no improvement, which caused me to look at the sorting state machine a bit more.
We can't simply raise the threshold because Shelving the DataFusion integration and focusing on landing the arrow-rs kernel for now. |
|
I'm also not sure why we default to batch size of 1024 for the sort benchmarks. |
Which issue does this PR close?
N/A.
Rationale for this change
MSD radix sort on row-encoded keys is 2-3x faster than
lexsort_to_indicesfor most multi-column sorts. See apache/arrow-rs#9683 for benchmarks and design rationale.The arrow-rs kernel hasn't been released yet, so this PR temporarily hoists the implementation into DataFusion. Once arrow-rs ships it, we replace the local copy with the import.
What changes are included in this PR?
sorts/radix.rs(new): MSD radix sort kernel +RowConverterwiring, copied from feat(arrow-row): add MSD radix sort kernel for row-encoded keys arrow-rs#9683.sorts/sort.rs:sort_batchbranches to radix sort whenfetchis not set and column types are favorable.ExternalSorterprecomputes the radix-vs-lexsort decision once at construction rather than per-batch.sorts/stream.rs:IncrementalSortIteratoraccepts the precomputeduse_radixflag.The heuristic falls back to
lexsort_to_indiceswhen:fetchis set (partial sort / top-N)Are these changes tested?
4 new tests:
The kernel itself has 17 tests in the arrow-rs PR. The 188 existing sort tests in
datafusion-physical-plancontinue to pass.Are there any user-facing changes?
No.