YimingQiao opened a new pull request, #10708: URL: https://github.com/apache/arrow-rs/pull/10708
# Which issue does this PR close? - Part of #10692. - Related to apache/datafusion#16206. # Rationale for this change DataFusion hash joins commonly produce a `take` + `BatchCoalescer` pattern. In the high-fanout reproduction from #10692, a `ByteView` array can carry thousands of buffer handles. `take_byte_view` and `filter_byte_view` currently call `data_buffers().to_vec()`, so every selection clones and later drops every `Buffer`, even though `GenericByteViewArray` has stored the collection as `Arc<[Buffer]>` since #9016. This can be viewed through the same lens as our SIGMOD 2025 paper, [*Data Chunk Compaction in Vectorized Execution*](https://yimingqiao.github.io/files/data-chunk-compaction-sigmod25.pdf). The paper studies when compaction is worth its copying cost because fragmented chunks make downstream interpretation expensive. `ByteView` exposes a reference-level variant: the rows and string payload can both be dense, while the indirect buffer table is fragmented and expensive to repeatedly interpret. This PR removes the repeated ownership work in selection kernels; a follow-up will canonicalize duplicate buffer references in `BatchCoalescer` itself. # What changes are included in this PR? - Add `GenericByteViewArray::data_buffers_ref` to expose the existing shared `Arc<[Buffer]>`. - Make `take_byte_view` and `filter_byte_view` clone that `Arc` in O(1), rather than allocate a new list and clone every `Buffer` in O(number of buffers). - Verify that StringView and BinaryView selection outputs share the input buffer list. This does not retain any payload that was not already retained: the old implementation cloned every input `Buffer` into the output. Values, null handling, GC policy, and buffer indexes are unchanged. # Are these changes tested? ```shell cargo fmt --all -- --check cargo test -p arrow-array -p arrow-select cargo clippy -p arrow-array -p arrow-select --all-targets --all-features -- -D warnings ``` On an Intel Xeon Platinum 8474C, the existing `take_kernels` benchmark changed as follows: | Benchmark | main | this PR | Change | |---|---:|---:|---:| | `take stringview 512` | 519.00 ns | 370.31 ns | -28.7% | | `take stringview 1024` | 717.62 ns | 637.13 ns | -11.3% | | `take stringview null indices 512` | 541.42 ns | 541.85 ns | no change | I also ran 2,000 takes of 8,192 rows while varying the number of entries in a shared-payload buffer table: | Buffer entries | main | this PR | Speedup | |---:|---:|---:|---:| | 0 | 8.39 ms | 8.32 ms | 1.01x | | 1 | 8.40 ms | 8.33 ms | 1.01x | | 64 | 10.29 ms | 8.31 ms | 1.24x | | 4,096 | 128.79 ms | 8.39 ms | 15.35x | The existing mixed StringView filter benchmarks showed no regression; the low-selectivity case improved by 1.8%. # Are there any user-facing changes? There is one non-breaking public accessor, `data_buffers_ref`. Selection results now share the same immutable buffer collection rather than independently allocating an equivalent collection. # AI assistance I used OpenAI Codex to help inspect the related implementation history, draft the initial patch and tests, and prepare the benchmark harness and PR text. I reviewed the implementation and benchmark methodology and ran the checks above locally. -- 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]
