RFR: 8291555: Implement alternative fast-locking scheme [v48]

Thomas Stuefe stuefe at openjdk.org
Fri Mar 31 16:01:02 UTC 2023


On Fri, 31 Mar 2023 15:24:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/JDK-8291555-v2' into JDK-8291555-v2
>>  - Check underflow, top-of-stack and mark-bits for sanity, in fast_unlock() (aarch64)
>
> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 6264:
> 
>> 6262:     ldrw(t1, Address(rthread, JavaThread::lock_stack_top_offset()));
>> 6263:     cmpw(t1, (unsigned)LockStack::start_offset());
>> 6264:     br(Assembler::GT, stack_ok);
> 
> I had to think hard about "GT" here. 
> 
> We could have entered with the thread holding just one inflated lock, then LockStack would be empty but the monitorexit would still be valid. You now do check in the callers for markWord::monitor_value. But the lock could have been inflated concurrently after the caller checks and before this point. 
> 
> But then the LockStack would not have changed, since it represents what the current thread *thinks* are thin locks, not what are actually thin locks? In other words, LockStack is only modified by its owning thread, never from the outside.
> 
> So this *should* be correct, but its certainly a brain teaser. Maybe add a comment? 
> 
> E.g. "These checks rely on the fact that LockStack is only ever modified by its owning stack, even if the lock got inflated concurrently; removal of LockStack entries after inflation will happen delayed in that case" or somesuch.

This also mandates that fast_lock can only ever entered if the current thread thinks that the lock in question is a thin lock. So the caller checks for markWord::monitor_value are mandatory now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1154619603


More information about the serviceability-dev mailing list