On Mon, 12 Jun 2023 17:33:11 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 incrementally with one additional 
> commit since the last revision:
> 
>   Addressing comments (test grouping, synchronization), minor optimization on 
> loop lookup

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

> 164:         // can subsequently be used by the Locale instance which
> 165:         // guarantees the locale components are properly cased/interned.
> 166:         synchronized (BaseLocale.class) {

The simplification is good but I wonder if this coarse locking is going to be a 
problem, do we need to use some concurrent to avoid contention here?

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

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

Reply via email to