[master] RFR: OMWorld: Spin Changes [v2]

[Coleen Phillimore]

On Fri, 24 May 2024 15:08:45 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> The fast lock spinning uses `sched_yield` which tends to be discouraged for spin locking code. Instead only use `SpinPause` with exponential backoff. Where after each failed CAS wait for exponentially more time until trying again in an attempt to reduce cache contention. 
>> 
>> This change also makes the spinning aware of safepoints, and tries to fast track the execution to next poll, which is either when successfully locked (VM backedge transition) or when going into blocked to enter the ObjectMonitor.
>> 
>> Have not removed `OMSpins` yet, as the exact value is not determined yet. It may have to be platform specific as `SpinPause` have different characteristics on different hardware. OMSpins is the number of fast lock, with each attempt spinning for twice as much as the last, so the total number of spins are on the order of O(2^OMSpins). It will probably land somewhere on the range of 7-14 (128 -16384 spins)
>
> Axel Boldt-Christmas has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Cancel spinning early in case of inflation
>  - Renamed first_time variable
>  - Extract fast_lock_spin_enter

Some small comments requested, please, and to help check my understanding of this code.  Thank you.

src/hotspot/share/runtime/globals.hpp line 2001:

> 1999:   product(bool, OMShrinkCHT, false, "")                                     \
> 2000:                                                                             \
> 2001:   product(int, OMSpins, 13, "")                                             \

Can you add a comment for now, even if we move this somewhere internal (as a Knob_OMSpin??)

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 684:

> 682:   while (true) {
> 683:     // Fast-locking does not use the 'lock' argument.
> 684:     if (fast_lock_spin_enter(obj(), current, observed_deflation)) {

So here the lock_stack doesn't contain the object but we're trying to spin in order to stall creating or fetching an ObjectMonitor for this lock.  Is that right?  Can you say why this is.  It seems to spin before testing whether it can get the lock.  Is that because the caller already tried and found the object locked?

The first_time => observed_deflation rename is good.  That helps a lot because reading this, you don't expect to have to wait.

src/hotspot/share/runtime/lightweightSynchronizer.cpp line 698:

> 696:     }
> 697: 
> 698:     observed_deflation = true;

I see now. Can you comment that if inflate_and_enter fails, it means that deflation was observed is why?

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

PR Review: https://git.openjdk.org/lilliput/pull/177#pullrequestreview-2077540175
PR Review Comment: https://git.openjdk.org/lilliput/pull/177#discussion_r1613833913
PR Review Comment: https://git.openjdk.org/lilliput/pull/177#discussion_r1613841296
PR Review Comment: https://git.openjdk.org/lilliput/pull/177#discussion_r1613845118