From 5f75518c969bc8834fbf4f129545e899826017b3 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Mon, 5 Oct 2026 13:30:01 +0100 Subject: [PATCH 1/3] perf: make checked lane chunking opt-in Signed-off-by: Connor Tsui --- .../scalar_fn/fns/binary/numeric/primitive.rs | 13 +++ .../src/scalar_fn/fns/binary/numeric/row.rs | 22 ++-- .../src/scalar_fn/fns/binary/numeric/tests.rs | 56 +++++++--- .../src/scalar_fn/unstable/row/execute/mod.rs | 2 + .../scalar_fn/unstable/row/execute/owned.rs | 38 +++++++ .../scalar_fn/unstable/row/execute/retry.rs | 80 ++++++++++++++ .../scalar_fn/unstable/row/visitor/execute.rs | 29 +++++ .../scalar_fn/unstable/row/visitor/retry.rs | 29 +++++ .../unstable/row/visitor/row_visitor.rs | 41 +++++++ vortex-compute/src/lane_kernels/map_into.rs | 100 ++++++++++++++++++ vortex-compute/src/lane_kernels/mod.rs | 3 +- 11 files changed, 387 insertions(+), 26 deletions(-) diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs index 7cbe88c7c8e..6ac3deb6fb4 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs @@ -6,6 +6,7 @@ use std::ops::BitOrAssign; use crate::dtype::NativePType; +use crate::dtype::PType; use crate::dtype::half::f16; /// Checked addition, failing on integer overflow. @@ -37,6 +38,11 @@ pub(super) trait CheckedPrimitiveOp: 'static + Sized { /// How this operation reports a failing row. See [`Failure`]. type Fail: Failure; + /// Select the dense reduction loop for this operation and primitive type. + fn use_chunked_loop() -> bool { + false + } + /// The result of this operation, paired with evidence of whether the row failed. fn apply(lhs: T, rhs: T) -> (T, Self::Fail); } @@ -68,6 +74,13 @@ impl CheckedPrimitiveOp for CheckedMul { type Fail = T::MulFailure; + #[inline] + fn use_chunked_loop() -> bool { + // Fixed-size reductions help LLVM combine signed i16/i32 overflow checks. Other widths + // and unsigned operations can lose efficient instruction selection with the same loop. + matches!(T::PTYPE, PType::I16 | PType::I32) + } + #[inline] fn apply(lhs: T, rhs: T) -> (T, T::MulFailure) { (lhs.mul_value(rhs), lhs.mul_failure(rhs)) diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs index ff7fbe7ab81..d267af12ae8 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs @@ -91,16 +91,20 @@ where Op: CheckedPrimitiveOp, V: RowVisitor, { - visitor.visit_deferred::<(T, T), T, Op::Fail>( - |(lhs, rhs)| Op::apply(lhs, rhs), - |failure| { - if failure != ::default() { - return Err(numeric_error(Op::ERROR)); - } + let apply = |(lhs, rhs)| Op::apply(lhs, rhs); + let finish_failure = |failure| { + if failure != ::default() { + return Err(numeric_error(Op::ERROR)); + } - Ok(()) - }, - ) + Ok(()) + }; + + if Op::use_chunked_loop() { + visitor.visit_deferred_chunked::<(T, T), T, Op::Fail>(apply, finish_failure) + } else { + visitor.visit_deferred::<(T, T), T, Op::Fail>(apply, finish_failure) + } } fn visit_div(visitor: V) -> VortexResult diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/tests.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/tests.rs index 3813c8612b3..74b9cf285a8 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/tests.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/tests.rs @@ -296,30 +296,54 @@ fn test_multiply_overflow_on_null_lane_ignored( Ok(()) } -/// An overflow late in the batch must still be reported, unless its row is null. +/// Failures at a chunk boundary or in the remainder must survive reduction unless the row is null. #[rstest] -#[case::reported(true)] -#[case::suppressed_by_null(false)] -fn test_multiply_overflow_survives_batch_reduction( - #[case] lane_is_valid: bool, +#[case::unsigned_32(u32::MAX, 3, 9)] +#[case::signed_16(i16::MAX, 3, 9)] +#[case::signed_32(i32::MAX, 3, 9)] +fn test_multiply_overflow_survives_batch_reduction>( + #[case] overflowing: T, + #[case] rhs: T, + #[case] product: T, + #[values(false, true)] lane_is_valid: bool, + #[values(63, 64, 128)] overflow_at: usize, + #[values(false, true)] constant_rhs: bool, ) -> VortexResult<()> { - const LEN: u32 = 1000; - const OVERFLOW_AT: u32 = 700; + const LEN: usize = 129; let mut ctx = array_session().create_execution_ctx(); - let mut lhs: Vec = (0..LEN).map(|i| i % 100 + 1).collect(); - lhs[OVERFLOW_AT as usize] = u32::MAX; - - let validity = Validity::from_iter((0..LEN).map(|i| i != OVERFLOW_AT || lane_is_valid)); + let mut lhs = vec![rhs; LEN]; + lhs[overflow_at] = overflowing; + let validity = if lane_is_valid { + Validity::NonNullable + } else { + Validity::from_iter((0..LEN).map(|index| index != overflow_at)) + }; + let rhs = if constant_rhs { + ConstantArray::new(rhs, LEN).into_array() + } else { + PrimitiveArray::from_iter(vec![rhs; LEN]).into_array() + }; let result = PrimitiveArray::new(Buffer::copy_from(&lhs), validity) .into_array() - .binary( - PrimitiveArray::from_iter(vec![3u32; LEN as usize]).into_array(), - Operator::Mul, - )? + .binary(rhs, Operator::Mul)? .execute::(&mut ctx); - assert_eq!(result.is_err(), lane_is_valid); + if lane_is_valid { + assert!( + result.is_err(), + "overflow at row {overflow_at} must be reported" + ); + return Ok(()); + } + + assert_arrays_eq!( + result?.0.into_array(), + PrimitiveArray::from_option_iter( + (0..LEN).map(|index| (index != overflow_at).then_some(product)), + ), + &mut ctx + ); Ok(()) } diff --git a/vortex-array/src/scalar_fn/unstable/row/execute/mod.rs b/vortex-array/src/scalar_fn/unstable/row/execute/mod.rs index c9e17da1d09..dc0a1b82e4c 100644 --- a/vortex-array/src/scalar_fn/unstable/row/execute/mod.rs +++ b/vortex-array/src/scalar_fn/unstable/row/execute/mod.rs @@ -8,6 +8,7 @@ mod owned; pub(super) use owned::execute_owned; +pub(super) use owned::execute_owned_chunked; pub(super) use owned::execute_owned_filtered; pub(super) use owned::execute_owned_infallible; pub(super) use owned::execute_owned_infallible_filtered; @@ -22,6 +23,7 @@ pub(super) use packed_bool::execute_owned_infallible_bool; mod retry; pub(super) use retry::DenseAttempt; pub(super) use retry::execute_owned_dense_attempt; +pub(super) use retry::execute_owned_dense_attempt_chunked; mod sink; pub(super) use sink::execute_sink; diff --git a/vortex-array/src/scalar_fn/unstable/row/execute/owned.rs b/vortex-array/src/scalar_fn/unstable/row/execute/owned.rs index b73b255223c..ab889bf34c6 100644 --- a/vortex-array/src/scalar_fn/unstable/row/execute/owned.rs +++ b/vortex-array/src/scalar_fn/unstable/row/execute/owned.rs @@ -300,3 +300,41 @@ where // SAFETY: normal completion of `map_checked_into` initializes every output slot. Ok(unsafe { values.finish(row_count, ctx.allocator()) }) } + +/// Decode every input column, then reduce failure evidence within fixed-size chunks. +/// +/// This stays separate from [`execute_owned`] to preserve the default loop's codegen context. +pub(crate) fn execute_owned_chunked( + args: &dyn ExecutionArgs, + ctx: &mut ExecutionCtx, + prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, + apply: impl Fn(&Prepared, Args::Elems<'_>) -> (Out, Fail), + finish_failure: impl FnOnce(Fail) -> VortexResult<()>, +) -> VortexResult +where + Args: IndexedElementTuple, + Out: OutputElement, + Fail: FailureEvidence, +{ + // Errors and unwinds abandon partially initialized slots. The assertion ensures that no + // initialized value requires a destructor to run. + const { assert_owned_output_needs_no_drop::() }; + + let columns = Args::decode(args, ctx)?; + let prepared = prepare(Args::const_values(&columns)); + + let row_count = args.row_count(); + let mut values = Out::with_capacity(row_count, ctx.allocator()); + let output = &mut values.slots()[..row_count]; + + let Some(source) = decoded_source::(&columns, row_count) else { + vortex_bail!("a decoded row input does not address exactly {row_count} rows"); + }; + let failure = source.map_checked_chunked_into(output, |elements| apply(&prepared, elements)); + + // Defer rich error construction until after the row loop. + finish_failure(failure)?; + + // SAFETY: normal completion of `map_checked_chunked_into` initializes every output slot. + Ok(unsafe { values.finish(row_count, ctx.allocator()) }) +} diff --git a/vortex-array/src/scalar_fn/unstable/row/execute/retry.rs b/vortex-array/src/scalar_fn/unstable/row/execute/retry.rs index ebf67461e6c..7523f45dd12 100644 --- a/vortex-array/src/scalar_fn/unstable/row/execute/retry.rs +++ b/vortex-array/src/scalar_fn/unstable/row/execute/retry.rs @@ -111,3 +111,83 @@ where Err(error) => Ok(DenseAttempt::DeferredError(error)), } } + +/// Attempt dense execution with chunked failure reduction for non-constant inputs. +/// +/// This follows the error contract of [`execute_owned_dense_attempt`]. +pub(in crate::scalar_fn::unstable::row) fn execute_owned_dense_attempt_chunked< + Args, + Out, + Prepared, + Fail, +>( + args: &dyn ExecutionArgs, + ctx: &mut ExecutionCtx, + prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, + apply: impl Fn(&Prepared, Args::Elems<'_>) -> (Out, Fail), + finish_failure: impl FnOnce(Fail) -> VortexResult<()>, +) -> VortexResult +where + Args: IndexedElementTuple, + Out: OutputElement, + Fail: FailureEvidence, +{ + // Errors and unwinds abandon partially initialized slots. The assertion ensures that no + // initialized value requires a destructor to run. + const { assert_owned_output_needs_no_drop::() }; + + // Keep this dense row loop separate from `execute_owned_chunked`. Factoring shared state into a + // helper changes LLVM's optimized dense kernel even when the helper is inlined. + let columns = Args::decode(args, ctx)?; + let prepared = prepare(Args::const_values(&columns)); + + let row_count = args.row_count(); + let mut values = Out::with_capacity(row_count, ctx.allocator()); + let output = &mut values.slots()[..row_count]; + + let failure_evidence = if let Some(views) = Args::views_if_no_consts(&columns) { + // Keep this validation beside the views so LLVM sees their common length here. + vortex_ensure!( + Args::view_lens_match(&views, row_count), + "a decoded row input does not address exactly {row_count} rows", + ); + + // SAFETY: `view_lens_match` checked that these exact retained views address `row_count` + // rows. + let source = unsafe { Args::indexed_source(views, row_count) }; + + source.map_checked_chunked_into(output, |elements| apply(&prepared, elements)) + } else { + // Keep this proof branch-local. Shared validation prevents LLVM from specializing this + // loop for each batch-constant arrangement, leaving it scalar under multiple CGUs without + // LTO. + vortex_ensure!( + Args::decoded_lens_match(&columns, row_count), + "a decoded row input does not address exactly {row_count} rows", + ); + + let mut accumulated_failure = Fail::default(); + + // Iterate over `output` directly. A `0..row_count` range reuses the address-taken value + // from the validation error formatter and retains an output bounds check. + for (index, slot) in output.iter_mut().enumerate() { + // LLVM unswitches the batch-constant checks in `Args::get` before vectorizing the loop. + let (value, row_failure) = apply(&prepared, Args::get(&columns, index)); + + slot.write(value); + accumulated_failure |= row_failure; + } + + accumulated_failure + }; + + match finish_failure(failure_evidence) { + Ok(()) => { + // SAFETY: normal completion of either path initializes every output slot. + let output = unsafe { values.finish(row_count, ctx.allocator()) }; + + Ok(DenseAttempt::Values(output)) + } + Err(error) => Ok(DenseAttempt::DeferredError(error)), + } +} diff --git a/vortex-array/src/scalar_fn/unstable/row/visitor/execute.rs b/vortex-array/src/scalar_fn/unstable/row/visitor/execute.rs index b5539d5b29e..e6c3a76ade8 100644 --- a/vortex-array/src/scalar_fn/unstable/row/visitor/execute.rs +++ b/vortex-array/src/scalar_fn/unstable/row/visitor/execute.rs @@ -35,6 +35,7 @@ use crate::scalar_fn::unstable::row::RowFn; use crate::scalar_fn::unstable::row::SinkResult; use crate::scalar_fn::unstable::row::execute::execute_owned; use crate::scalar_fn::unstable::row::execute::execute_owned_bool; +use crate::scalar_fn::unstable::row::execute::execute_owned_chunked; use crate::scalar_fn::unstable::row::execute::execute_owned_filtered; use crate::scalar_fn::unstable::row::execute::execute_owned_infallible; use crate::scalar_fn::unstable::row::execute::execute_owned_infallible_bool; @@ -184,6 +185,34 @@ impl RowVisitor for ExecuteRows<'_, '_, F> { ) } + fn visit_prepared_deferred_chunked( + self, + prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, + apply: impl Fn(&Prepared, Args::Elems<'_>) -> (Out, Fail), + finish_failure: impl FnOnce(Fail) -> VortexResult<()>, + ) -> VortexResult + where + Args: IndexedElementTuple, + Out: OutputElement, + Fail: FailureEvidence, + { + const { assert_deferred_visit_contract::() }; + let visited = BatchPlan::new( + validate_owned_visit::(self.dtypes)?, + self.output_dtype, + RowPolicy::for_deferred_output::(), + )?; + self.plan.ensure_reproduced_by(&visited)?; + + execute_owned_chunked::( + self.args, + self.ctx, + prepare, + apply, + finish_failure, + ) + } + fn visit_prepared_deferred_bool( self, prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, diff --git a/vortex-array/src/scalar_fn/unstable/row/visitor/retry.rs b/vortex-array/src/scalar_fn/unstable/row/visitor/retry.rs index 1f045a97f4f..c2435f5ea44 100644 --- a/vortex-array/src/scalar_fn/unstable/row/visitor/retry.rs +++ b/vortex-array/src/scalar_fn/unstable/row/visitor/retry.rs @@ -33,6 +33,7 @@ use crate::scalar_fn::unstable::row::batch::BorrowedRowFnArgs; use crate::scalar_fn::unstable::row::execute::DenseAttempt; use crate::scalar_fn::unstable::row::execute::execute_bool_dense_attempt; use crate::scalar_fn::unstable::row::execute::execute_owned_dense_attempt; +use crate::scalar_fn::unstable::row::execute::execute_owned_dense_attempt_chunked; use crate::scalar_fn::unstable::row::execute::execute_owned_infallible; use crate::scalar_fn::unstable::row::execute::execute_sink; @@ -154,6 +155,34 @@ impl RowVisitor for ExecuteDenseWithRetry<'_, '_, '_, F> { ) } + fn visit_prepared_deferred_chunked( + self, + prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, + apply: impl Fn(&Prepared, Args::Elems<'_>) -> (Out, Fail), + finish_failure: impl FnOnce(Fail) -> VortexResult<()>, + ) -> VortexResult + where + Args: IndexedElementTuple, + Out: OutputElement, + Fail: FailureEvidence, + { + const { assert_deferred_visit_contract::() }; + let visited = BatchPlan::new( + validate_owned_visit::(self.args.dtypes())?, + self.output_dtype, + RowPolicy::for_deferred_output::(), + )?; + self.args.plan().ensure_reproduced_by(&visited)?; + + execute_owned_dense_attempt_chunked::( + self.args, + self.ctx, + prepare, + apply, + finish_failure, + ) + } + fn visit_prepared_deferred_bool( self, prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, diff --git a/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs b/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs index 952d0b478ea..8fb3275affe 100644 --- a/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs +++ b/vortex-array/src/scalar_fn/unstable/row/visitor/row_visitor.rs @@ -313,6 +313,29 @@ pub trait RowVisitor: private::Sealed + Sized { ) } + /// Request chunked failure reduction for dense owned execution. + /// + /// This has the requirements and error handling of [`visit_deferred`](Self::visit_deferred). + /// Failure reduction must be associative, with [`Default`] as its identity. Dense execution + /// can reduce evidence within fixed-size chunks, while valid-row retries use their usual loops. + /// Choose this variant only with benchmark evidence for the dispatched computation. + fn visit_deferred_chunked( + self, + apply: impl Fn(Args::Elems<'_>) -> (Out, Fail), + finish_failure: impl FnOnce(Fail) -> VortexResult<()>, + ) -> VortexResult + where + Args: IndexedElementTuple, + Out: OutputElement, + Fail: FailureEvidence, + { + self.visit_prepared_deferred_chunked::( + |_| (), + move |&(), args| apply(args), + finish_failure, + ) + } + /// Visit a deferred row computation whose Boolean output is packed during evaluation. /// /// This has the same requirements and failure handling as @@ -384,6 +407,24 @@ pub trait RowVisitor: private::Sealed + Sized { Out: OutputElement, Fail: FailureEvidence; + /// The prepared form of [`visit_deferred_chunked`](Self::visit_deferred_chunked). + /// + /// Planning and valid-row execution can use the ordinary deferred visit. Dense owned + /// execution overrides this method to select the chunked kernel. + fn visit_prepared_deferred_chunked( + self, + prepare: impl FnOnce(Args::ConstElems<'_>) -> Prepared, + apply: impl Fn(&Prepared, Args::Elems<'_>) -> (Out, Fail), + finish_failure: impl FnOnce(Fail) -> VortexResult<()>, + ) -> VortexResult + where + Args: IndexedElementTuple, + Out: OutputElement, + Fail: FailureEvidence, + { + self.visit_prepared_deferred::(prepare, apply, finish_failure) + } + /// The prepared form of [`visit_deferred_bool`](Self::visit_deferred_bool). fn visit_prepared_deferred_bool( self, diff --git a/vortex-compute/src/lane_kernels/map_into.rs b/vortex-compute/src/lane_kernels/map_into.rs index 326b1bb2034..12bebfab8b3 100644 --- a/vortex-compute/src/lane_kernels/map_into.rs +++ b/vortex-compute/src/lane_kernels/map_into.rs @@ -355,6 +355,75 @@ pub trait IndexedSourceExt: IndexedSource + Sized { failed } + /// Write every lane and reduce failure evidence within fixed-size chunks. + /// + /// This has the value, validity, and output-length requirements of + /// [`map_checked_into`](Self::map_checked_into). Failure reduction must be associative, with + /// [`Default`] as its identity, because each chunk starts with fresh failure evidence. + /// + /// Fixed trip counts let LLVM combine checks across rows, but can also prevent efficient + /// instruction selection. Use this variant only when benchmarks favor it for the callback. + #[inline] + fn map_checked_chunked_into( + self, + out: &mut [MaybeUninit], + apply: Apply, + ) -> Fail + where + Fail: Copy + Default + BitOrAssign, + Apply: Fn(Self::Item) -> (R, Fail), + { + const { + assert!( + size_of::() <= size_of::(), + "failure evidence must be no wider than the value, or it bounds the vector width" + ) + }; + + #[allow(clippy::inline_always)] + #[inline(always)] + fn chunk( + values: &S, + out: &mut [MaybeUninit], + apply: &Apply, + base: usize, + count: usize, + ) -> Fail + where + S: IndexedSource, + Fail: Copy + Default + BitOrAssign, + Apply: Fn(S::Item) -> (R, Fail), + { + let mut failed = Fail::default(); + for offset in 0..count { + let idx = base + offset; + // SAFETY: the caller proves base + count <= values.len() == out.len(). + let value = unsafe { values.get_unchecked(idx) }; + let (result, failure) = apply(value); + failed |= failure; + // SAFETY: the same chunk bounds prove idx < out.len(). + unsafe { out.get_unchecked_mut(idx).write(result) }; + } + failed + } + + let values = self; + let len = values.len(); + assert_eq!(out.len(), len, "out must have the same length as values"); + + // Fixed trip counts let LLVM unroll checked loops independently of the surrounding code. + let chunks_count = len / CHUNK_LEN; + let remainder = len % CHUNK_LEN; + let mut failed = Fail::default(); + for chunk_idx in 0..chunks_count { + failed |= chunk(&values, out, &apply, chunk_idx * CHUNK_LEN, CHUNK_LEN); + } + if remainder != 0 { + failed |= chunk(&values, out, &apply, chunks_count * CHUNK_LEN, remainder); + } + failed + } + /// Fallible map with **no validity awareness at all** — every `None` returned /// by the closure is treated as a failure, even at null lanes. /// @@ -752,6 +821,37 @@ mod tests { assert_eq!(write_t(out)[76], 76); } + #[test] + fn map_checked_chunked_into_reduces_all_chunks_and_initializes_every_lane() { + for len in [0usize, 1, 63, 64, 65, 127, 128, 129] { + let values: Vec = (0..len as u32).collect(); + let mut out = vec![MaybeUninit::::uninit(); len]; + let failed = values + .as_slice() + .map_checked_chunked_into(&mut out, |value| { + let failure = if value % 64 == 0 { + 1u32 << (value / 64) + } else { + 0 + }; + (value + 1, failure) + }); + + assert_eq!(failed, (1u32 << len.div_ceil(64)) - 1, "length {len}"); + assert_eq!(write_t(out), (1..=len as u32).collect::>()); + } + } + + #[test] + #[should_panic(expected = "out must have the same length as values")] + fn map_checked_chunked_into_rejects_different_output_length() { + let values = [1u32; 64]; + let mut out = [MaybeUninit::::uninit(); 63]; + values + .as_slice() + .map_checked_chunked_into(&mut out, |value| (value, false)); + } + #[test] fn map_bits_into_packs_full_and_remainder_words() { let values: Vec = (0..130).collect(); diff --git a/vortex-compute/src/lane_kernels/mod.rs b/vortex-compute/src/lane_kernels/mod.rs index 5fd464f6a85..e2462022dd8 100644 --- a/vortex-compute/src/lane_kernels/mod.rs +++ b/vortex-compute/src/lane_kernels/mod.rs @@ -35,7 +35,8 @@ pub use source::IndexedSource; pub use source::LaneZip; /// Loop-tiling chunk length for the **no-mask** kernels ([`IndexedSourceExt::map_into`], -/// [`IndexedSourceExt::try_map_into`], [`IndexedSinkExt::map_into_in_place`]). +/// [`IndexedSourceExt::try_map_into`], [`IndexedSourceExt::map_checked_chunked_into`], +/// [`IndexedSinkExt::map_into_in_place`]). /// /// This is a pure tuning knob: those kernels split the lane range into /// `len / CHUNK_LEN` full chunks plus a remainder, so any value yields correct From 10a8626abb952cbcfbe117cf7f63fd91bc55a62c Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Mon, 5 Oct 2026 13:40:51 +0100 Subject: [PATCH 2/3] fix: refresh transitive asyncband lockfile references Signed-off-by: Connor Tsui --- Cargo.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index db3052e5132..0a35c84a451 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7624,7 +7624,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "009a43e88f0f7b3b3c2c9de5384a0ac2cef8f33996b16f9e6738e24661618775" dependencies = [ "async-trait", - "asyncband 0.7.2", + "asyncband 0.7.3", "bytes", "chrono", "futures", @@ -7726,7 +7726,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ddff5c648dcb3922c2bec71e0d26e881ddf9c367cb7c9a59ed7f2d39a4f38218" dependencies = [ "anyhow", - "asyncband 0.7.2", + "asyncband 0.7.3", "base64 0.23.1", "bytes", "futures", @@ -7765,7 +7765,7 @@ version = "0.59.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b454e68e5f6b331bd079a2ee02ebe05078fc0ddaae6822767ef54ab3c2ac0aa8" dependencies = [ - "asyncband 0.7.2", + "asyncband 0.7.3", "futures", "http", "opendal-core 0.59.3", From b6ff4dc0e76240ae0baa737cc467de669bd81272 Mon Sep 17 00:00:00 2001 From: Connor Tsui Date: Mon, 5 Oct 2026 14:20:39 +0100 Subject: [PATCH 3/3] perf: preserve default checked visitor code generation Signed-off-by: Connor Tsui --- .../scalar_fn/fns/binary/numeric/primitive.rs | 13 ----- .../src/scalar_fn/fns/binary/numeric/row.rs | 56 ++++++++++++++----- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs index 6ac3deb6fb4..7cbe88c7c8e 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/primitive.rs @@ -6,7 +6,6 @@ use std::ops::BitOrAssign; use crate::dtype::NativePType; -use crate::dtype::PType; use crate::dtype::half::f16; /// Checked addition, failing on integer overflow. @@ -38,11 +37,6 @@ pub(super) trait CheckedPrimitiveOp: 'static + Sized { /// How this operation reports a failing row. See [`Failure`]. type Fail: Failure; - /// Select the dense reduction loop for this operation and primitive type. - fn use_chunked_loop() -> bool { - false - } - /// The result of this operation, paired with evidence of whether the row failed. fn apply(lhs: T, rhs: T) -> (T, Self::Fail); } @@ -74,13 +68,6 @@ impl CheckedPrimitiveOp for CheckedMul { type Fail = T::MulFailure; - #[inline] - fn use_chunked_loop() -> bool { - // Fixed-size reductions help LLVM combine signed i16/i32 overflow checks. Other widths - // and unsigned operations can lose efficient instruction selection with the same loop. - matches!(T::PTYPE, PType::I16 | PType::I32) - } - #[inline] fn apply(lhs: T, rhs: T) -> (T, T::MulFailure) { (lhs.mul_value(rhs), lhs.mul_failure(rhs)) diff --git a/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs b/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs index d267af12ae8..2655b7c5bbc 100644 --- a/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs +++ b/vortex-array/src/scalar_fn/fns/binary/numeric/row.rs @@ -63,6 +63,10 @@ impl RowFn for NumericBinary { ScalarFnVTable::id(&Binary) } + #[allow( + clippy::cognitive_complexity, + reason = "the primitive-type macro repeats the same operator dispatch for each type" + )] fn dispatch( &self, op: &Self::Options, @@ -74,11 +78,20 @@ impl RowFn for NumericBinary { .ok_or_else(|| vortex_err!("a numeric operator takes two operands, got none"))?, )?; + // Keep this choice inside the type macro. An outer operator/type match loses ARM i64/u64 + // vectorization by changing CGU placement. See the settings at `visit_checked_chunked`. match_each_native_ptype!(ptype, |T| { match op { NumericOperator::Add => visit_checked::(visitor), NumericOperator::Sub => visit_checked::(visitor), - NumericOperator::Mul => visit_checked::(visitor), + NumericOperator::Mul => { + // Chunking improves these signed widths across the measured x86 and ARM targets. + if matches!(T::PTYPE, PType::I16 | PType::I32) { + visit_checked_chunked::(visitor) + } else { + visit_checked::(visitor) + } + } NumericOperator::Div => visit_div::(visitor), } }) @@ -91,20 +104,37 @@ where Op: CheckedPrimitiveOp, V: RowVisitor, { - let apply = |(lhs, rhs)| Op::apply(lhs, rhs); - let finish_failure = |failure| { - if failure != ::default() { - return Err(numeric_error(Op::ERROR)); - } + visitor.visit_deferred::<(T, T), T, Op::Fail>( + |(lhs, rhs)| Op::apply(lhs, rhs), + |failure| { + if failure != ::default() { + return Err(numeric_error(Op::ERROR)); + } - Ok(()) - }; + Ok(()) + }, + ) +} - if Op::use_chunked_loop() { - visitor.visit_deferred_chunked::<(T, T), T, Op::Fail>(apply, finish_failure) - } else { - visitor.visit_deferred::<(T, T), T, Op::Fail>(apply, finish_failure) - } +// Keep the visit bodies separate. With rustc 1.98.0, LLVM 22.1.8, 16 CGUs, LTO off, and +neon, +// sharing callback construction and dispatch here loses the two-lane dense i64/u64 loops. +// Recheck binary_ops multiplication IR and timings before combining these helpers. +fn visit_checked_chunked(visitor: V) -> VortexResult +where + T: NativePType, + Op: CheckedPrimitiveOp, + V: RowVisitor, +{ + visitor.visit_deferred_chunked::<(T, T), T, Op::Fail>( + |(lhs, rhs)| Op::apply(lhs, rhs), + |failure| { + if failure != ::default() { + return Err(numeric_error(Op::ERROR)); + } + + Ok(()) + }, + ) } fn visit_div(visitor: V) -> VortexResult