Repository navigation
Description
Context
ExternalSorter branches on sort_in_place_threshold_bytes (default 1MB) in in_mem_sort_stream():
- Below 1MB: concatenate all buffered batches into one
RecordBatch, sort in place - Above 1MB: sort each batch individually, then streaming-merge them
This threshold was introduced in May 2023 by @tustvold in #6163 ("Adaptive in-memory sort") with the comment: "This is a very rough heuristic and likely could be refined further." It was later extracted to a config option by @alamb in #7130 with the same 1MB default. The default hasn't changed since, though the surrounding sort architecture has evolved significantly: multi-level merge (#15700), chunked sort output (#19494), IncrementalSortIterator (#20314), and PartialSortExec (#9125).
Problem
The sort-each-batch-then-merge path dominates real workloads because typical in-memory buffer sizes exceed 1MB. In this path, each batch (often 1024–8192 rows) is sorted individually via lexsort_to_indices and then merged via StreamingMergeBuilder. This means:
-
Per-batch sort kernels can't amortize overhead. Row-format sorting (e.g., MSD radix sort on
RowConverteroutput, feat(arrow-row): add MSD radix sort kernel for row-encoded keys arrow-rs#9683) is 2–3x faster thanlexsort_to_indicesat 32K+ rows, but at 1K–8K rows theRowConverterencoding cost dominates. The sort-then-merge path never gives these kernels enough rows to benefit. perf: Bring over apache/arrow-rs/9683 radix sort, integrate into ExternalSorter #21525 attempted to integrate the radix sort kernel intoExternalSorterand saw no improvement for this reason. -
The concat path is gated on memory, not row count. The 1MB threshold is a memory proxy, but the actual concern is the temporary 2x memory spike from
concat_batches(plusRowConverterallocation on top). A row-count or batch-count heuristic might be a better fit. -
The
sortbenchmark doesn't exercise the merge path. The benchmark produces 8 partitions of ~12 batches at 1024 rows each. In thesort partitionedvariant, each partition's ~12K rows (~100KB for integers) falls well below the 1MB threshold, so it always takes the concat-and-sort-in-place path. This means benchmark results don't reflect the sort-then-merge path that dominates at larger data sizes.
Prior art: DuckDB
For comparison, DuckDB's sort redesign encodes into its normalized key format as data arrives during the sink phase, accumulating into large thread-local sorted runs. The encoding cost is amortized across the entire input stream, and sorting happens once per run on already-encoded data. This avoids the small-batch problem entirely — by the time sorting begins, each thread has a single large run to sort.
Possible directions
These aren't mutually exclusive:
-
Raise or rethink the threshold. The 1MB limit was chosen conservatively. With
IncrementalSortIterator(added in fix: Unaccounted spill sort in row_hash #20314) now yielding sorted output in chunks, the peak memory of the concat-and-sort path may be more manageable than it was in 2023. Could we raise it, or gate on row count instead? -
Coalesce batches before sorting in the merge path. When the merge path is taken, we could concatenate small batches into larger ones (e.g., 32K–64K rows) before sorting, giving row-format kernels enough rows to amortize encoding. This also reduces the merge fan-in, which is related to Improve performance of large sorts with Cascaded merge / tree #7181 (cascaded merge for large fan-in). This doesn't require concatenating the entire buffer — just local coalescing.
-
Incremental
Rowsencoding.RowConverter::appendalready supports incrementally extending aRowsbuffer across batches.ExternalSortercould maintain aRowsalongside itsin_mem_batches, callingappendas each batch arrives (similar to DuckDB's approach). At sort time, the encoding is already done — you just sort the accumulatedRowsand use the indices to reorder the original batches. The tradeoff is higher memory during accumulation (raw batches + encoded rows), but encoding cost is fully amortized and radix sort gets a large contiguous run to work with. -
Fix the benchmark. Increase
BATCH_SIZEand/orINPUT_SIZEinbenches/sort.rsso thatsort partitionedexercises the sort-then-merge path.
Related issues
- Improve performance of large sorts with Cascaded merge / tree #7181 — Cascaded merge to reduce fan-in when merging many small sorted batches
- Operators should respect batch_size instead of Emit::All #19481 — Operators should respect
batch_sizeinstead ofEmit::All(large batches flowing intoExternalSortercause memory pressure) - Consider spilling batch in chunks in ExternalSorter. #19679 — Spill batches in chunks to avoid requiring full sort memory upfront
References
- Adaptive in-memory sort (~2x faster) (#5879) #6163 — Adaptive in-memory sort (introduced the 1MB threshold)
- Account for memory usage in SortPreservingMerge (#5885) #7130 — Extracted threshold to config
- perf: Bring over apache/arrow-rs/9683 radix sort, integrate into ExternalSorter #21525 — Radix sort integration attempt (benchmarks show no improvement due to small batch sizes)
- feat(arrow-row): add MSD radix sort kernel for row-encoded keys arrow-rs#9683 — MSD radix sort kernel (2–3x faster at 32K+ rows)
Activity
mbutrovich commentedon Apr 10, 2026
Leaving some notes to myself for next week, and anyone else to give feedback on. One idea I came up with:
Chunked sort with incremental accumulation
RecordBatches as they arrive (same as today).take(), stash as a sorted run.For radix-eligible types: encode the chunk to
Rows, radix sort, drop theRows. Encoding is transient. For dictionaries/nested types:lexsort_to_indiceson the concatenated chunk, same as the current sub-1MB path.This design is similar to the new immediate-mode shuffle writer in DataFusion Comet: apache/datafusion-comet#3845
Why this helps
lexsort_to_indicesat 32K rows (feat(arrow-row): add MSD radix sort kernel for row-encoded keys arrow-rs#9683), but needs large batches to amortizeRowConverterencoding. This gives it that.raw_chunk + Rows + sorted_outputfor one 32K-row chunk. Bounded and predictable. The 1MB threshold question mostly goes away — sort always operates on fixed-size chunks, memory pressure is handled by spilling sorted runs.For context, DuckDB's sort redesign takes this further — encoding into normalized keys as data arrives, accumulating large thread-local runs. This proposal is a step in that direction while staying within DataFusion's existing architecture.
Degenerate cases
I don't think there are new ones beyond what already exists. Fan-in is strictly better (fewer, larger runs). Skew doesn't affect radix sort (O(n × key_width)). Small datasets (< 32K rows) reduce to a single chunk — same as the current sub-1MB path. The existing multi-level merge handles large fan-in when many chunks are needed.
Dandandan commentedon Apr 11, 2026
Using https://docs.rs/arrow/latest/arrow/compute/struct.BatchCoalescer.html
might also help a bit (instead of using
concat_batches).(It still uses
takebut avoids the double memory usage caused byconcat_batches.Additionally it allows doing the copying earlier, which also reduces the "final" allocation/CPU spike and might also be a bit more efficient as batch is likely be in CPU cache when processed (and not when doing
concat_batcheson a large number of batches), spreading out deallocations, etc.(There is potential to make it even faster by fusing
take+push_batch(inpush_batch_with_indices, but that is yet to be implemented).Dandandan commentedon Apr 11, 2026
Another option would perhaps to increase the target
batch_sizeof a downstream operator (which already use the coalesce kernel) wheneverSortExecis present, this would avoid the double copying in both the operator andSortExecalamb commentedon Apr 13, 2026
I think this is a good idea to pursue as well -- I also wonder if we have already created data in the row format, we could avoid the second copy entirely perhaps by keeping a list of sorted indices and then merging using those rather than copying the data again 🤔
mbutrovich commentedon Apr 13, 2026
Yeah I was looking at this right now, in fact: could a radix sort in row format benefit the merge phase? It's more plumbing and might be one of the later PRs in a sequence of changes, but on paper it seems like a good idea.
bench: Scale sort benchmarks to 1M rows to exercise merge path (#21630)
mbutrovich commentedon Apr 16, 2026
So the TPC-H and TPC-DS results in #21629 are very exciting but I am concerned about regressions with string types (not string view) and generally wide schemas. I wonder:
take? I think joins do something similar when join keys span input batchesSorts are fun.
Dandandan commentedon Apr 16, 2026
We should be able to run the benchmarks with string types (with the right env variable)
mbutrovich commentedon Apr 17, 2026
I worked on several implementations tonight, summarizing findings (thanks Claude):
gratus00 commentedon Apr 21, 2026
Hi, related to the "Fix the benchmark" direction, I noticed a small benchmark gap in
datafusion/core/benches/sort.rs.The existing tuple/string/dictionary cases use
make_sort_exprs(schema), which sorts by every column. Would a small benchmark-only PR adding a case that includes a table with a cheap sort-able key such as ani64key and non-key utf8/dictionary payload columns but ONLY gets sorted by thei64key be useful?The goal would be to make the sort key cheap and better measure the cost of
takereordering wider payload column cases. I think this case is not covered at the moment.Would love this to be a way I start working on this project!
@Dandandan @mbutrovich
bench: Scale sort benchmarks to 1M rows to exercise merge path (apach…
mbutrovich commentedon Apr 22, 2026
Hi @gratus00! We could definitely use more sort benchmarks, and in fact some of what you're describing I have in #21688 if you want to look at those for inspiration and bring them to a separate PR.
I had Claude summarize my notes from over the weekend of looking at this. I'm kinda stumped. I can make TPC-H faster, but at the expense of other schemas I care about (e.g., unconditional coalescing hurts wide/large schemas).
gratus00 commentedon Apr 23, 2026
@mbutrovich I will take a deeper look into this, and will add the PR when I get the chance this week
But yeah seems like a trade off one way or another so far