RFR: 8343840: Rewrite the ObjectMonitor lists [v2]

[Fredrik Bredberg]

On Wed, 26 Feb 2025 06:19:38 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update after review by David and Coleen.
>
> src/hotspot/share/runtime/objectMonitor.cpp line 724:
> 
>> 722:   for (;;) {
>> 723:     ObjectWaiter* front = Atomic::load(&_entry_list);
>> 724: 
> 
> No need for blank line.

Fixed

> src/hotspot/share/runtime/objectMonitor.cpp line 731:
> 
>> 729: 
>> 730:     // Interference - the CAS failed because _entry_list changed.  Just retry.
>> 731:     // As an optional optimization we retry the lock.
> 
> Suggestion:
> 
>     // Interference - the CAS failed because _entry_list changed.  Before
>     // retrying the CAS retry taking the lock as it may now be free.

Fixed

> src/hotspot/share/runtime/objectMonitor.cpp line 812:
> 
>> 810:   guarantee(_entry_list == nullptr,
>> 811:             "must be no entering threads: entry_list=" INTPTR_FORMAT,
>> 812:             p2i(_entry_list));
> 
> Mustn't re-read _entry_list in the p2i as it may have changed from the value that is causing the guarantee to fail. The old guarantees were buggy in this regard - a temp is needed.

Fixed

> src/hotspot/share/runtime/objectMonitor.cpp line 1299:
> 
>> 1297:     assert(_entry_list_tail == nullptr || _entry_list_tail == currentNode, "invariant");
>> 1298: 
>> 1299:     ObjectWaiter* v = Atomic::load(&_entry_list);
> 
> Nit: use `w` to be consistent with similar code. The original used `w` for EntryList and `v` for cxq IIRC.

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23421#discussion_r1974268658
PR Review Comment: https://git.openjdk.org/jdk/pull/23421#discussion_r1974268941
PR Review Comment: https://git.openjdk.org/jdk/pull/23421#discussion_r1974267878
PR Review Comment: https://git.openjdk.org/jdk/pull/23421#discussion_r1974269555