<i18n dev> RFR: 8345668: ZoneOffset.ofTotalSeconds performance regression [v3]

[Chen Liang]

On Fri, 27 Dec 2024 16:27:10 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> For sure we should use result of `putIfAbsent`. Let's do this for all cases. See how it was implemented in my first commit - https://github.com/openjdk/jdk/pull/9208/commits/73a2f6cb1b91f4d7ee374089f35a72ef7b94433b
>
>> For sure we should use result of `putIfAbsent`
> 
> Drive-by comment...
> 
> From what i can infer, the performance regression being addressed here is caused in part by the fact that (for example) `ConcurrentHashMap.computeIfAbsent()` provides an atomicity guarantee, which is a stronger requirement than is necessary here, and therefore by splitting up that call up into two separate, non-atomic `get()` and `put()` calls we get (counter-intuitively) faster execution time, even though there are more lines of code. Note `putIfAbsent()` also guarantees atomicity, so the same problem of slowness caused by "unnecessary atomicity" might occur with it as well.

Indeed, just noticed that both `computeIfAbsent` and `putIfAbsent` may acquire the lock when the key is present, while `get` never acquires a lock for read-only access.

Maybe the implementation was written back when locking was less costly (with biased locking, etc.). Now we might have to reconsider locking until we know for sure a plain get fails.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22854#discussion_r1898748424