<i18n dev> RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v10]

[Jaikiran Pai]

On Thu, 10 Mar 2022 18:34:27 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> Several attempts have been made to improve Formatter's numeric performance by caching the current Locale zero. Such fixes, however, ignore the real issue, which is the slowness of fetching DecimalFormatSymbols. By directly caching DecimalFormatSymbols in the Formatter, this enhancement streamlines the process of accessing Locale DecimalFormatSymbols and specifically getZeroDigit(). The result is a general improvement in the performance of numeric formatting. 
>> 
>> 
>> @Benchmark 
>> public void bigDecimalDefaultLocale() { 
>>     result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f", X); 
>> } 
>> 
>> @Benchmark 
>> public void bigDecimalLocaleAlternate() { 
>>     result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f", X); 
>>     other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f", X); 
>> } 
>> 
>> @Benchmark 
>> public void bigDecimalThaiLocale() { 
>>     result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f", X); 
>> } 
>> 
>> @Benchmark 
>> public void integerDefaultLocale() { 
>>     result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d", x); 
>> } 
>> 
>> @Benchmark 
>> public void integerLocaleAlternate() { 
>>     result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d", x); 
>>     other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d", x); 
>> } 
>> 
>> @Benchmark 
>> public void integerThaiLocale() { 
>>     result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d", x); 
>> } 
>> 
>> 
>> Before: 
>> Benchmark Mode Cnt Score Error Units 
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s 
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s 
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s 
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s 
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s 
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s 
>> 
>> After: 
>> Benchmark Mode Cnt Score Error Units 
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s 
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s 
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s 
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s 
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s 
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey 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 14 additional commits since the last revision:
> 
>  - Merge branch 'master' into 8282625
>  - Correct indentation
>  - Add unit test for DecimalFormatSymbols.getLocale()
>  - Add comment about DecimalFormatSymbols being mutable objects.
>  - Revert "Cache DecimalFormatSymbols in DecimalFormatSymbols instead of Formatter. No significant performance degradation."
>    
>    This reverts commit fcbf66a2fe9641d3c3349f12cc7b32d8b84c6f72.
>  - Revert "Drop DecimalFormatSymbols.getLocale change"
>    
>    This reverts commit b93cdb03ec68f24f4b8851c0966bb144c30b5110.
>  - Revert "Correct caching test"
>    
>    This reverts commit bf7975396aaad4ed58d053bde8f54984215eeba5.
>  - Correct caching test
>  - Drop DecimalFormatSymbols.getLocale change
>  - Cache DecimalFormatSymbols in DecimalFormatSymbols instead of Formatter. No significant performance degradation.
>  - ... and 4 more: https://git.openjdk.java.net/jdk/compare/11e9685d...84fa1fe7

src/java.base/share/classes/java/text/DecimalFormatSymbols.java line 192:

> 190: 
> 191:     /**
> 192:      * {@return locale used to create this instance}

Hello Jim, the opening and closing `{`, I think aren't needed for a `@return`

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

PR: https://git.openjdk.java.net/jdk/pull/7703