On Tue, 5 Mar 2024 00:32:47 GMT, Justin Lu <[email protected]> wrote:
>> Please review this PR and corresponding CSR which prevents an
>> OutOfMemoryError by restricting the initial maximum fraction digits for an
>> empty pattern DecimalFormat.
>>
>> For an empty String pattern DecimalFormat, the maximum fraction digits is
>> initialized to `Integer.MAX_VALUE`. When toPattern() is invoked,
>> StringBuilder internally doubles capacity attempting to append
>> Integer.MAX_VALUE digits until OOME occurs. CSR covers potential behavioral
>> compatibility changes.
>
> Justin Lu has updated the pull request with a new target base due to a merge
> or a rebase. The incremental webrev excludes the unrelated changes brought in
> by the merge/rebase. The pull request contains five additional commits since
> the last revision:
>
> - cleanup/typo
> - Merge branch 'master' into JDK-8326908-emptyPattern-OOME
> - implement feedback + improve pattern related tests
> - minor additions
> - init
src/java.base/share/classes/java/text/DecimalFormat.java line 3367:
> 3365: }
> 3366: }
> 3367: return false;
can simply be a single return statement.
test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java line 60:
> 58: dFmt.setMinimumFractionDigits(minFrac);
> 59:
> 60: String[] patterns = dFmt.toPattern().split("\\.");
Instead of assuming/hardcoding the decimal separator,
`DecimalFormatSymbols.getInstance(Locale.US).getDecimalSeparator()` may be
better.
test/jdk/java/text/Format/DecimalFormat/ToPatternTest.java line 110:
> 108: // 8326908: Verify that an empty pattern DecimalFormat does not
> throw an
> 109: // OutOfMemoryError when toPattern() is invoked. Behavioral change of
> 110: // MAXIMUM_INTEGER_DIGITS replaced with DOUBLE_FRACTION_DIGITS for
> empty
Should we explicitly check `new DecimalFormat("").getMaximumFractionDigits() ==
DOUBLE_FRACTION_DIGITS`?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513356393
PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513364152
PR Review Comment: https://git.openjdk.org/jdk/pull/18094#discussion_r1513378668
