feat: reduce allocations in Spark Decimal access - #9842
Conversation
Read small-precision Decimal128 values from native long words, retaining the full-width Arrow conversion when the integer cannot fit in a long. Handle both native word orders and use the source scale when constructing BigDecimal, leaving requested rescaling and overflow checks to Spark. Preserve Spark's expanded Decimal representation for checked integer casts. Add tests for decimal values, rescaling, malformed buffers, both word orders, slices, object independence, negative scales and checked casts. Validated on Spark 3.5.9/Scala 2.12 and Spark 4.1.2/Scala 2.13, with 26 targeted tests per version plus Javadoc and test formatting checks. AI-assisted implementation and tests. Signed-off-by: Peifeng Li <lipeifeng@xiaohongshu.com>
Merging this PR will improve performance by 78.89%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | take_fsl_random[128, 10] |
59.1 µs | 33 µs | +78.89% |
| Simulation | take_fsl_u32_random[256, 10] |
< 1 ns | < 1 ns | N/A | |
| Simulation | fixed_16_advancing_ptr_safe[100] |
< 1 ns | < 1 ns | N/A | |
| Simulation | preverify_advancing_ptr_unchecked[1000] |
< 1 ns | < 1 ns | N/A | |
| Simulation | preverify_advancing_ptr_unchecked[10000] |
< 1 ns | < 1 ns | N/A | |
| Simulation | bench_compare_sliced_dict_primitive[(3333, 10000)] |
77.5 µs | < 1 ns | N/A |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing xiaoh1024:exp/decimal-accessor-pr (614ad61) with develop (1eb5b43)
Footnotes
-
329 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
|
Hi @robert3005, would you have a chance to review this when you have time? |
robert3005
left a comment
robert3005
left a comment
There was a problem hiding this comment.
Some nits, I would like to understand why we need BigDecimal.valueOf
| if (high == (unscaled >> 63)) { | ||
| // Decode with the source scale; Spark performs the requested rescaling and overflow checks. | ||
| // Keep the expanded Decimal representation for Spark's checked integer casts. | ||
| return Decimal.apply(BigDecimal.valueOf(unscaled, accessor.getScale()), precision, scale); |
There was a problem hiding this comment.
can we use Decimal.apply(unscaled, p, s) ? What does BigDecimal.valueOf change here?
There was a problem hiding this comment.
I initially tried Decimal.apply(unscaled, p, s), but it changes the internal representation and some downstream conversion behavior.
In Spark 3.5.9 and 4.1.2, for example, 127.999999999999999 throws an overflow exception on roundToByte() through the existing BigDecimal-based path, while the compact Decimal created by the long overload returns 127. The checked integer conversion test covers this difference.
Using BigDecimal.valueOf(unscaled, accessor.getScale()) preserves that behavior. It also reconstructs the value with the source scale, before Spark applies the requested precision and scale. Directly using the requested scale with the unscaled integer would change the value when the scales differ.
We still avoid the temporary byte array and BigInteger created by Arrow’s getObject(), while retaining the existing Spark Decimal behavior.
Initialize both UnsafeRowWriter instances before writing and verify that each row reads back the expected Decimal. Add braces around the small Decimal null check. Signed-off-by: Peifeng Li <lipeifeng@xiaohongshu.com>
2 participants
Summary
Closes #9837. See the issue for the motivation, design, benchmark results, and decimal-specific validation.
Changes
SmallDecimalAccessorfor source precision 1–18, preserving full-width fallback and Spark's decimal conversion semantics.AI assistance: AI assistance was used for implementation, tests, validation tooling, and this description, including translation from Chinese.