<i18n dev> RFR: 8358880: Performance of parsing with DecimalFormat can be improved [v2]

[Justin Lu]

On Mon, 9 Jun 2025 22:46:50 GMT, Johannes Graham <duke at openjdk.org> wrote:

>> This PR replaces construction of intermediate strings to be parsed with more direct manipulation of numbers. It also has a more streamlined mechanism of handling `Long.MIN_VALUE` when parsing longs by using `Long.parseUnsignedLong`
>> 
>> As a small side-effect it also eliminates the use of a cached StringBuilder in DigitList.
>> 
>> Testing:
>> - GHA
>> - Local run of tier 2 and jtreg:jdk/java/text
>> - New benchmark: DecimalFormatParseBench
>
> Johannes Graham has updated the pull request incrementally with one additional commit since the last revision:
> 
>   catch ArithmeticException

Thanks for the improvements. I think we need to prioritize behavioral compatibility with this change, so we will want to run the JCK tests as well for the extra safety.

src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java line 134:

> 132:      * @return The double-precision value of the conversion
> 133:      */
> 134:     public static double parseDoubleDigits(int decExp, char[] digits, int length) throws NumberFormatException {

I don't think this method needs to declare `NFE` in the signature since it is not reading in any strings so it should not throw.

src/java.base/share/classes/jdk/internal/math/FloatingDecimal.java line 1848:

> 1846:             buf[i] = (byte) digits[i];
> 1847:         }
> 1848:         return new ASCIIToBinaryBuffer(false, decExp, buf, length);

Since `negSign` is always false, adding a _positive_ somewhere to the method name might be helpful/explicit. Although you could argue that the array parameter itself is telling enough, but one could misinterpret the char array as including a sign character.

test/jdk/java/text/Format/DecimalFormat/CloneTest.java line 98:

> 96:                 }
> 97: 
> 98:                 Object tempBuilder = valFromDigitList(original, "tempBuilder");

Since this is required as a direct result of this change, I think that warrants adding the JBS bug ID to the Jtreg header.

test/micro/org/openjdk/bench/java/text/DecimalFormatParseBench.java line 28:

> 26: import java.text.DecimalFormat;
> 27: import java.text.ParseException;
> 28: import java.util.Locale;

Looks unused.

-------------

PR Review: https://git.openjdk.org/jdk/pull/25644#pullrequestreview-2911579886
PR Review Comment: https://git.openjdk.org/jdk/pull/25644#discussion_r2136657118
PR Review Comment: https://git.openjdk.org/jdk/pull/25644#discussion_r2136676237
PR Review Comment: https://git.openjdk.org/jdk/pull/25644#discussion_r2136643849
PR Review Comment: https://git.openjdk.org/jdk/pull/25644#discussion_r2136644390