On Wed, 10 Sep 2025 23:54:31 GMT, Johannes Graham <d...@openjdk.org> wrote:
>> 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.
>
> Without the fast-path, there was a performance drop with the "small"
> BigDecimals (from memory about 10%). That's what drove me to tinker with
> `BigInteger.toString`. But if this one can stand on its own, despite the perf
> drop, I'm happy to separate out the BigInteger part. I'll generate some new
> perf numbers after the Level.Invocation fix.
OK understood. I think we would only want to go through with this PR if we
could avoid that perf regression in the
_DefFormatterBench.testSmallBigDecDefNumberFormatter_ case. So either including
this BI fast path as part of this change, or (if there are scope concerns),
making a separate change for it first and making this PR dependent on it seems
reasonable to me. It would be good to get @rgiulietti's opinion on this.
I think it would also be good to include the _java.math.BigIntegers_ results
since this change directly impacts those results as well. (Which look to have
sizeable improvements for _BigIntegers.testSmallToString_ and
_BigIntegers.testLargeToString_.)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27118#discussion_r2345424936
