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

[Roman Kennke]

On Tue, 4 Apr 2023 06:46:02 GMT, David Holmes <dholmes at openjdk.org> wrote:

> One thing I can't quite get clear in my head is whether the small window where an object's monitor is inflated and the object is still in the thread's lock-stack, could cause an issue for any external observers trying to determine the object's locked state.

Most observers are thread-local and are basically asking 'am I locking this object?'. Almost all cases where an external thread is checking lightweight- and monitor locks are doing so from a safepoint. The only exception seems to be the path through management.cpp that you are mentioning below, and you say we're ok there (and I tend to agree).

> I'm still unclear how we can have non-JavaThreads here. Only JavaThreads can lock object monitors.

I've checked again. This particular case is only reached through verification and can easily be solved. The other instance of similar check in synchronizer.cpp seems no longer be called from non-Java-thread, it's probably been fixed by some other change earlier in this PR. In any case, I changed these code paths to assume Java thread and it's not failing tier1 and tier2, but may be worth to do more extensive testing.

> src/hotspot/share/runtime/synchronizer.cpp line 1314:
> 
>> 1312:     LogStreamHandle(Trace, monitorinflation) lsh;
>> 1313:     if (mark.is_fast_locked()) {
>> 1314:       assert(LockingMode == 2, "can only happen with new lightweight locking");
> 
> I'd rather see this entire case guarded by "`if (LockingMode ==2)`" and have `is_fast_locked()` assert if called in any other mode. Similarly for the stack-locked case below.

Ok, I am doing this change. But it means many more LockingMode == 1 or 2 checks in other places, and also means that we need to do a raw-check under VerifyHeavyMonitor paths that the mark-word is not stack-/fast-locked. Overall it does look cleaner and gives a better idea which code path deals with what kind of locking.

> Not at all clear to me how this fits here. ??

This block checks whether the monitor is in waiting state. When it is anonymously locked it must be waiting. I added a comment.

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

PR Comment: https://git.openjdk.org/jdk/pull/10907#issuecomment-1496449012
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1157647952
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1157651107
PR Review Comment: https://git.openjdk.org/jdk/pull/10907#discussion_r1157652282