RFR: 8340796: Use a consistent order when loading cxq and EntryList

[David Holmes]

On Mon, 21 Oct 2024 23:40:04 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Made sure we use a consistent order when reading ObjectMonitor EntryList and cxq while exiting the monitor.
>> That consistent order is EntryList before cxq. Added a comment in the shared code to explaining why it's okay that waiters slip undetected between reading EntryList and cxq.
>> 
>> Tested ok tier1-3.
>
> src/hotspot/share/runtime/objectMonitor.cpp line 1158:
> 
>> 1156: // The race could be removed by reading cxq before EntryList, but it
>> 1157: // would come with an added cost of needing a loadload fence between the
>> 1158: // reads.
> 
> I think you can write this much more succinctly as this isn't really a race per-se, but part of the overall operation of the protocol. Suggestion:
> 
> // Note that we read the EntryList and then the cxq after dropping the lock, so the values need not form a stable
> // snapshot. In particular, after reading the (empty) EntryList, another thread could acquire and release the lock, moving 
> // any entries in the cxq to the EntryList, causing the current thread to see an empty cxq and conclude there are no
> // waiters. But this is okay as the thread that moved the cxq is responsible for waking the successor.

Update: okay the fields are volatile so maybe the compiler will preserve the written order.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21584#discussion_r1809668294