This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11525-c07cd7a9be59f09aa97c6d66336c4fd042509eda in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 626984707f73fd01a6f17f5576a39988c5c364d0 Author: Matt Butrovich <[email protected]> AuthorDate: Tue Oct 13 11:15:05 2026 +0000 perf(arrow-array): move the closure into the iterator in `PrimitiveArray::unary` (#11525) # Which issue does this PR close? - Part of #11421. # Rationale for this change [`PrimitiveArray::unary`](https://github.com/apache/arrow-rs/blob/01b1275aab90c033da3f64f5edbfb4f061606cae/arrow-array/src/array/primitive_array.rs#L933) maps the values with `|v| op(*v)`, which captures `op` by reference. When `Vec::from_iter` isn't inlined into `unary`, the loop reads the captures of `op` through that reference and reloads them for every value. I found this in #11467 ([comment](https://github.com/apache/arrow-rs/pull/11467#issuecomment-6085517937)), where it kept LLVM from moving a `match` on the divisor out of the loop. The decimal downscale cast on `main` has the same problem. With 16 codegen units, the default, `from_iter` for the infallible closure from [`make_downscaler`](https://github.com/apache/arrow-rs/blob/01b1275aab90c033da3f64f5edbfb4f061606cae/arrow-cast/src/cast/decimal.rs#L238-L263) isn't inlined. In the thin LTO disassembly for aarch64 and x86-64, the `Decimal64` loop reloads `div`, `half` and `half_neg` for each value, checks `div` for zero and -1, and branches on the sign of the value. With `op` moved into the closure, it loads the captures and checks `div` once, before the loop, and the rounding uses `csel` on aarch64 and `cmov` on x86-64 instead of a branch. # What changes are included in this PR? `unary` maps the values with `move |v| op(*v)`, so the iterator holds `op` instead of a reference to it. # Are these changes tested? The existing tests cover `unary`, and its results don't change. The benchmark bot ran the `decimal_downscale` cases of the `cast_decimal` benchmark from #11426 on [x86_64](https://github.com/apache/arrow-rs/pull/11525#issuecomment-6128933620) (Xeon Platinum 8581C) and [arm64](https://github.com/apache/arrow-rs/pull/11525#issuecomment-6128950402) (Neoverse-V2). <img width="1600" height="1124" alt="unary-move-bot" src="https://github.com/user-attachments/assets/c1d7d1f6-f14b-4ff6-ab1e-c7b0db5ebc9f"; /> <details><summary>Numbers behind the chart</summary> In us per 8192 values with no nulls: | Case | x86_64: `main` | This PR | arm64: `main` | This PR | |---|---|---|---|---| | `Decimal32` (9, 4) to (9, 2) | 29.1 | 15.0 | 30.4 | 30.4 | | `Decimal64` (18, 6) to (18, 2) | 25.3 | 27.6 | 46.7 | 46.7 | | `Decimal128` (18, 6) to (18, 2) | 74.0 | 67.9 | 51.7 | 48.2 | | `Decimal128` (38, 10) to (38, 2), 15 digits | 74.1 | 67.8 | 49.4 | 42.9 | | `Decimal128` (38, 10) to (38, 2), 38 digits | 83.6 | 77.8 | 114.7 | 115.5 | | `Decimal128` (38, 20) to (38, 2), 38 digits | 83.7 | 68.6 | 170.2 | 170.7 | </details> The `Decimal256` and fallible cases changed by 6% or less on both runners. On x86_64, `Decimal64` was 9% slower without nulls and 2% slower with nulls. `unary` computes every slot either way, so these two cases should cost the same, and I don't know yet why they differ. The single-value case of `decimal_downscale_small` was 8% slower on x86_64 (138 vs 150 ns). The gain depends on the number of codegen units. These numbers are from an Apple M5 Max with rustc 1.99.0, in us per 8192 values with no nulls. <details><summary>Apple M5 Max by build setting</summary> | Case | 16 codegen units, no LTO: `main` | This PR | 1 codegen unit, fat LTO: `main` | This PR | |---|---|---|---|---| | `Decimal32` (9, 4) to (9, 2) | 7.25 | 4.56 | 4.50 | 4.49 | | `Decimal64` (18, 6) to (18, 2) | 7.21 | 4.48 | 4.30 | 4.22 | | `Decimal128` (18, 6) to (18, 2) | 21.36 | 19.52 | 20.27 | 20.10 | </details> Thin LTO with 16 codegen units gave a similar gain (6.06 to 4.49 us for `Decimal32`, 6.86 to 4.27 us for `Decimal64`), and thin LTO with one codegen unit measured the same as fat LTO. With one codegen unit, `main` is already as fast as this PR. # Are there any significant user-facing changes? No. ## AI Disclosure I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage. --- arrow-array/src/array/primitive_array.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arrow-array/src/array/primitive_array.rs b/arrow-array/src/array/primitive_array.rs index b88089f527..ebe02103a3 100644 --- a/arrow-array/src/array/primitive_array.rs +++ b/arrow-array/src/array/primitive_array.rs @@ -930,7 +930,9 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> { F: Fn(T::Native) -> O::Native, { let nulls = self.nulls().cloned(); - let values = self.values().into_iter().map(|v| op(*v)); + // `move` puts `op`'s captures in the iterator rather than behind a reference, so the + // loop doesn't reload them for each value when `collect` isn't inlined + let values = self.values().into_iter().map(move |v| op(*v)); let buffer: Vec<_> = values.collect(); PrimitiveArray::new(buffer.into(), nulls) }
