<i18n dev> RFR: 8309622: Re-examine the cache mechanism in BaseLocale [v4]

[Chen Liang]

On Thu, 29 Jun 2023 19:28:05 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> This is stemming from the PR: https://github.com/openjdk/jdk/pull/14211 where aggressive GC can cause NPE in `BaseLocale$Key` class. I refactored the in-house cache with WeakHashMap, and removed the Key class as it is no longer needed (thus the original NPE will no longer be possible). Also with the new JMH test case, it gains some performance improvement:
>> 
>> (w/o fix)
>> 
>> Benchmark                       Mode  Cnt      Score     Error  Units
>> LocaleCache.testForLanguageTag  avgt   20   5781.275 ± 569.580  ns/op
>> LocaleCache.testLocaleOf        avgt   20  62564.079 ± 406.697  ns/op
>> 
>> (w/ fix)
>> Benchmark                       Mode  Cnt      Score     Error  Units
>> LocaleCache.testForLanguageTag  avgt   20   4801.175 ± 371.830  ns/op
>> LocaleCache.testLocaleOf        avgt   20  60394.652 ± 352.471  ns/op
>
> Naoto Sato 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 'pull/14684' into JDK-8309622-Cache-BaseLocale
>  - Use ReferencedKeyMap in place for LocaleObjectCache
>  - Merge branch 'pull/14684' into JDK-8309622-Cache-BaseLocale
>  - Addressing review comments
>  - Addressing comments (test grouping, synchronization), minor optimization on loop lookup
>  - minor comment fix
>  - equals/hash fix, constructor simplification
>  - Removed Key
>  - minor fixup
>  - Use WeakHashMap
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/87a0fc50...870ec1fe

src/java.base/share/classes/sun/util/locale/BaseLocale.java line 167:

> 165:         // can subsequently be used by the Locale instance which
> 166:         // guarantees the locale components are properly cased/interned.
> 167:         return CACHE.computeIfAbsent(new BaseLocale(language, script, region, variant),

This `computeIfAbsent` will store the non-normalized base locale as a soft key in the cache, which is undesirable. I have commented on the base patch https://github.com/openjdk/jdk/pull/14684#discussion_r1266914091 to support distinct keys for querying and storing like those in the old `LocalObjectCache`, and we can then simply use a `ReferenceKeySet` for the cache.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14404#discussion_r1266923844