perf(arrow-cast): optimize parsing of decimals from strings - #10668
Open
neilconway wants to merge 2 commits into
Open
perf(arrow-cast): optimize parsing of decimals from strings#10668neilconway wants to merge 2 commits into
neilconway wants to merge 2 commits into
Conversation
Rewrite parse_string_to_decimal_native to accumulate digits in u64 chunks that are folded into the target native type with checked arithmetic (one wide multiply per 19 digits), instead of splitting the string and round-tripping through i256 and intermediate allocations. String-to-decimal128 casts are ~8x faster and decimal256 casts ~6.5x faster; the parser microbenchmarks improve 64-92% across all cases. The checked arithmetic also fixes two bugs in the previous implementation: Decimal256 values whose unscaled magnitude exceeds the i256 range could silently wrap to an arbitrary in-range value instead of reporting overflow, and inputs with more than 76 fractional digits were rejected even when the scaled value fits the target type. Also use PrimitiveBuilder instead of an intermediate Vec and unsafe from_trusted_len_iter in the strict (safe=false) cast path. The safe path keeps the unsafe trusted-len construction: it measures 15-19% faster than a PrimitiveBuilder equivalent, and its justification comment now records that. Also document the accepted syntax and rounding behavior, and add benchmark coverage for string-to-decimal parsing.
Contributor
|
i wonder if this will fix/affect some of the other issues we have open for decimal parsing/casting, see: edit: seems like none, those other issues are for a different decimal parsing function |
Contributor
Author
Yeah, I believe those issues are distinct. Although I think it might make sense to unify some of the code here, both to simplify the implementation and to address some inconsistencies (e.g., rounding vs. truncating when given more decimal digits than required for the target type's scale). I can take a look at some of that once this PR lands. |
neilconway
force-pushed
the
neilc/perf-decimal-parser
branch
from
August 13, 2026 14:41
8012d45 to
7f9a781
Compare
Contributor
|
run benchmark parse_decimal |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing neilc/perf-decimal-parser (7f9a781) to 4f0c3c8 (merge-base) diff Run configurationrun benchmark parse_decimalCPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Contributor
|
run benchmark cast_kernels |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing neilc/perf-decimal-parser (7f9a781) to 4f0c3c8 (merge-base) diff Run configurationrun benchmark cast_kernels
env:
BENCH_FILTER: "string to decimal"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Jefffrey
approved these changes
Aug 15, 2026
Jefffrey
left a comment
Jefffrey
left a comment
Contributor
There was a problem hiding this comment.
my local run of the parse_decimal benchmark, with the new benches:
string decimal128 integer
time: [15.733 ns 15.758 ns 15.782 ns]
+ change: [−86.119% −86.045% −85.974%] (p = 0.00 < 0.05)
+ Performance has improved.
Found 9 outliers among 100 measurements (9.00%)
2 (2.00%) low severe
4 (4.00%) low mild
3 (3.00%) high mild
string decimal128 exact scale
time: [15.383 ns 15.443 ns 15.508 ns]
+ change: [−86.121% −86.036% −85.955%] (p = 0.00 < 0.05)
+ Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
1 (1.00%) low mild
8 (8.00%) high mild
3 (3.00%) high severe
string decimal128 padded scale
time: [15.591 ns 15.615 ns 15.638 ns]
+ change: [−87.092% −87.039% −86.988%] (p = 0.00 < 0.05)
+ Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) low severe
2 (2.00%) low mild
string decimal128 rounded scale
time: [16.002 ns 16.066 ns 16.134 ns]
+ change: [−90.851% −90.820% −90.786%] (p = 0.00 < 0.05)
+ Performance has improved.
string decimal128 signed
time: [15.932 ns 15.987 ns 16.045 ns]
+ change: [−92.027% −91.995% −91.961%] (p = 0.00 < 0.05)
+ Performance has improved.
string decimal128 38 digits
time: [27.153 ns 27.196 ns 27.239 ns]
+ change: [−77.117% −77.040% −76.963%] (p = 0.00 < 0.05)
+ Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mild
string decimal256 76 digits
time: [61.585 ns 61.706 ns 61.824 ns]
+ change: [−64.960% −64.841% −64.712%] (p = 0.00 < 0.05)
+ Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) low mild
string decimal256 rounded scale
time: [61.868 ns 61.990 ns 62.116 ns]
+ change: [−85.577% −85.534% −85.487%] (p = 0.00 < 0.05)
+ Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
1 (1.00%) low severe
2 (2.00%) low mild
1 (1.00%) high mildvery impressive results
| v.map(|v| { | ||
| parse_string_to_decimal_native::<T>(v, scale as usize) | ||
| .map_err(|_| { | ||
| let mut builder = PrimitiveBuilder::<T>::with_capacity(from.len()); |
Contributor
There was a problem hiding this comment.
note: this change isnt captured in cast_kernels benchmark since we only benchmark for safe = true
| if fractionals == scale { | ||
| first_discarded_digit.get_or_insert(digit); | ||
| index += 1; | ||
| continue; |
Contributor
There was a problem hiding this comment.
i do wonder if instead of continuing via the loop to verify we have a valid number, we specialize to another loop that checks if the remainder characters is an ascii digit; it might be able to vectorize better?
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?
Decimal256#10665Rationale for this change
The previous implementation of
parse_string_to_decimal_nativeused a string-manipulation approach: it trimmed the input string, split it based on".", parsed both halves withi256::from_string, converted the result back to a string withformat!, and parsed that again withi256::from_string.Instead, we use a single pass over the input bytes. A simple state machine walks over the input digits, accumulating a running sum. This avoids all of the string manipulation and heap allocation of the previous approach.
We further optimize this by accumulating the running sum in a
u64, and then periodically folding that partial value into the runningdecimalvalue (we do this often enough that there is no risk of overflowing theu64). That trades a bit of redundant computation for doing more work inu64and less work indecimal; based on benchmarking, this is a clear win.Finally, we don't need to accumulate digits from the suffix of the string. Values beyond the target type's scale don't contribute to the result value; only the first such digit influences rounding behavior.
This new approach also fixes two correctness bugs (#10664 and #10665) in the previous implementation.
Benchmarks (M4 Max)
Parser microbenchmarks (arrow-cast/benches/parse_decimal.rs):
End-to-end cast kernel (arrow/benches/cast_kernels.rs, 512-row string array, safe mode):
What changes are included in this PR?
parse_string_to_decimal_nativeas described aboveVec+unsafewithPrimitiveBuilder, which saves an allocationAre these changes tested?
Yes; existing tests pass, and new tests have been added. I also checked the new implementation against a naive oracle built using the num-bigint crate; the new implementation was consistent with
num-bigintfor 120M randomly generated inputs.Are there any user-facing changes?
No, aside from fixed bugs.
AI usage
Iterated with the help of Claude Fable; I reviewed and understand the resulting code.