perf(arrow-cast): speed up infallible numeric casts - #10958
Open
alexeigor wants to merge 5 commits into
Open
Conversation
- Explain in `cast_numeric_arrays` that `unary` runs the conversion on every slot, so the values buffer under a null now holds the converted input value instead of the zero written by `unary_opt` / `try_unary`. Undefined by the Arrow specification and consistent with the other `unary` kernels, but it does change the bytes an IPC writer emits. - State the real invariant on `is_infallible_numeric_cast`: agreement with `AsPrimitive::as_`, not merely that `num_cast` is total. Record why the set is conservative and why `Float16` is excluded. - The fast path converts with `AsPrimitive::as_`, so assert against that as well as against `as`, and note that the test pins `num_traits` behaviour the fast path depends on. - Add end to end coverage: every fast-path pair through `cast_with_options` under both `safe` settings and at several array offsets, compared against a per-element `num_cast` reference; `Float32 -> Float64` checked bitwise so NaN and signed zero are covered; and a negative control asserting narrowing, unsigned to signed of the same width, and signed to unsigned still return null under `safe` and an error under `safe: false`.
alexeigor
force-pushed
the
perf/infallible-numeric-casts
branch
from
September 2, 2026 17:08
6bc87b2 to
54c55dc
Compare
Contributor
|
run benchmark arrow-cast |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/infallible-numeric-casts (54c55dc) to 8c49c71 (merge-base) diff Run configurationrun benchmark arrow-castBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow-cast File an issue against this benchmark runner |
|
Benchmark for this request failed before finishing (Kubernetes reason: Benchmarks requested: Kubernetes messageFile an issue against this benchmark runner |
Author
|
Thanks for kicking off the benchmark @Rich-T-kid. That run failed on the trigger argument rather than on anything in the branch — $ cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow-cast
error: no bench target named `arrow-cast` in default-run packages
help: available bench targets:
aggregate_kernels
arithmetic_kernels
...which is what the 6-second The $ cargo bench --features=arrow,async,test_common,experimental,object_store --bench cast_kernels --no-run
Finished `bench` profile [optimized] target(s) in 21.60s
Executable benches/cast_kernels.rsCould someone re-run it as |
Contributor
|
run benchmark cast_kernels |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/infallible-numeric-casts (54c55dc) to 8c49c71 (merge-base) diff Run configurationrun benchmark cast_kernelsBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench cast_kernels File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf/infallible-numeric-casts (54c55dc) to 8c49c71 (merge-base) diff Run configurationrun benchmark cast_kernelsCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
nice |
`test_infallible_float32_to_float64_cast_matches_reference` compared the converted value bitwise, including for `f32::NAN`. Only NaN-ness is guaranteed across a float conversion; the sign and payload are unspecified, and Miri randomises them deliberately, so the assertion failed under Miri with 0xFFF8000000000000 against 0x7FF8000000000000. Assert `is_nan()` for NaN inputs and keep the bitwise comparison for every other value, which is what makes the signed zero case meaningful.
Rich-T-kid
suggested changes
Sep 2, 2026
Rich-T-kid
left a comment
Rich-T-kid
left a comment
Contributor
There was a problem hiding this comment.
Benchmark results look very good, I think we can trim the amount of added test & new benchmarks introduced in the PR though.
The primitive to primitive cast benchmarks were fixed at 512 elements, about 2 KiB, where the measurement is dominated by dispatch rather than by the per-element conversion the kernels control. Run them at 512, 1024, 8192 and 65536 instead. The 512 benchmark names are unchanged, so those rows stay comparable with historical runs, and the inputs keep the 10% null density `build_array` has always produced.
alexeigor
force-pushed
the
perf/infallible-numeric-casts
branch
from
September 2, 2026 18:41
7c6e927 to
1059e59
Compare
Addresses review feedback that the PR added more test and benchmark surface than it needs. - Drop the `infallible numeric cast i32 to i64` benchmark group. The size sweep over the existing primitive cast benchmarks now covers the same ground, and unlike a newly added group it can be compared against the merge base. - Reduce the end to end cast test from all 34 integer-source pairs to one per conversion class. The property test already covers every pair numerically; the end to end test exists to check length, validity and array offsets, which a representative sample exercises just as well. `Float32 -> Float64` folds into the same test rather than having one of its own. The predicate's correctness matrix and the negative control are unchanged.
Author
|
Thanks @Rich-T-kid — both points addressed. Benchmark sizes. Now Trimming. The PR goes from Benchmarks — removed the Tests — the end-to-end cast test went from all 34 integer-source pairs to one per conversion class (signed widening, signed to float, unsigned widening, unsigned to wider signed, unsigned to float), and The reasoning for keeping a sample rather than the matrix: the property test already covers all 35 pairs numerically, so re-enumerating them end to end adds no numeric coverage. The end-to-end test is there to check what I left two things intact deliberately: the predicate's correctness matrix, and the negative control asserting narrowing / unsigned-to-signed-of-the-same-width / signed-to-unsigned still return null under Also in this push: a fix for the Locally: Ready for another look when you have a moment. A re-run of |
Contributor
|
run benchmark cast_kernels |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/infallible-numeric-casts (935ffa7) to 8c49c71 (merge-base) diff Run configurationrun benchmark cast_kernelsBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench cast_kernels File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf/infallible-numeric-casts (935ffa7) to 8c49c71 (merge-base) diff Run configurationrun benchmark cast_kernelsCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Author
|
@Rich-T-kid please run below workflows |
Contributor
|
run benchmark cast_kernels |
Contributor
|
🤔 looking the this again Im not sure we really need this many new benchmarks either is there anyway to reduce this to just two benchmarks be type cast? this is really more of a personal nit. I'll try and take a second look at the PR this evening |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing perf/infallible-numeric-casts (935ffa7) to 8c49c71 (merge-base) diff Run configurationrun benchmark cast_kernelsBENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench cast_kernels File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing perf/infallible-numeric-casts (935ffa7) to 8c49c71 (merge-base) diff Run configurationrun benchmark cast_kernelsCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
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
4 participants
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.
Which issue does this PR close?
Rationale for this change
cast_numeric_arrayscurrently sends every numeric pair throughnum_cast, including total conversions such asInt32 -> Int64. The safe path rebuilds validity throughunary_opt; the unsafe path retains a fallible operation and walks valid indices. Both prevent the straight-line widening loop used byPrimitiveArray::unary.What changes are included in this PR?
num_castis total and agrees withAsPrimitive::as_, and route them throughunary.cast_with_options, under bothsafesettings and at several array offsets, for one pair per conversion class.Float16 conversions are intentionally left out of the fast path because they cannot use a primitive
ascast and need separate correctness reasoning. The pair set is deliberately conservative rather than exhaustive; because the match is fail-closed, anything left out simply keeps the checked path.Are these changes tested?
Yes.
cargo test -p arrow-cast --lib(381 passed),cargo fmt --all -- --check,cargo clippy -p arrow-cast --lib --tests -- -D warnings,cargo clippy -p arrow --bench cast_kernels --features test_utils -- -D warnings.Two layers of tests:
test_infallible_numeric_castsasserts the numeric property the fast path rests on: for every listed pair,num_cast::<FROM, TO>(v) == Some(v as TO)andAsPrimitive::as_agrees withas. This also pinsnum_traitsbehaviour that the fast path assumes; if a future version made any of these conversions fallible, the fast path would silently diverge from the checked path and this test is what would catch it.test_infallible_numeric_cast_fast_path_matches_referenceandtest_fallible_numeric_casts_remain_checkeddrive the publiccast_with_optionsentry point, so length, validity, array offsets and the untouched checked paths are covered as well. Since the pairs are already covered numerically above, this runs one pair per conversion class rather than repeating the matrix.Float32 -> Float64is compared bitwise, except for NaN, whose sign and payload are unspecified across a float conversion.Offline, the property was additionally verified exhaustively over every value of every 8-bit and 16-bit source (23 of the 35 pairs), over all 2^32
f32bit patterns forFloat32 -> Float64, and over edge values plus 2M random samples per pair for the 32-bit and 64-bit sources. Negative controls confirm the excluded pairs (i64 -> i32,u32 -> i32,i8 -> u8,i8 -> u16,u8 -> i8,u64 -> i64) really are not total.Benchmarks
The primitive to primitive cast benchmarks were fixed at 512 elements, about 2 KiB, where dispatch dominates. They now run at 512, 1024, 8192 and 65536. The
512names are unchanged, so those rows stay comparable with historical runs, and the inputs keep the 10% null densitybuild_arrayhas always produced.Ratios below are main / branch, so higher is faster. Apple arm64, Criterion with 2 s warm-up and 4 s measurement:
int32 -> float32int32 -> float64int32 -> int64float32 -> int32(checked)float64 -> float32(checked)float64 -> uint64(checked)int32 -> uint32(checked)int64 -> int32(checked)The gain peaks at 8192, DataFusion's default batch size:
int32 -> int64goes from 5.22 us to 707.7 ns. Every pair that stays on the checked path is flat within noise, which is the control for the change being confined to the listed pairs. The 65536 column is not listed above because it was added after these numbers were taken; a comparison run on the PR will produce it.The earlier run on this PR, on neutral hardware, showed the same shape at the one size it could compare: 4.06x, 3.88x and 3.35x for
int32 -> float32,int32 -> int64andint32 -> float64at 512.A DataFusion 55 reproduction over 9,994,240 rows improves
SUM(Int32)from 7.40 ms to 3.08 ms (2.40x), while theSUM(Int64)control stays at 2.14 ms.Are there any user-facing changes?
No API changes, and no change to any logical value produced by
cast.There is one observable difference at the buffer level.
PrimitiveArray::unaryapplies the conversion to every slot and reuses the input null buffer, so for the 35 fast-path pairs the values buffer underneath a null slot now holds the converted input value instead of the zero previously written byunary_opt/try_unary:Values under a null slot are undefined by the Arrow specification and this matches the other
unary-based kernels in arrow-rs, but it does change whatPrimitiveArray::values()returns at those positions and the bytes an IPC writer emits for them. Noted here in case anyone compares buffers byte for byte, or relied on the cast to mask data under nulls.