On Mon, 20 Jun 2022 15:01:55 GMT, liach <d...@openjdk.org> wrote:
>> src/java.base/share/classes/java/time/format/DateTimeTextProvider.java line
>> 319:
>>
>>> 317: store = prev;
>>> 318: }
>>> 319: }
>>
>> You could do better here and use `computeIfAbsent` with `createStore` as its
>> lambda. You could even change the signature of `createStore` to take
>> `Entry<TemporalField, Locale>` as its parameter and then you could have this
>> method be:
>>
>> private Object findStore(TemporalField field, Locale locale) {
>> return CACHE.computeIfAbsent(createEntry(field, locale),
>> this::createStore);
>> }
>>
>> ...
>>
>> private Object createStore(Entry<TemporalField, Locale> entry) {
>> ...
>> }
>>
>> This applies to most other changes you made, the one in `ZoneOffset` is the
>> only one that's slightly different because there you have adjustment of
>> `ID_CACHE` too.
>
> This behaves slightly different from the old initialization; the concurrent
> hash map blocks when the mapping function is run, just in case if
> non-blocking instantiation is what we want. If that's not a problem, I would
> prefer szegedi's suggestion.
@liach advance apologies for nitpicking: `ConcurrentHashMap` doesn't in general
block while the mapping function runs. It can block _some_ concurrent updates,
namely those that [hash to the same
bin](https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/0f801fe6fd2fcc181121f9846f6869ca3a03e18a/src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java*L324__;Iw!!ACWV5N9M2RV99hQ!JBDYAOKXAgaJoVrQyfzX6Cpak1HLEt3HG254q8COTDzpz7Ui1_Jm8iUTLqBdiluzP1PMfTEA4n31sDYGYuZr2t8$
) and it won't block concurrent reads. I'm not saying the concern you raised
isn't valid, just wanted to clarify that the situation is not nearly as grave
as overall blocking; after all it ain't a `Collections.synchronizedMap` :-)
-------------
PR: https://git.openjdk.org/jdk/pull/9208
