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

LGTM

FWIW I am not sure the race would have been benign - as `put` is not atomic - 
so I wouldn't be surprised if concurrent `put`s  could have resulted in an 
inconsistent internal state for the `WeakHashMap`.

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

Marked as reviewed by dfuchs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14404#pullrequestreview-1475620449

Reply via email to