This is an automated email from the ASF dual-hosted git repository.
Jefffrey pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new e097ebede1 perf: Improve decimal addition and subtraction when scale
is equal (#10333)
e097ebede1 is described below
commit e097ebede174c03a0ef791499f397fc7067886df
Author: Adam Gutglick <[email protected]>
AuthorDate: Tue Jul 14 11:39:35 2026 +0100
perf: Improve decimal addition and subtraction when scale is equal (#10333)
# Which issue does this PR close?
- Closes #NNN.
# Rationale for this change
For cases where adding or subtracting decimal values that have the same
scale, we end up doing a bunch of extra checked mul operations, and
turns out if we don't it improves performance significantly.
# What changes are included in this PR?
1. Specialize add/sub for decimals when they have the same scale.
2. Benchmark to measure the impact on all decimals types.
# Are these changes tested?
Existing tests, 1 additional test I added to i256 which was my original
target.
# Are there any user-facing changes?
None
---
Cargo.lock | 1 +
arrow-arith/Cargo.toml | 7 +++
arrow-arith/benches/decimal_arithmetic.rs | 66 ++++++++++++++++++++++++++++
arrow-arith/src/numeric.rs | 73 +++++++++++++++++++++++++++++++
4 files changed, 147 insertions(+)
diff --git a/Cargo.lock b/Cargo.lock
index 1aba3d551d..b0e5f93428 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -204,6 +204,7 @@ dependencies = [
"arrow-data",
"arrow-schema",
"chrono",
+ "criterion",
"num-traits",
]
diff --git a/arrow-arith/Cargo.toml b/arrow-arith/Cargo.toml
index f2a4604c11..c3222eccf2 100644
--- a/arrow-arith/Cargo.toml
+++ b/arrow-arith/Cargo.toml
@@ -42,3 +42,10 @@ arrow-data = { workspace = true }
arrow-schema = { workspace = true }
chrono = { workspace = true }
num-traits = { version = "0.2.19", default-features = false, features =
["std"] }
+
+[dev-dependencies]
+criterion = { workspace = true }
+
+[[bench]]
+name = "decimal_arithmetic"
+harness = false
diff --git a/arrow-arith/benches/decimal_arithmetic.rs
b/arrow-arith/benches/decimal_arithmetic.rs
new file mode 100644
index 0000000000..c958caff6b
--- /dev/null
+++ b/arrow-arith/benches/decimal_arithmetic.rs
@@ -0,0 +1,66 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::hint;
+
+use arrow_arith::numeric::{add, sub};
+use arrow_array::PrimitiveArray;
+use arrow_array::types::{
+ Decimal32Type, Decimal64Type, Decimal128Type, Decimal256Type, DecimalType,
+};
+use arrow_buffer::ArrowNativeType;
+use criterion::{Criterion, Throughput, criterion_group, criterion_main};
+
+const SIZE: usize = 1024;
+
+fn decimal<T: DecimalType>(
+ values: impl Iterator<Item = T::Native>,
+ scale: i8,
+) -> PrimitiveArray<T> {
+ PrimitiveArray::<T>::new(values.collect::<Vec<_>>().into(), None)
+ .with_precision_and_scale(T::MAX_PRECISION, scale)
+ .unwrap()
+}
+
+fn benchmark<T: DecimalType>(c: &mut Criterion, name: &str) {
+ for (scale, right_scale) in [("equal", 0), ("different", 1)] {
+ let left = decimal::<T>((0..SIZE).map(T::Native::usize_as), 0);
+ let right = decimal::<T>(
+ (0..SIZE).map(|i| T::Native::usize_as(SIZE - i)),
+ right_scale,
+ );
+ let mut group = c.benchmark_group(format!("{name}_{scale}_scale"));
+ group.throughput(Throughput::Elements(SIZE as u64));
+ group.bench_function("add", |b| {
+ b.iter(|| hint::black_box(add(&left, &right).unwrap()))
+ });
+ group.bench_function("sub", |b| {
+ b.iter(|| hint::black_box(sub(&left, &right).unwrap()))
+ });
+ group.finish();
+ }
+}
+
+fn decimal_arithmetic(c: &mut Criterion) {
+ benchmark::<Decimal32Type>(c, "decimal32");
+ benchmark::<Decimal64Type>(c, "decimal64");
+ benchmark::<Decimal128Type>(c, "decimal128");
+ benchmark::<Decimal256Type>(c, "decimal256");
+}
+
+criterion_group!(benches, decimal_arithmetic);
+criterion_main!(benches);
diff --git a/arrow-arith/src/numeric.rs b/arrow-arith/src/numeric.rs
index f5a844ffd2..7e0bb2e7aa 100644
--- a/arrow-arith/src/numeric.rs
+++ b/arrow-arith/src/numeric.rs
@@ -821,6 +821,13 @@ fn decimal_op<T: DecimalType>(
let r_mul = T::Native::usize_as(10).pow_checked((result_scale -
s2) as _)?;
match op {
+ // Equal scales make both decimal multipliers one.
+ Op::Add | Op::AddWrapping if s1 == s2 => {
+ try_op!(l, l_s, r, r_s, l.add_checked(r))
+ }
+ Op::Sub | Op::SubWrapping if s1 == s2 => {
+ try_op!(l, l_s, r, r_s, l.sub_checked(r))
+ }
Op::Add | Op::AddWrapping => {
try_op!(
l,
@@ -1290,6 +1297,72 @@ mod tests {
assert_eq!(err, "Divide by zero error");
}
+ #[test]
+ fn test_decimal256_same_scale_add_sub() {
+ let lhs = Decimal256Array::from(vec![
+ Some(i256::from_parts(u128::MAX, 0)),
+ Some(i256::MINUS_ONE),
+ None,
+ ])
+ .with_precision_and_scale(70, 2)
+ .unwrap();
+ let rhs = Decimal256Array::from(vec![Some(i256::ONE), Some(i256::ONE),
Some(i256::MAX)])
+ .with_precision_and_scale(70, 2)
+ .unwrap();
+
+ let expected =
+ Decimal256Array::from(vec![Some(i256::from_parts(0, 1)),
Some(i256::ZERO), None])
+ .with_precision_and_scale(71, 2)
+ .unwrap();
+ for operation in [add, add_wrapping] {
+ let result = operation(&lhs, &rhs).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+ }
+
+ let expected = Decimal256Array::from(vec![
+ Some(i256::from_parts(u128::MAX - 1, 0)),
+ Some(i256::from_i128(-2)),
+ None,
+ ])
+ .with_precision_and_scale(71, 2)
+ .unwrap();
+ for operation in [sub, sub_wrapping] {
+ let result = operation(&lhs, &rhs).unwrap();
+ assert_eq!(result.as_primitive::<Decimal256Type>(), &expected);
+ }
+
+ let lhs = Decimal256Array::from(vec![i256::MAX])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ let rhs = Decimal256Array::from(vec![i256::ONE])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ for operation in [add, add_wrapping] {
+ assert_eq!(
+ operation(&lhs, &rhs).unwrap_err().to_string(),
+ format!(
+ "Arithmetic overflow: Overflow happened on: {:?} + {:?}",
+ i256::MAX,
+ i256::ONE
+ )
+ );
+ }
+
+ let lhs = Decimal256Array::from(vec![i256::MIN])
+ .with_precision_and_scale(76, 0)
+ .unwrap();
+ for operation in [sub, sub_wrapping] {
+ assert_eq!(
+ operation(&lhs, &rhs).unwrap_err().to_string(),
+ format!(
+ "Arithmetic overflow: Overflow happened on: {:?} - {:?}",
+ i256::MIN,
+ i256::ONE
+ )
+ );
+ }
+ }
+
fn test_timestamp_impl<T: TimestampOp>() {
let a = PrimitiveArray::<T>::new(vec![2000000, 434030324,
53943340].into(), None);
let b = PrimitiveArray::<T>::new(vec![329593, 59349, 694994].into(),
None);
