Repository navigation
Use NEON packing for wide primitive array comparisons - #10843
Merged
Merged
Conversation
Use the existing fill-and-pack collector for i64, u64, and f64 array pairs on AArch64 with NEON. Keep this dispatch and its bounds proof inside the private primitive comparison kernel. Extend word-boundary coverage to wide types with sliced inputs and exercise floating-point total ordering through full words and a tail. Signed-off-by: Connor Tsui <[email protected]>
Merging this PR will regress 1 benchmark4 warnings, 1 note
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
| f: impl Fn(T, T) -> bool, | ||
| allocator: &BufferAllocatorRef, | ||
| ) -> BitBuffer { | ||
| #[cfg(all(target_arch = "aarch64", target_feature = "neon"))] |
Member
Author
There was a problem hiding this comment.
I don't think neon is an architecture though?
mhk197
approved these changes
Oct 27, 2026
Member
Author
|
From Claude so take with a grain of salt:
DetailsNow the two lower-level kernels: the existing `collect_zip_bits` lane kernel and the multiversioned fill-and-pack loop.The short answer is that x86 can turn a vector compare into a bitmask in one instruction, at every element width. NEON can't. The lane kernel is near-optimal on x86 and expensive on ARM. This is reasoning from the code, not from reading the generated assembly, so treat it as a hypothesis. The two kernel shapes
Why x86 gains nothing
Why NEON gains
Two side notes
If you want to confirm the core claim cheaply, look at the aarch64 assembly for the i64 lane kernel. If it is scalar |
connortsui20
added a commit
that referenced
this pull request
Nov 5, 2026
Tracking Issue: #9128 ## Summary Combines planning and execution for eligible RowFn batches in one type dispatch. This is the shared framework layer for #10676, which contains the primitive comparison migration. It builds on #10899, which separates batch inspection from planning and adds the regression cases this PR must keep passing. ## Changes - Decodes a direct Boolean batch straight from the borrowed inputs, after `BatchProbe` has classified all of them, so a later null literal still skips an earlier fallible expression or device transfer. Fallible decoders keep valid-row execution. - Finishes all-null, all-constant, null-free, and dense batches in the planning visit. Every other batch comes back as a `PartiallyValidBatch`, the only input that dense retry and valid-row execution accept, so every caller runs the fast paths first. - Constructs Boolean output with its validity in the shared executor. Dense execution masks any output that does not carry that validity, and a validity of the wrong length returns an error. Primitive-specific CPU and lane-width packing policies stay out of this layer. - In builds that enable AVX-512F and AVX-512BW at compile time, `collect_bool_words_multiversioned` uses the inline packing loop instead of the `#[target_feature]` copy, so its inlining no longer depends on codegen-unit placement. ## Performance This PR lowers the fixed cost of each RowFn call, so CodSpeed shows no gain for it alone. The gain shows up in #10676, which moves primitive comparison onto the RowFn executor. To isolate this PR, #11220 applied the two #10676 commits directly on `develop`. Without this PR, comparisons on small arrays are 10% to 16% slower than `develop` in CPU simulation, including the zone map pruning benchmarks, `i64_lt`, and `timestamp_gte`. The added time barely changes between 16 and 1,024 zones (18.0 µs and 17.3 µs for `int_gt`), so it is paid per call, not per row. On top of this PR, #10676 shows none of these regressions, and its walltime comparison gains are larger: `compare_u64_avx2` improves by 31% instead of 16%, and seven more comparison benchmarks improve by 12% to 18%. In the AVX-512 build, the RowFn changes moved the deferred Boolean kernel's AVX-512 packing loop out of its caller's codegen unit, so LLVM could no longer inline it. CodSpeed reported the two partially valid `deferred_bool_avx512` cases as 42% and 16% slower. With the last commit, CodSpeed no longer flags either case. Builds that detect AVX-512 at runtime compile the same code as before. #11220 also shows `compare_u8_avx512` and `compare_f32_avx512` as 14% and 10% slower without this PR, which matches the same packing problem, but that build's assembly wasn't inspected. CodSpeed on the current head flags three varbin comparison cases and `random_i8[0.01]` as 12% to 18% slower, and one `take_filter` case as 13% faster. None of them runs a RowFn. #10676 reverses the varbin and `take_filter` changes without touching their code, so they follow code placement. `random_i8[0.01]` isn't a stable signal either: its `develop` baseline moved from 99.6 µs to 119.3 µs between `bc58587` and `c9a3fbe`, and #11220 moved it 13% the other way. <details> <summary>CodSpeed results for #10676 with and without this PR</summary> The first column is #11220, which compares the #10676 commits on `develop` with `develop` (`c9a3fbe`). The second column is the #10676 report, which compares the same commits on this PR with this PR (`b0a64dd`). Against `develop` (`bc58587`), this PR flags none of these benchmarks. CodSpeed marks the walltime runners as unreliable, and the two columns come from different runs. | Benchmark | Mode | Without this PR | With this PR | | --- | --- | ---: | ---: | | `int_gt[16]` | CPU simulation | 15.6% slower | not flagged | | `int_gt[1024]` | CPU simulation | 14.5% slower | not flagged | | `float_gt[16]` | CPU simulation | 12.2% slower | not flagged | | `float_gt[1024]` | CPU simulation | 12.1% slower | not flagged | | `or_chain[16]` | CPU simulation | 12.3% slower | not flagged | | `or_chain[1024]` | CPU simulation | 11.9% slower | not flagged | | `is_not_null_pred[16]` | CPU simulation | 11.6% slower | not flagged | | `is_not_null_pred[1024]` | CPU simulation | 10.5% slower | not flagged | | `i64_lt` | CPU simulation | 11.7% slower | not flagged | | `timestamp_gte` | CPU simulation | 10.3% slower | not flagged | | `bench_compare_sliced_dict_primitive[(1000, 10000)]` | CPU simulation | 11.5% slower | not flagged | | `encode_primitives[f32, (2000, 2)]` | CPU simulation | 12.8% slower | not flagged | | `compare_u8_avx512` | Walltime | 14.4% slower | not flagged | | `compare_u8_neon` | Walltime | 13.3% slower | not flagged | | `compare_f32_avx512` | Walltime | 10.4% slower | not flagged | | `compare_float_avx2` | Walltime | 18.4% faster | 28.2% faster | | `compare_u64_avx2` | Walltime | 15.7% faster | 30.8% faster | | `lt_i64_nullable_avx2` | Walltime | not flagged | 18.4% faster | | `compare_int_constant_neon` | Walltime | not flagged | 16.2% faster | | `compare_int_eq_avx2` | Walltime | not flagged | 15.4% faster | | `compare_f32_avx2` | Walltime | not flagged | 15.4% faster | | `compare_int_constant_avx2` | Walltime | not flagged | 15.3% faster | | `compare_int_avx2` | Walltime | not flagged | 13.5% faster | | `compare_int_nullable_avx2` | Walltime | not flagged | 12.5% faster | </details> <details> <summary>AVX-512 investigation on EC2</summary> C7i metal (Xeon Platinum 8488C) with the CodSpeed AVX-512 build flags, the bench profile, one isolated CPU, and 21 paired blocks of 1,000 samples. Ratios are the median paired ratio to `develop` (`8c87c4c457`). Below 1 is faster. An A/A comparison of two identical binaries flags none of the 100 cases. | `deferred_bool_avx512` case | `develop` | Before fix | With fix | | --- | ---: | ---: | ---: | | `ConstantLhs, PartialAccepted` | 32.1 µs | 1.354 | 1.025 | | `Columns, PartialAccepted` | 37.7 µs | 1.159 | 1.018 | Before the fix, the kernel executed 15% more instructions per row. LLVM inline remarks show `collect_bool_words_avx512` inlined on `develop` and "definition is unavailable" on this branch. Across all 100 `#[cpu_features]` cases, the fix also returns `compare_string_lt` to the `develop` speed. It makes two `intersect_by_rank` cases and one `starts_with` case 3.5% to 5.6% slower, but those execute the same instructions as before, so the difference is code placement. The AVX2 build is unchanged. </details> <details> <summary>Historical EC2 framework measurements</summary> These numbers belong to an older framework replay and do not establish the performance of this revision. Ratios are the median of 21 paired candidate/baseline elapsed-time ratios. Below 1 is faster. This compares the framework alone, F `94be28140d5bd7e6b654af5e462253add2e49044`, with D `0291778db36809f77ce0219bf9850506d3cba3b6`. | Workload | C7g NEON | C6a AVX2 | C7i AVX2 build | C7i AVX-512 build | | --- | ---: | ---: | ---: | ---: | | `u64` multiply, 8,192 rows | 1.732 | 0.979 | 0.992 | 0.998 | | Nullable `i64` add, 32,768 rows | 1.006 | 1.004 | 1.154 | 1.005 | The actual framework pair order was imbalanced: D ran before F in 16 blocks and after it in five. Treat these figures as diagnostic observations, not reliable causal effect sizes. EC2 C7g (Neoverse V1), C6a (EPYC 7R13), and C7i (Xeon Platinum 8488C). Rust 1.99.1 / LLVM 23.1.1, bench profile with 16 CGUs and no LTO, MiMalloc, CPU 2 pinned. The same 455-case public-path microbenchmark suite ran serially across hosts, with 21 paired blocks, 10 ms per case, A/A calibration, and a separate control executable. These are microbenchmarks, not query results. C7i can dispatch to AVX-512 at runtime even in its AVX2 build. </details> The separately confirmed NEON comparison gains were published in #10843, which has merged. They do not measure this framework. CI passed on `b0a64dd`, including all-target, all-feature Clippy with warnings denied and the Linux, Windows, sanitizer, and Miri test jobs. The CodSpeed check fails on the flags described above. --------- Signed-off-by: Connor Tsui <[email protected]>
connortsui20
added a commit
that referenced
this pull request
Nov 12, 2026
Tracking Issue: #9128 ## Summary Moves primitive comparisons onto the shared RowFn executor from #10675, so comparison code only selects the predicate while the framework handles decoding, constants, validity, and Boolean output. ## Changes Preserves the public `Binary` identity and floating-point total ordering. Uses borrowed operands and dtypes, and moves a constant left operand to the right with the swapped operator. Relies on the existing `primitive_comparisons_across_bitmap_words` test for all six swapped operators and keeps the existing comparison benchmarks. Based on #10675. Keeps the wide-type, sliced-input, bitmap-boundary, NaN, and signed-zero tests added upstream in #10843. ## Performance CodSpeed compares this PR with #10675 (`b0a64dd`). Nine primitive comparison benchmarks are 12% to 31% faster in walltime, led by `compare_u64_avx2` and `compare_float_avx2` at 31% and 28%, and no comparison benchmark regresses. These gains depend on #10675. #11220 measured the same two commits directly on `develop`. Without #10675, comparisons on small arrays were 10% to 16% slower than `develop` in CPU simulation, and only two walltime gains remained, at 16% and 18%. The #10675 description has the full comparison. The baseline already includes the private NEON packing kernel from #10843, which this PR replaces with RowFn execution. So the gains above are measured against that improved baseline, and the old packing gains aren't credited to this PR. <details> <summary>CodSpeed comparison benchmarks against #10675</summary> CodSpeed marks the walltime runners as unreliable, but all nine cases move in the same direction. | Benchmark | Faster by | | --- | ---: | | `compare_u64_avx2` | 30.8% | | `compare_float_avx2` | 28.2% | | `lt_i64_nullable_avx2` | 18.4% | | `compare_int_constant_neon` | 16.2% | | `compare_int_eq_avx2` | 15.4% | | `compare_f32_avx2` | 15.4% | | `compare_int_constant_avx2` | 15.3% | | `compare_int_avx2` | 13.5% | | `compare_int_nullable_avx2` | 12.5% | </details> <details> <summary>Historical EC2 accumulated-stack measurements</summary> These runs measured an earlier accumulated stack, not this layer in isolation or the current head. They showed gains on some configurations and regressions on others. Ratios are the median of 21 paired candidate/baseline elapsed-time ratios. Below 1 is faster. This compares the earlier complete migration, M0 `ddf8a16df6b7d7c20cfd860e5f23438d97ca4238`, with D `0291778db36809f77ce0219bf9850506d3cba3b6`. That baseline predates #10843. The array-pair cases use all-valid nullable inputs. The addition case is an arithmetic control whose algorithm was unchanged. | Workload | C7g NEON | C6a AVX2 | C7i AVX2 build | C7i AVX-512 build | | --- | ---: | ---: | ---: | ---: | | `f32` array-pair Gte, 8,192 rows | 1.014 | 1.040 | 0.873 | 1.036 | | `i64` array-pair Gte, 8,192 rows | 0.900 | 1.139 | 0.929 | 0.943 | | Nullable `i64` add, 32,768 rows | 1.002 | 1.123 | 1.154 | 1.004 | For example, the old stack took about 10% less time for the ARM `i64` comparison, but about 14% more for the same case on AVX2-only C6a. C6a nullable `i64` addition took about 12% more time. These whole-executable measurements do not establish that the comparison algorithm caused movement in arithmetic controls. In the four-label matrix, D ran before M0 in 11 blocks and after it in ten. The experimental one-pass decoder variants are excluded from this table. EC2 C7g (Neoverse V1), C6a (EPYC 7R13), and C7i (Xeon Platinum 8488C). Rust 1.99.1 / LLVM 23.1.1, bench profile with 16 CGUs and no LTO, MiMalloc, CPU 2 pinned. The same 455-case public-path microbenchmark suite ran serially across hosts, with 21 paired blocks, 10 ms per case, A/A calibration, and a separate control executable. These are microbenchmarks, not query results. C7i can dispatch to AVX-512 at runtime even in its AVX2 build. Saved evidence is in the local investigation archive `row-fn-evaluation-20261026`: `manifest.json`, `*-matrix-*-medians.csv`, `*-matrix-*-summary.json`, and `results/<host>/results/matrix-*.json`. </details> <details> <summary>Confirmed independent packing gains, now in develop via #10843</summary> The ARM-only private-kernel change reduced long array-pair cohort time on Graviton3 as follows. Each cohort contains 32 cases, including partially valid inputs and lengths around 16K. | Cohort | Confirmation elapsed-time ratio | Paired 95% interval | Time reduction | | --- | ---: | --- | ---: | | `i64` | 0.846 | 0.843 to 0.856 | 15.4% | | `u64` | 0.860 | 0.858 to 0.864 | 14.0% | | `f64` | 0.882 | 0.880 to 0.886 | 11.8% | Baseline `a59294e35e7109fac894a7a38a68f42db0c3c967`, candidate `f018aaeef739145e1bc50079a441c5d9fd53404a`. The cohort uses a geometric mean of case ratios within each block, then the median across blocks. Two 21-block sessions used complementary AB/BA orders, the same full 455-case suite, and 1,000 single-iteration samples per case. The initial fixed-order source comparisons were discarded. The AVX2 builds had identical `.text` bytes, so no x86 speedup was attributed to this change. See #10843 for the harness and full methodology. These gains required neither this PR nor #10675. </details> On `e724610`, CI passed all-target, all-feature Clippy with warnings denied and the Linux, CPU feature, sanitizer, and Miri test jobs. --------- Signed-off-by: Connor Tsui <[email protected]>
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.
Related to #10676. This PR is independent of the RowFn stack.
Uses the existing fill-and-pack collector for
i64,u64, andf64array-pair comparisons on AArch64 with NEON. The dispatch and the unchecked slice reads stay inside the private primitive kernel, where equal lengths and the collector's index range establish the bounds proof. Constant comparisons, other primitive types, and validity handling retain their existing paths.Extends bitmap-boundary tests to wide types with differently sliced inputs, and checks floating-point total ordering through full words and a tail.
Validation: 236 comparison tests and all-target, all-feature Clippy with warnings denied pass on macOS ARM. Native ARM and AVX2 builds pass the 455-case probe (452 reference-array checks plus three unchanged Boolean controls) and 3,456 comparison edge checks plus four arithmetic error checks.
On Graviton3, the two counterbalanced full-suite sessions show the following elapsed-time reductions in the long array-pair cohorts:
i640.846 [0.843, 0.856]u640.860 [0.858, 0.864]f640.882 [0.880, 0.886]Each cohort contains 32 cases. Ratios are candidate/baseline, with a geometric mean across cases within a paired block and the median across 21 blocks. Intervals bootstrap those paired block ratios. Neither ARM session has a case crossing the 3% regression threshold with its interval above 1.
The AVX2 baseline and candidate
.textsections are byte-for-byte identical. Control timings still vary, so no x86 speedup is attributed to this change. These are public-path microbenchmarks, not query results. Release-profile and AVX-512 timing were not repeated for this ARM-only variant.Measurement protocol and reproduction
Baseline:
a59294e35e7109fac894a7a38a68f42db0c3c967. Candidate:f018aaeef739145e1bc50079a441c5d9fd53404a.Uses rustc 1.99.1 (LLVM 23.1.1), the repository's bench profile (16 codegen units, no LTO), MiMalloc, and CPU 2. Graviton3 uses
-C target-feature=+neon -C force-frame-pointers=yes. The AVX2 control uses-C target-feature=+avx2 -C force-frame-pointers=yeson AMD EPYC 7R13. Optimized LLVM IR and assembly were emitted during both builds, and the resulting executables were timed without instrumentation.Both labels run the same full 455-case suite in the same order. Each case has exactly 1,000 single-iteration samples. Each session has 21 baseline/baseline calibration blocks and 21 paired baseline/candidate blocks with alternating executable order. Separate arithmetic control binaries bracket the blocks. The two hosts are measured serially, and a second session confirms the result. The confirmation session reverses the starting order, so the combined sessions contain equal AB and BA counts. Preliminary fixed-order source comparisons were excluded, while valid identical-binary calibration was retained. The long-array cohorts each contain 32 cases, including partially valid inputs and lengths around the 16K boundary.
To reproduce, save the harness below as
vortex-array/benches/interaction_probe.rson each revision and add this target tovortex-array/Cargo.toml:Set
RUSTFLAGSto the host flags above. Build both revisions with those same flags, retain separate immutable executables, and alternate the commands below. UnsetVORTEX_PERF_CHECKfor timing.Exact benchmark harness
The measured harness SHA-256 is
0a7de7999381d237ad0471716107f7e1972a7c6b11c0936a7d3b7d8c15b6c37d.