alexeigorwork opened a new issue, #10953:
URL: https://github.com/apache/arrow-rs/issues/10953
## Is your feature request related to a problem or challenge?
All 110 numeric↔numeric arms of `cast_with_options` route through
`cast_numeric_arrays` (`arrow-cast/src/cast/mod.rs:2506`), which performs a
per-element
checked `num_traits::cast` — including for conversions that are total, such
as
`Int32 → Int64`. Neither `CastOptions::safe` setting avoids this:
```rust
if cast_options.safe {
Ok(Arc::new(numeric_cast::<FROM, TO>(from.as_primitive::<FROM>())))
// unary_opt
} else {
Ok(Arc::new(try_numeric_cast::<FROM, TO>(from.as_primitive::<FROM>())?))
// try_unary
}
```
* `safe: true` → `unary_opt`, which allocates and **rebuilds a new validity
bitmap**
from each element's `Option`. For a widening cast the result is provably
identical to
the input's bitmap.
* `safe: false` → `try_unary`, which clones the null buffer but still calls
a fallible
`op` per element and iterates through `nulls.try_for_each_valid_idx` — an
index-by-index walk rather than a linear scan.
For `Int32 → Int64` the range check can never fire, the rebuilt bitmap is
redundant, and
the per-element `Option`/`Result` prevents the compiler from emitting a
widening SIMD
instruction. The docs on the adjacent `try_unary_mut` already state the
consequence:
> Note: LLVM is currently unable to effectively vectorize fallible operations
### Measurements
`i32 → i64`, Criterion medians. Apple M5 Pro, rustc 1.98.0 stable, `lto =
"fat"`,
`codegen-units = 1`, mimalloc.
| n | nulls | `cast` (safe) | `cast` (safe=false) | `unary(\|v\| v as i64)`
| gap |
|---|---|---|---|---|---|
| 1 000 | 0% | 0.205 µs | 0.202 µs | **0.101 µs** | 2.0× |
| 100 000 | 0% | 10.8 µs | 11.7 µs | **9.07 µs** | 1.2× |
| 10 000 000 | 0% | 1263.9 µs | 1236.0 µs | **940.9 µs** | 1.3× |
| 1 000 | 10% | 0.563 µs | 0.534 µs | **0.100 µs** | 5.3× |
| 100 000 | 10% | 53.4 µs | 52.1 µs | **8.57 µs** | 6.1× |
| 10 000 000 | 10% | 5353.9 µs | 5329.5 µs | **934.3 µs** | 5.7× |
The cost is concentrated in the null path: stock `cast` gets **4.3× slower**
once a
column has any nulls (1236 → 5330 µs at 10M), while the `unary` version is
flat
(941 → 934 µs). The ratio holds from n=1 000 to n=10 000 000, including at
n=8 192.
### Downstream impact
DataFusion 55.0.0 pins arrow 59.3.0 — the version measured above — so
DataFusion can be
built against the modified `arrow-cast` and the same query run both ways. The
substitution was confirmed present in the resolved dependency graph before
measuring.
DataFusion's `sum` signature coerces `Int8/Int16/Int32 → Int64` explicitly,
so
`SUM(int32_col)` casts the whole column.
Two queries over identical data (1220 × 8192 = 9,994,240 rows, 10% nulls,
`target_partitions = 1`, DataFusion's default 8192-row batches). The only
difference
between them is the cast:
| query | stock | patched | speedup |
|---|---|---|---|
| `SELECT SUM(c_i32) FROM t` | 7.426 ms | **3.349 ms** | **2.22×** |
| `SELECT SUM(c_i64) FROM t` *(control)* | 2.097 ms | 2.205 ms | 0.95× |
The cast component drops from **5.329 ms to 1.144 ms (4.66×)**, taking it
from **72% to
34%** of query time. The unchanged control shows nothing else moved.
Both arms were run back to back in one session, using exactly the predicate
shown below.
An earlier run of a wider predicate (one that also covered `Float16`
targets) gave
2.24× / 4.60× — the same result within noise, as expected, since those arms
are not
reachable from an `Int32 → Int64` cast.
DataFusion reaches this path from more than `SUM`: comparison against an
`Int64`
literal, join keys of differing width, `UNION` branch unification,
`CASE`/`IN` result
types, and schema unification across files with differing types.
## Describe the solution you'd like
A fast path in `cast_numeric_arrays` for conversions where `num_cast` is
total.
`num_traits::cast::AsPrimitive` is already imported in the module, and since
all 110
call sites use concrete types, adding the bound needs no call-site changes:
```rust
fn cast_numeric_arrays<FROM, TO>(from: &dyn Array, cast_options:
&CastOptions)
-> Result<ArrayRef, ArrowError>
where
FROM: ArrowPrimitiveType,
TO: ArrowPrimitiveType,
FROM::Native: NumCast + AsPrimitive<TO::Native>,
TO::Native: NumCast,
{
if is_infallible_numeric_cast(&FROM::DATA_TYPE, &TO::DATA_TYPE) {
return Ok(Arc::new(
from.as_primitive::<FROM>().unary::<_, TO>(|v| v.as_()),
));
}
// ... existing safe / try paths unchanged
}
```
`unary` clones the input `NullBuffer` (a refcount bump — no scan, no
allocation) and
maps the closure over the values, which vectorizes.
### The predicate
```rust
/// True when `num_cast::<FROM, TO>` is total — it can never return `None`,
so the
/// per-element check can never fire and the conversion is a plain `as`.
fn is_infallible_numeric_cast(from: &DataType, to: &DataType) -> bool {
use DataType::*;
matches!(
(from, to),
(Int8, Int16 | Int32 | Int64 | Float32 | Float64)
| (Int16, Int32 | Int64 | Float32 | Float64)
| (Int32, Int64 | Float32 | Float64)
| (Int64, Float32 | Float64)
| (UInt8, UInt16 | UInt32 | UInt64 | Int16 | Int32 | Int64 |
Float32 | Float64)
| (UInt16, UInt32 | UInt64 | Int32 | Int64 | Float32 | Float64)
| (UInt32, UInt64 | Int64 | Float32 | Float64)
| (UInt64, Float32 | Float64)
| (Float32, Float64)
)
}
```
**It costs nothing at runtime.** It is called with `FROM::DATA_TYPE` /
`TO::DATA_TYPE`,
which are associated constants rather than values read from the array.
`cast_numeric_arrays` is monomorphised per concrete pair, so the `Int32 →
Int64`
instantiation sees `is_infallible_numeric_cast(&Int32, &Int64)`, folds it to
`true`, and
drops the branch and the checked path entirely. It reads like a runtime
match; it is not
one.
**Why each group is total:**
| group | reasoning |
|---|---|
| integer widening within a signedness (`i8→i16→i32→i64`, `u8→u16→u32→u64`)
| the target range strictly contains the source range |
| unsigned → strictly wider signed (`u8→i16`, `u16→i32`, `u32→i64`) | `i64`
has 63 magnitude bits, so every `u32` fits. Note `u32→i32` is absent: same
width, so it can overflow |
| integer → float | `num_cast` never fails here, it rounds. Lossy above the
mantissa width, but `as` rounds *identically*, so the substitution is
value-preserving. Totality is the property that matters, not exactness |
| `f32 → f64` | every `f32` is exactly representable, NaN and infinities
included |
**Every narrowing conversion is excluded**, where the check is load-bearing.
**The predicate is fail-safe by construction.** `matches!` yields `false`
for anything
unlisted, and `false` means "use the existing checked path". An omission
therefore costs
performance, never correctness. A wrong *addition* would be a correctness
bug, which is
what the negative control below guards.
**On `Float16`:** the version I benchmarked also included `Float16` targets.
They are
omitted above because the correctness test cannot cover them — `v as f16` is
not a
primitive cast, so the direct numeric proof does not apply, and their safety
would rest
on `half::f16`'s `NumCast` impl agreeing with saturation on values like
`i64::MAX`.
That is plausible but unproven, and the set above is already sufficient to
produce the
DataFusion result. `Float16` can be added later with a purpose-built test.
### Correctness
The substitution is safe iff `num_cast::<FROM, TO>(v) == Some(v as TO)` for
every value
of `FROM`. That is a claim about the numbers, not about arrow, so it is
worth testing
directly rather than by comparing two arrow code paths:
```rust
for v in samples!($from) { // MIN, MAX, 0, 1 + a
65-point spread
assert_eq!(num_cast::<$from, $to>(v), Some(v as $to));
}
```
This passes for every pair in the proposed set, with a negative control
asserting that
the narrowing pairs genuinely are *not* infallible (`num_cast::<i64,
i32>(i64::MAX)` is
`None`, whereas `i64::MAX as i32` is `-1` — which is exactly why those keep
the check).
The patched build also passes all **360 existing tests in `arrow-cast`**.
I am happy to open a PR if the approach looks right.
## Describe alternatives you've considered
* **`CastOptions { safe: false }`** — does not help. It swaps `unary_opt` for
`try_unary`, keeping the per-element check; measured within 2% of the safe
path.
DataFusion already uses `safe: false` throughout
(`datafusion-physical-expr/src/expressions/cast.rs:37`) and so cannot opt
out.
* **Callers using `unary` directly** — works, and is what I used to obtain
the numbers,
but it is not reachable through the public `cast` API. It also pushes the
"is this conversion infallible?" decision onto every caller, which is
exactly the
knowledge `arrow-cast` already has.
* **Leaving it to downstream** — the two closest prior issues (#7097, #7055)
were
resolved in the Parquet reader rather than in `cast_numeric_arrays`, so
this function
has already been worked around once from another direction.
* **Expressing infallibility in the type system** rather than as a matched
pair — an
associated `const INFALLIBLE: bool` on a `CastFrom`-style trait would make
it a
compile-time fact instead of relying on the match being folded, and would
make an
unhandled pair a compile error rather than a silent slow path. It is more
invasive,
and it duplicates knowledge `num_traits` already encodes, but if
maintainers prefer
that shape I am happy to write it that way. The matched-pair version above
was chosen
because it needs no new trait and no call-site changes.
## Additional context
### Prior art in other Arrow implementations
* **polars-compute** encodes the distinction in its dispatch table:
`primitive_to_primitive_dyn` branches on `options.wrapped`, choosing
`primitive_as_primitive` (`unary` + `AsPrimitive::as_`) for 48 arms tagged
`as_options`, and the checked `primitive_to_primitive` otherwise.
* **Arrow C++** already skips the check for a widening cast:
`CastOptions::Unsafe()`
measured identical to the safe default (23.9 vs 23.6 µs at 100 k), i.e.
the safe path
was not doing extra work to begin with.
### Why the existing benchmarks do not surface this
`arrow/benches/cast_kernels.rs` builds its numeric inputs with
`build_array::<T>(512)`.
`cast int32 to int64 512` exists, but 512 × 4 bytes = 2 KB measures dispatch
rather than
the per-element conversion. There is also no `unary` reference arm in the
suite —
grepping `unary|try_unary|unary_opt` across all 63 bench files in the
workspace matches
only `array_iter.rs`, and not as a kernel comparison.
Two suggestions, independent of whether the fix lands:
* raise the numeric cast sizes in `cast_kernels.rs` to at least 64 Ki and
sweep null
density — the null path is where the cost concentrates
* add `unary` as a reference arm so the cost of the checked conversion is
visible
### Related issues (searched; none appear to duplicate this)
* **#1918 — "Replace checked casts with `as` for performance"** (closed
2022). Same
words, different subject: `try_into().unwrap()` on offsets and indices in
array-data
internals, not `num_traits::cast` in the cast kernel.
* **#7097 / #7055 — Parquet performance reading int8/int16** (closed).
*Narrowing*
casts (32 bits down to 8/16), where the range check is load-bearing.
Opposite
direction from this report.
* **#10726 — "support unsafe casting with masked null values"** (open PR).
Touches
`safe: false` behaviour for string, list, struct and map casts; does not
modify
`cast_numeric_arrays`.
* **#9789 — "Remove redundant benchmarks in `cast_kernels`"** (merged).
Pruned
redundant decimal cases to cut benchmark runtime; noted only because it
bears on the
benchmark suggestions above.
### Environment
* arrow-rs 59.3.0, rustc 1.98.0 stable, `aarch64-apple-darwin`
* Apple M5 Pro (6P + 12E), 48 GB, macOS Darwin 25.6.0
* Criterion 1 s warm-up / 3 s measurement; the DataFusion arm built with
`CARGO_PROFILE_BENCH_LTO=thin`, applied identically to both sides
Numbers come from a laptop with no core pinning, so absolute values are
directional;
the ratios reproduced across two independent full runs.
--
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]
