On Thu, 29 Jun 2023 19:28:05 GMT, Naoto Sato <na...@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
