RFR: 8319797: Recursive lightweight locking: Runtime implementation [v6]
Daniel D. Daugherty
dcubed at openjdk.org
Wed Nov 22 21:36:13 UTC 2023
On Thu, 16 Nov 2023 08:51:47 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> First of let us note that when reaching this code the unstructured exit is the common case. The normal exit and recursive exit is usually handled in the emitted code (this includes the interpreter). We reach this because either a CAS failed somewhere due to a concurrent hashCode instalment, or the exit was unstructured. Inflated monitors exit just jumps passed this code (everything is conditioned on `mark.is_fast_locked()`).
>>
>> Is this motivated by the structure/layout of the C++ code. Or an optimisation?
>>
>> If it is motivated by the structure/layout. Then we can lay it out as you described. It would add some code duplication.
>>
>> If it is motivated as an optimisation then after the recursive exit fail, we should just call remove and act based on the return value.
>
> I would not go so far as to say the unstructured locking case is common. Sure we are on the slow-path, which may be due to unstructured locking, or we may be here through deop (also a slow path) or through the native method wrapper, or ... but yes this is not really performance critical.
Perhaps changing this comment will help. Please consider:
// This lock is recursive but is not at the top of the lock stack so we're
// doing an unstructured exit. We have to fall thru to inflation below and
// let ObjectMonitor::exit() do the unlock.
This check on L570 has to be done before the block that begins on L572
because that block assumes that `object` is not a recursive lock and it
will remove all instances of `object` from the lock stack on L578. That
would unexpectedly unlock `object` too many times.
Just as a side note: The block from L572 -> L584 also handles both
a structured exit of a non-recursive lock AND an unstructured exit of a
non-recursive lock. This is true because we use `remove()` on L578
instead of a `pop()` which assumes top-of-stack/structured locking.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16606#discussion_r1402712313
More information about the hotspot-dev
mailing list