comphead opened a new pull request, #26698: URL: https://github.com/apache/datafusion/pull/26698
## Which issue does this PR close? - Closes #26627. Stacked on #26688, because lengths are not valid offsets and only read back when spill reads skip validation. Only the last commit is new here. ## Rationale for this change See #26627: spill files are mostly `Utf8` offsets, which zstd and lz4 barely compress, while lengths repeat. ## What changes are included in this PR? - Compressed spill files (`lz4_frame` and `zstd`) hold lengths instead of offsets for `Utf8`, `Binary`, `List`, `Map` and their large variants, at any depth through `Struct` and `FixedSizeList`. The buffer is `[0, len_0, ..., len_(n-2), total]`. The IPC writer reads only its first and last entries, to slice the values, so the stock encoder writes it as is. - The reader rebuilds the offsets of compressed record batches with a running sum, in place in the decompressed buffer. It copies only a buffer that the IPC writer stored raw because compressing it did not help. - Uncompressed spill files are unchanged. The issue's reproducer (`datafusion-cli -m 100m`, median of 7 interleaved runs): | codec | spilled, main -> PR | query | |---|---:|---:| | zstd | 32.0 -> 8.6 MB | 314 -> 274 ms | | lz4_frame | 54.0 -> 16.8 MB | 258 -> 250 ms | TPC-H SF1 sorts with `Utf8` columns (`schema_force_view_types = false`, one partition, `-m 200m`, median of 5 runs): | query | lz4_frame | zstd | |---|---:|---:| | sort-tpch Q3 | 92.4 -> 78.0 MB, +0.8% | 61.4 -> 50.3 MB, +1.1% | | sort-tpch Q7 | 306.3 -> 229.5 MB, +0.6% | 171.3 -> 122.0 MB, -2.5% | | sort-tpch Q10 | 398.9 -> 308.7 MB, +3.0% | 228.2 -> 168.4 MB, -2.7% | In the `spill_io` benchmark against #26688, files with strings are 27% to 41% smaller. Writing plus reading them takes up to 7% less time with zstd and about 8.5% more with lz4, because lz4 stored the incompressible offsets raw and now compresses the lengths. Open question: should `lz4_frame` keep this, or only `zstd`? ## What is the testing strategy for this PR? - `test_spill_offsets_as_lengths_round_trip` spills every rewritten type, nested and sliced, with all three codecs. Its empty and two-row batches cover buffers the IPC writer stores raw. - `test_spill_offsets_as_lengths_compress` checks that a zstd spill of similar strings is under a quarter of plain zstd IPC. - `test_restore_offsets_in_place_unless_shared` checks that the restore reuses an unshared buffer. ## Are there any user-facing changes? Smaller spill files with `lz4_frame` or `zstd`. No configuration or API changes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
