RFR: 8320318: ObjectMonitor Responsible thread [v3]

Axel Boldt-Christmas aboldtch at openjdk.org
Wed Sep 18 20:58:39 UTC 2024


On Wed, 18 Sep 2024 18:29:23 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> Removed the concept of an ObjectMonitor Responsible thread.
>> 
>> The reason to have an ObjectMonitor Responsible thread was to avoid threads getting stranded due to a hole in the successor protocol. This hole was there because adding the necessary memory barrier was considered too expensive some 20 years ago.
>> 
>> The ObjectMonitor Responsible thread code adds complexity, and doing timed parks just to avoid getting stranded is not the way forward. More info about the problems with the ObjectMonitor responsible thread can be found in [JDK-8320318](https://bugs.openjdk.org/browse/JDK-8320318).
>> 
>> After removing the ObjectMonitor Responsible thread we see increased performance on all supported platforms except Windows. [JDK-8339730](https://bugs.openjdk.org/browse/JDK-8339730) has been created to handle this.
>> 
>> Passes tier1-tier7 on supported platforms.
>> x64, AArch64, Riscv64, ppc64le and s390x passes ok on the test/micro/org/openjdk/bench/vm/lang/LockUnlock.java test.
>> Arm32 and Zero doesn't need any changes as far as I can tell.
>
> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update two, after the review

I think this looks good now. Just a small comment on the `ObjectMonitor::try_enter` function.

src/hotspot/share/runtime/objectMonitor.cpp line 385:

> 383: }
> 384: 
> 385: bool ObjectMonitor::try_enter(JavaThread* current, bool check_owner) {

The `check_owner` name is a little confusing to me. To me it looks more like `check_for_recursion` or `handle_recursion`.

src/hotspot/share/runtime/objectMonitor.cpp line 396:

> 394:   // to use ObjectMonitor::try_enter() as a public way of doing TryLock().
> 395:   // Used this way in SharedRuntime::monitor_exit_helper().
> 396:   if (check_owner) {

Probably preference but an early return here is easier for me to parse.

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

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19454#pullrequestreview-2313781060
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1765709613
PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1765709568


More information about the hotspot-dev mailing list