RFR: 8320318: ObjectMonitor Responsible thread [v4]
Fredrik Bredberg
fbredberg at openjdk.org
Wed Sep 25 19:39:45 UTC 2024
On Wed, 25 Sep 2024 18:08:46 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> Fredrik Bredberg has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
>>
>> - Update three, after the review
>> - Merge branch 'master' into 8320318_objectmon_responsible_thread
>> - Update two, after the review
>> - Update one, after the review
>> - Small fixes before the review
>> - Merge branch 'master' into 8320318_objectmon_responsible_thread
>> - Merge branch 'master' into 8320318_objectmon_responsible_thread
>> - Removed _Responsible
>> - Fixed s390
>> - Fixed legacy locking
>> - ... and 4 more: https://git.openjdk.org/jdk/compare/0f253d11...8140570f
>
> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 474:
>
>> 472:
>> 473: // Set owner to null.
>> 474: // Release to satisfy the JMM
>
> This comment doesn't make sense to me. The JMM is the Java Memory Model, and this just releases the lock. The Java memory model implies memory ordering and optimizations. The next comment about fence is more meaningful. Did someone want this comment?
Someone did want this comment in each of the cpu-specific files. I do like to use the same comment in all equal places, so I did what someone wanted. But having said that, CPUs are different. In some you need an explicit release-instruction (like a `membar` of some sort), and in others you don't. I do agree that in a cpu-specific file where it's not needed with an explicit release-instruction, this comment makes no (or less sense). What to do?
> src/hotspot/share/runtime/objectMonitor.inline.hpp line 231:
>
>> 229: // lifetime" of the contention mark.
>> 230: assert(!_extended, "extending twice is probably a bad design");
>> 231: _monitor->add_to_contentions(1);
>
> This looks really good.
Thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1775898830
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1775899089
More information about the hotspot-dev
mailing list