adriangb commented on PR #25910: URL: https://github.com/apache/datafusion/pull/25910#issuecomment-6425035757
Reworked as discussed: this PR now changes only what is written to spill files. https://github.com/apache/datafusion/pull/25565 sizes the merge output. Production code is now about 260 lines (was about 950). The commits are split for review: failing test → refactor → bench → fix → test updates. @andygrove: | # | Status | |---|---| | 1, 2 | Merge output cap removed. Output batches and `size_intermediate_merges` are as on `main`. | | 3 | Bound is computed from `MIN_SPILL_MERGE_FAN_IN × get_reserved_bytes_for_record_batch_size`. Floor is 64 KiB, also for `0`. | | 4 | Custom compaction removed. Pieces are copied with `MutableArrayData`. | | 5 | A batch within the bound is written as on `main`, so its dictionary stays shared. New `sort_spill` bench, dictionary case: 35 ms (`main`) → 21–25 ms. | | 6, 7 | Removed. Each piece is copied and measured once. It is halved again only if it is still too large. | | 8 | Size model removed. Uses `get_sliced_size`, `gc_view_arrays` and `garbage_collect_any_dictionary`. | | 9 | The bound is set on the writer, not on `SpillManager`. `spill_pool` and SMJ cannot get split batches. | ```rust spill_manager .create_in_progress_file("Sorting")? .with_max_batch_bytes(Some(bound)) ``` @Weijun-H: - `ListView`, `Union`, REE and nested dictionaries: the batch is written whole. Unit test: 1,024 rows that share one 64 KiB child. - Pieces are written one at a time. - List/Utf8View accounting is still needed in #25565 before #25853 removes its cap. Test change: `test_sort_with_limited_memory_and_oversized_record_batch` now checks that the sort spills and finishes. The merge no longer needs the half-batch re-spill, which keeps its unit tests in `multi_level_merge.rs`. -- 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]
