david-mollitor-db opened a new pull request, #59067:
URL: https://github.com/apache/spark/pull/59067
### What changes were proposed in this pull request?
`Decimal` stores small values in a compact `long` (`decimalVal == null`) and
only "expands" to a
`java.math.BigDecimal` beyond 18 digits. Its arithmetic, however, boxed to
BigDecimal more than
necessary:
- `Decimal.*` always computed
`Decimal(toJavaBigDecimal.multiply(that.toJavaBigDecimal, MATH_CONTEXT))`,
with no compact path.
- `Decimal.+` / `Decimal.-` had a compact long path, but only when `scale ==
that.scale`; differing
scales fell back to BigDecimal.
So a row-level `a * b`, or a cross-scale `a + b`, on two compact operands
allocated two operand
`BigDecimal`s, a result `BigDecimal`, and its `BigInteger`.
This adds compact long fast paths, mirroring the existing same-scale `+`/`-`:
- `*` multiplies the unscaled longs directly when the exact product fits the
18-digit compact
range, guarded by an overflow-free `Math.multiplyHigh` check.
- `+`/`-` align the lower-scale operand by `POW_10(diff)` (guarded exactly
like `changePrecision`'s
scale-up path) and add/subtract as longs.
Both fall back to the existing BigDecimal path for non-compact operands or
when the exact result
leaves the compact range.
### Why are the changes needed?
The fast paths avoid per-row `BigDecimal`/`BigInteger` allocation for the
common small-decimal case
(e.g. `decimal(7,2)` prices/amounts). Profiling a full TPC-DS v1.4 run
(scale factor 10) with JFR,
before vs. after:
- `BigDecimal` allocations triggered by `Decimal.*` dropped ~93% (the
remainder are the
high-precision / overflow fallbacks).
- Total sampled allocation dropped ~5.8%.
This is an allocation / GC-pressure reduction (it also replaces
`BigDecimal.multiply` with a `long`
multiply on the hot path). On an unconstrained run it did not change
end-to-end wall-clock beyond
run-to-run noise; the benefit is reduced allocation, which matters most
under memory pressure and
high executor concurrency.
### Does this PR introduce _any_ user-facing change?
No. Results are byte-identical. The fast path only runs when the exact
result fits the compact long
range, where it has far fewer than `MATH_CONTEXT`'s 39 significant digits
(so no rounding occurs);
high-precision operands, including the SPARK-45786 multiply/divide case,
stay on the BigDecimal
path. Downstream `CheckOverflow` re-normalizes precision/scale exactly as
before.
### How was this patch tested?
- New `DecimalSuite` test sweeping a grid of `(unscaled, scale)` operands —
including values whose
exact result lands just under and just over the compact range, exercising
both the fast path and
the fallback — asserting `+`/`-`/`*` on compact operands match the
BigDecimal path on both value
and scale.
- Existing `DecimalSuite`, `ArithmeticExpressionSuite` (including the
SPARK-45786 cases), and
`DecimalExpressionSuite` pass.
- Allocation impact measured via JFR on a full TPC-DS v1.4 (SF10) run, as
above.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Isaac
This pull request and its description were written by Isaac.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
