On Fri, 5 Sep 2025 14:18:19 GMT, Johannes Graham <[email protected]> wrote:
> When formatting doubles or BigDecimals, DigitList first formats them as a
> string and then parses the resultant string to extract the mantissa and the
> exponent. This can be done more directly. This allows removing some parsing
> code and removes a cached byte array.
>
> This also facilitates potential cleanups in FloatingDecimal (removal of
> getChars method) but I've left that for later to minimize conflicts with
> other changes there.
On a first pass, the changes in `DigitList` look good to me: we utilize the
`BigDecimal` to retrieve the exponent + unscaled value so that we don't need
to re-parse the String representation to determine `decimalAt` and inflate
`digits`.
Thanks for this improvement which also simplifies and cleans up the code as
well. I left some minor comments.
src/java.base/share/classes/java/math/BigInteger.java line 4184:
> 4182:
> 4183: if (fitsIntoLong()) {
> 4184: return Long.toString(longValue(), radix);
We may want to consider separating this `BigInteger` fast path from this
PR/change, since it is independent of the speedup in the `DigitList` changes.
Others may not have a problem with it though, so maybe we can wait and see what
they say.
test/jdk/java/text/Format/DecimalFormat/CloneTest.java line 1:
> 1: /*
I know the JBS issue was not yet created when the PR was up, but now the Jtreg
header needs 8367324 added to bug section.
test/micro/org/openjdk/bench/java/text/DefFormatterBench.java line 1:
> 1: /*
Nit: Copyright year bump.
test/micro/org/openjdk/bench/java/text/DefFormatterBench.java line 88:
> 86:
> 87: // This will create "small" BigDecimals where unscaled value
> fits in a long.
> 88: BigDecimal bd = BigDecimal.valueOf(value);
Can we separate the creation of the BD from the benchmark here and create these
values in setup as well.
test/micro/org/openjdk/bench/java/text/DefFormatterBench.java line 100:
> 98: // of the toString value if the BigDecimal instance was
> reused.
> 99:
> 100: BigDecimal bd = new BigDecimal(value.unscaledValue(),
> value.scale());
If we use `@Setup(Level.Iteration)` on setup, I believe we can ensure a fresh
`bdValues` for each benchmark iteration measurement. Then we can remove the BD
recreation to not be included within the benchmark results which should give us
more accurate results.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27118#pullrequestreview-3207801811
PR Review Comment: https://git.openjdk.org/jdk/pull/27118#discussion_r2337937683
PR Review Comment: https://git.openjdk.org/jdk/pull/27118#discussion_r2337894410
PR Review Comment: https://git.openjdk.org/jdk/pull/27118#discussion_r2337894989
PR Review Comment: https://git.openjdk.org/jdk/pull/27118#discussion_r2337905594
PR Review Comment: https://git.openjdk.org/jdk/pull/27118#discussion_r2337914280
