RFR: 8319773: Avoid inflating monitors when installing hash codes for LM_LIGHTWEIGHT [v10]

[Daniel D. Daugherty]

On Thu, 30 Nov 2023 08:06:28 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> So what did you decide to do here?
>
> For now I believe the extra code noise from trying to handle this race with deflation is not worth it. I creates some questionable code paths and head scratchers. If we were to add a separate FastHashCode just for LM_LIGHTWEIGHT it would be worth it as the while loop body would look quite a bit different and be easier to reason about. 
> 
> But I was looking for input if we should handle this case regardless of code complexity. Or maybe taking this all the way and create a separate FastHashCode with its own more understandable logic which does not have to try to fit in with the legacy locking/inflation protocol.
> 
> Regardless if we were to just go with it as it is now there should probably be a comment here along the line:
> ```c++
>   // With LM_LIGHTWEIGHT FastHashCode may race with deflation here and cause a monitor to be re-inflated.

I don't think the race with deflation is limited to LM_LIGHTWEIGHT. The inflation
code below detects when there is a collision with async deflation and retries
which can lead to a re-inflation when we loop around again. We can reach the
code below with LM_LEGACY, LM_LIGHTWEIGHT, or LM_MONITOR so I don't
think you need the LM_LIGHTWEIGHT specific comment.

Yes, we can reach this point in the code when `mark.has_monitor() == true` and
not just when `LockingMode == LM_LIGHTWEIGHT`, but the `inflate()` function
already has to handle that race (and it does). When a Java monitor is lightweight
locked or stack-locked, there can be more than one contending thread and each
of those threads will attempt to `inflate()` the Java monitor into an ObjectMonitor.
Only one thread can win the inflation race and all of the racers trust `inflate()`
to do the right thing. What's the "right thing"? One of the callers to `inflate()` will
install the ObjectMonitor successfully and return it to that caller. All of the other
callers to `inflate()` will detect that they lost the race and return the winner's
ObjectMonitor to their callers.

There's no reason for the logic to skip the call to `inflate()` because races are
already handled by `inflate()`.

We got into this spiraling thread because we were trying to figure out if a
non-JavaThread could call `inflate()` because `inflate()` can call `is_lock_owned()`
which has a header comment which talks about non-JavaThreads...

I believe that is possible with JVM/TI tagging even when we are in
LM_LIGHTWEIGHT mode because a lightweight monitor can be inflated
by a contending thread which can cause the ObjectMonitor to have an
anonymous owner. In that case, this if-statement in `inflate()` can execute:

      if (LockingMode == LM_LIGHTWEIGHT && inf->is_owner_anonymous() && is_lock_owned(current, object)) {
        inf->set_owner_from_anonymous(current);
        JavaThread::cast(current)->lock_stack().remove(object);
      }

Of course, if our caller is the VMThread, `is_lock_owned()` will return
false so we won't execute the if-statement's code block.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16603#discussion_r1411319912