RFR: 8320318: ObjectMonitor Responsible thread [v2]

[Fredrik Bredberg]

On Wed, 11 Sep 2024 12:04:58 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 482:
>> 
>>> 480:   // This is faster on Nehalem and AMD Shanghai/Barcelona.
>>> 481:   // See https://blogs.oracle.com/dave/entry/instruction_selection_for_volatile_fences
>>> 482:   lock(); addl(Address(rsp, 0), 0);
>> 
>> Since there's a membar above, do you need this lock/addl instructions?
>
> Well spotted @coleenp! No it's not needed. It was meant to be replaced by `membar(StoreLoad)`, which as @xmas92  wrote, does exactly that. Also, since I use `membar(StoreLoad)` in all other platforms, I wanted it to be consistent.

fixed

>> src/hotspot/share/runtime/objectMonitor.cpp line 353:
>> 
>>> 351: 
>>> 352: void ObjectMonitor::enter_for_with_contention_mark(JavaThread* locking_thread, ObjectMonitorContentionMark& contention_mark) {
>>> 353:   DEBUG_ONLY(bool success = ) ObjectMonitor::enterI_with_contention_mark(locking_thread, contention_mark);
>> 
>> This is kind of noisy with DEBUG_ONLY.  If you remove DEBUG_ONLY, does the windows compiler complain that you're not using the variable success in the product build?
>
> I don't know. I come from a planet where warnings was errors, and just brought along the old habit to my new planet. I'll check with the Windows compiler.

fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1758844955
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1758850687