On Fri, 27 Dec 2024 16:27:10 GMT, Archie Cobbs <aco...@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
