RFR: 8319799: Recursive lightweight locking: x86 implementation [v8]

[Axel Boldt-Christmas]

On Thu, 18 Jan 2024 22:27:43 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Axel Boldt-Christmas has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319799
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319799
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319799
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319799
>>  - top load adjustments
>>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319799
>>  - Fix type
>>  - Move inflated check in fast_locked
>>  - Move top load
>>  - 8319799: Recursive lightweight locking: x86 implementation
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/2adc7506...71c48af6
>
> src/hotspot/cpu/x86/c2_CodeStubs_x86.cpp line 120:
> 
>> 118:     // The owner may be anonymous and we removed the last obj entry in
>> 119:     // the lock-stack. This loses the information about the owner.
>> 120:     // Write the thread to the owner field so the runtime knows the owner.
> 
> I'm confused by this comment.  We get here if the monitor is inflated, so we didn't remove it from the lock stack.

True. This comment was written when there was an explicit monitor check before the CAS that jumped to inflated. I am not sure if there is a situation where the owner is anonymous here now. 

It should be invariant that if a thread's lock stack does not contain the oop, performs an unlock/monitorexit, the monitor is inflated and the owner is not anonymous. 

At all places in the runtime when removing the oops from the lock stack the owner field is fixed.

And in the emitted code the oop is pushed back to the lock stack incase of a failed unlock. 

There may be worth keeping this, and in the slow path after the CAS failed, check if it failed because of inflation, fix the owner field and jump back to the inflated fast path without transitioning to VM.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16607#discussion_r1458579707