RFR: 8319801: Recursive lightweight locking: aarch64 implementation [v11]

Thu Feb 1 20:57:09 UTC 2024

On Fri, 26 Jan 2024 09:24:00 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Implements the aarch64 port of JDK-8319796.
>> 
>> There are two major parts for the port implementation. The C2 part, and the part shared by the interpreter, C1 and the native call wrapper.
>> 
>> The biggest change for both parts is that we check the lock stack first and if it is a recursive lightweight [un]lock and in that case simply pop/push and finish successfully.
>> 
>> Only if the recursive lightweight [un]lock fails does it look at the mark word. 
>> 
>> For the shared part if it is an unstructured exit, the monitor is inflated or the mark word transition fails it calls into the runtime.
>> 
>> The C2 operates under a few more assumptions, that the locking is structured and balanced. This means that some checks can be elided. 
>> 
>> First this means that in C2 unlock if the obj is not on the top of the lock stack, it must be inflated. And reversely if we reach the inflated C2 unlock the obj is not on the lock stack. This second property makes it possible to avoid reading the owner (and checking if it is anonymous). Instead it can either just do an un-contended unlock by writing null to the owner, or if contention happens, simply write the thread to the owner and jump to the runtime. 
>> 
>> The aarch64 C2 port tries to avoid stronger memory semantics where ever possible. In C2 lock it first does a relaxed load of the mark word to check for inflation. Both lock and unlock uses a load/store exclusive register pair to transition the mark word.
>
> 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 17 additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>  - Drop memory order comments
>  - Preloads markWord unconditionally
>  - Revert "Add preload_mark to MacroAssembler::lightweight_lock"
>    
>    This reverts commit 8950f503aa5dba0e203613bd9737ea0d50388ca3.
>  - Add preload_mark to MacroAssembler::lightweight_lock
>  - Rename box to t1
>  - Remove third tmp from fast_lock
>  - Merge remote-tracking branch 'upstream_jdk/pr/16606' into JDK-8319801
>  - Switch to CAS over LXSX
>  - Fix missing $
>  - ... and 7 more: https://git.openjdk.org/jdk/compare/7f5d8421...d0a02754

I've done a crawl thru review of the v10 webrev. I only have a couple of editorial comments.

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 59:

> 57:   Label count, no_count;
> 58: 
> 59:   assert(LockingMode != LM_LIGHTWEIGHT, "uses fast_lock_lightweight");

Perhaps: "lightweight locking should use fast_lock_lightweight"

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 158:

> 156:   Label count, no_count;
> 157: 
> 158:   assert(LockingMode != LM_LIGHTWEIGHT, "uses fast_unlock_lightweight");

Perhaps "lightweight locking should use fast_unlock_lightweight"

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 234:

> 232:   // Handle inflated monitor.
> 233:   Label inflated;
> 234:   // Finish fast lock successfully. MUST reach to with flag == EQ

nit typo: s/reach to/branch to/

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 250:

> 248:   { // Lightweight locking
> 249: 
> 250:     // Push lock to the lock stack and finish successfully. MUST reach to with flag == EQ

nit typo: s/reach to/branch to/

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 341:

> 339:   // Handle inflated monitor.
> 340:   Label inflated, inflated_load_monitor;
> 341:   // Finish fast unlock successfully. MUST reach to with flag == EQ

nit typo: s/reach to/branch to/

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 6384:

> 6382: void MacroAssembler::lightweight_unlock(Register obj, Register t1, Register t2, Register t3, Label& slow) {
> 6383:   assert(LockingMode == LM_LIGHTWEIGHT, "only used with new lightweight locking");
> 6384:   assert_different_registers(obj, t1, t2, t3, rscratch1);

I'm not seeing `rscratch1` being used in this function.

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

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16608#pullrequestreview-1857258375
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1474932331
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1474933142
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1474973301
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1475094131
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1475108159
PR Review Comment: https://git.openjdk.org/jdk/pull/16608#discussion_r1475130312