Repository navigation
Conversation
`StreamEncoder` returns every buffer of a batch and its padding as a separate `Buffer`, and the spill writers wrote them one by one. For file backed spills that is one `write` system call per buffer, about 35 for a batch of six string and integer columns. Copy runs of buffers under 64 KB into one, so a spilled batch typically takes a single write.
Closes apache#26627. Offsets are a running total, so zstd keeps about 2.5 of every 4 bytes and lz4 all of them, while the lengths of similar values repeat. Compressed spill files now store each top level `Utf8`, `LargeUtf8`, `Binary` and `LargeBinary` column as a struct of its lengths and of its value bytes as a single run. Both are valid arrays, so the IPC reader decodes them as usual and the spill reader rebuilds the offsets with a running sum.
- `FileSpillWriter` writes through a 256 KB `BufWriter` instead of the IPC encoder copying small buffers together. One bounded buffer is reused, large buffers are not copied, and uncompressed spills benefit too. - Byte arrays read back from compressed spill files are rebuilt with `OffsetBuffer::try_from_lengths` and `GenericByteArray::try_new`, so a corrupted file fails instead of producing invalid arrays. - `IPCStreamEncoder` keeps the encoded schema and reuses its struct fields for every batch. - `aggregate_memory_spill.slt` spills byte array keys with zstd and lz4.
comphead
marked this pull request as ready for review
November 2, 2026 16:41
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?
Utf8offsets, which zstd and lz4 barely compress #26627.Rationale for this change
Compressed spill files are mostly string offsets, which zstd and lz4 barely compress, see #26627. Spilling is also slower than it needs to be: a batch of the table in #26627 takes about 35
writecalls, one per IPC buffer.This replaces #26698, which needed spill reads to skip validation (#26688).
What changes are included in this PR?
lz4_frameorzstd, each top levelUtf8,LargeUtf8,BinaryandLargeBinarycolumn is spilled as a struct of its value lengths and of its value bytes as one run-end encoded value. Both are valid arrays, so the IPC reader decodes them as usual before the spill reader rebuilds and validates the original arrays. Uncompressed spills keep their layout.FileSpillWriterwrites through a 256 KBBufWriter.Benchmarks
Apple M5 Max, interleaved runs against
main.The reproducer from #26627 (
datafusion-cli -m 100m, median of 7):sort_tpchqueries at SF1 onUtf8columns (one partition,-m 200m, median of 5), spilled bytes and time againstmain:In the
spill_iobenchmark, writing and reading spill files is 22-27% faster uncompressed, 4-9% faster with zstd and unchanged to 10% faster with lz4, and the string datasets spill 27-41% smaller.spill_ioresults and setupcargo bench -p datafusion-physical-plan --bench spill_io, mean of two alternating runs per build.The
sort_tpchruns set:What is the testing strategy for this PR?
aggregate_memory_spill.sltspillsUtf8,LargeUtf8,BinaryandLargeBinarygroup keys with nulls underzstdandlz4_frameand checks their values.test_spill_lengths_round_tripcovers sliced and empty batches, andtest_spill_lengths_compresschecks that a zstd spill of similar strings is under a quarter of plain zstd IPC.spill_pooltests read files while they are being written, which now relies on the flush after each batch.Are there any user-facing changes?
Smaller compressed spill files and faster spilling. No configuration or API changes.