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 2f03e2262b perf(arrow-cast): Scan integer and fractional decimal
digits in separate loops (#10974)
2f03e2262b is described below
commit 2f03e2262ba233587a26be93c6d23a21b6b57a53
Author: Neil Conway <[email protected]>
AuthorDate: Fri Sep 4 07:10:38 2026 -0400
perf(arrow-cast): Scan integer and fractional decimal digits in separate
loops (#10974)
# Which issue does this PR close?
- Closes #10961.
# Rationale for this change
We recently consolidated the decimal parsing code to use a single, more
correct implementation (#10850); however, this resulted in regression
performance somewhat.
`parse_decimal` used a single loop to scan each digit of the input.
Digits before and after the decimal point are treated very differently;
profiling the code revealed that doing a data-dependent branch for each
digit resulted in worse codegen.
Instead, we can split the parsing logic to use two loops, in sequence:
first look for digits that precede the decimal point, then those that
follow. This avoids the data-dependent branch and seems to significantly
improve codegen.
On M4 Max, this improves the end-to-end CSV parsing benchmark by 5-15%,
and the `parse_decimal` microbenchmark by 5-33%.
# What changes are included in this PR?
* Split decimal parsing logic into two sequential loops
* Refactor digit accumulation logic to avoid duplication
* Mark `fold_decimal_chunk` as `inline(always)` -- the refactor resulted
in LLVM deciding not to inline this call inside the hot loop, which
regressed performance significantly.
# Are these changes tested?
Yes, covered by existing tests.
# Are there any user-facing changes?
No.
# Tool usage
Explored optimization ideas and developed this optimization with Claude
Code (Fable 5.1). I revised and understand the resulting code.
---
arrow-cast/src/parse.rs | 107 +++++++++++++++++++++++++++++++++---------------
1 file changed, 74 insertions(+), 33 deletions(-)
diff --git a/arrow-cast/src/parse.rs b/arrow-cast/src/parse.rs
index 81b4b5949e..d077a9c5a4 100644
--- a/arrow-cast/src/parse.rs
+++ b/arrow-cast/src/parse.rs
@@ -962,54 +962,60 @@ fn parse_decimal_mantissa<T: DecimalType>(
}
};
- let mut value = T::Native::ZERO;
- let mut chunk = 0_u64;
- let mut chunk_len = 0_usize;
- let mut saw_point = false;
+ let mut acc = DecimalAccumulator::<T> {
+ value: T::Native::ZERO,
+ chunk: 0,
+ chunk_len: 0,
+ negative,
+ };
let mut int_kept = 0_usize;
let mut frac_kept = 0_usize;
let mut first_discarded_digit = None;
+ // Digits before the decimal point
let mut index = 0;
while let Some(&b) = mantissa.get(index) {
- match b {
- b'0'..=b'9' => {
- let digit = b - b'0';
- let (kept, keep) = if saw_point {
- (&mut frac_kept, frac_keep)
- } else {
- (&mut int_kept, int_keep)
- };
- if *kept < keep {
- *kept += 1;
- // Cannot overflow: the chunk is folded into `value`
before it
- // exceeds MAX_CHUNK_DIGITS digits, all of which fit in a
u64
- chunk = chunk * 10 + digit as u64;
- chunk_len += 1;
- if chunk_len == MAX_CHUNK_DIGITS {
- value = fold_decimal_chunk::<T>(value, chunk,
chunk_len, negative)?;
- chunk = 0;
- chunk_len = 0;
- }
- } else {
- first_discarded_digit.get_or_insert(digit);
- }
- }
- b'.' if !saw_point => saw_point = true,
- b'e' | b'E' => return Err(MantissaError::Exponent(index)),
- _ => return Err(MantissaError::InvalidFormat),
+ if !b.is_ascii_digit() {
+ break;
+ }
+ if int_kept < int_keep {
+ int_kept += 1;
+ acc.push(b - b'0')?;
+ } else {
+ first_discarded_digit.get_or_insert(b - b'0');
}
index += 1;
}
- if chunk_len > 0 {
- value = fold_decimal_chunk::<T>(value, chunk, chunk_len, negative)?;
+ // Digits after the decimal point
+ if mantissa.get(index) == Some(&b'.') {
+ index += 1;
+ while let Some(&b) = mantissa.get(index) {
+ if !b.is_ascii_digit() {
+ break;
+ }
+ if frac_kept < frac_keep {
+ frac_kept += 1;
+ acc.push(b - b'0')?;
+ } else {
+ first_discarded_digit.get_or_insert(b - b'0');
+ }
+ index += 1;
+ }
+ }
+
+ match mantissa.get(index) {
+ None => {}
+ Some(b'e' | b'E') => return Err(MantissaError::Exponent(index)),
+ Some(_) => return Err(MantissaError::InvalidFormat),
}
if int_kept == 0 && frac_kept == 0 && first_discarded_digit.is_none() {
return Err(MantissaError::InvalidFormat);
}
+ let mut value = acc.finish()?;
+
// Scale the value up to the target scale. Skipped for zero, where
computing
// 10^missing could overflow the native type even though the result (zero)
// is always representable.
@@ -1062,10 +1068,45 @@ fn split_sign(bytes: &[u8]) -> (bool, &[u8]) {
}
}
+/// Accumulates decimal digits into `chunk`, folding it into `value` whenever
+/// it reaches [`MAX_CHUNK_DIGITS`] digits
+struct DecimalAccumulator<T: DecimalType> {
+ value: T::Native,
+ chunk: u64,
+ chunk_len: usize,
+ negative: bool,
+}
+
+impl<T: DecimalType> DecimalAccumulator<T> {
+ #[inline(always)]
+ fn push(&mut self, digit: u8) -> Result<(), DecimalParseError> {
+ // Cannot overflow: the chunk is folded into `value` before it exceeds
+ // MAX_CHUNK_DIGITS digits, all of which fit in a u64
+ self.chunk = self.chunk * 10 + digit as u64;
+ self.chunk_len += 1;
+ if self.chunk_len == MAX_CHUNK_DIGITS {
+ self.value =
+ fold_decimal_chunk::<T>(self.value, self.chunk,
self.chunk_len, self.negative)?;
+ self.chunk = 0;
+ self.chunk_len = 0;
+ }
+ Ok(())
+ }
+
+ /// Folds the digits still in `chunk` into the value
+ #[inline]
+ fn finish(self) -> Result<T::Native, DecimalParseError> {
+ if self.chunk_len == 0 {
+ return Ok(self.value);
+ }
+ fold_decimal_chunk::<T>(self.value, self.chunk, self.chunk_len,
self.negative)
+ }
+}
+
/// Folds a chunk of up to [`MAX_CHUNK_DIGITS`] digits into `value`, producing
/// `value * 10^chunk_len + chunk` (`chunk` is negated first when parsing a
/// negative number).
-#[inline]
+#[inline(always)]
fn fold_decimal_chunk<T: DecimalType>(
value: T::Native,
chunk: u64,
