RFR: 8320318: ObjectMonitor Responsible thread
Fredrik Bredberg
fbredberg at openjdk.org
Wed Sep 11 12:48:08 UTC 2024
On Wed, 11 Sep 2024 06:16:33 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:
>> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 476:
>>
>>> 474: // Release lock.
>>> 475: movptr(Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(owner)), NULL_WORD);
>>> 476: membar(StoreLoad);
>>
>> Why a standalone `storeload` here? This does not define a fence, nor release semantics - as per the definitions in orderAccess.hpp
>
> On x86 `membar(LoadStore | StoreStore /* release */)` would be a nop. Not sure if adding it before nulling the pointer would make things clearer.
>
> `membar(StoreLoad);` is all that we need between clearing the owner and checking the queues / successor.
As @xmas92 wrote, membar(StoreLoad); is all that we need between clearing the owner and checking the queues / successor. And, since I use membar(StoreLoad) in all other platforms, I wanted it to be consistent.
Also if you look in [ObjectMonitor::exit](https://github.com/openjdk/jdk/blob/f9ddfc6fb0780a7d6e933a40ecd3cd458a058f04/src/hotspot/share/runtime/objectMonitor.cpp#L1132C6-L1132C25)() you'll see that this there is a call to [OrderAccess::storeload](https://github.com/openjdk/jdk/blob/f9ddfc6fb0780a7d6e933a40ecd3cd458a058f04/src/hotspot/share/runtime/objectMonitor.cpp#L1184)() just after [release_clear_owner](https://github.com/openjdk/jdk/blob/f9ddfc6fb0780a7d6e933a40ecd3cd458a058f04/src/hotspot/share/runtime/objectMonitor.cpp#L1183)(), so I'm just doing the same as has been done in the C++ slow-path for long.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1754393843
More information about the hotspot-dev
mailing list