RFR: 8367982: Unify ObjectSynchronizer and LightweightSynchronizer [v2]

David Holmes dholmes at openjdk.org
Mon Oct 27 00:33:09 UTC 2025


On Fri, 24 Oct 2025 14:20:36 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> This is the last PR in a series of PRs (see: [JDK-8344261](https://bugs.openjdk.org/browse/JDK-8344261)) to obsolete the LockingMode flag and related code.
>> 
>> The main focus is to to unify `ObjectSynchronizer` and `LightweightSynchronizer`.
>> There used to be a number of "dispatch functions" to redirect calls depending on the setting of the `LockingMode` flag.
>> Since we now only have lightweight locking, there is no longer any need for those dispatch functions, so I removed them.
>> To remove the dispatch functions I renamed the corresponding lightweight functions and call them directly.
>> This ultimately led me to remove "lightweight" from the function names and go back to "fast" instead, just to avoid having some with, and some without the "lightweight" part of the name.
>> 
>> This PR also include a small simplification of `ObjectSynchronizer::FastHashCode`.
>> 
>> Tested tier1-7 (on supported platforms) without seeing any problems that can be traced to this code change.
>> All other platforms (`arm`, `ppc`, `riscv`, `s390`) has been sanity checked using QEMU.
>
> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update after review

Overall this looks fine. I have a couple of suggestions/requests below.

Thanks

src/hotspot/share/runtime/abstract_vm_version.hpp line 195:

> 193: 
> 194:   // Is recursive fast locking implemented for this platform?
> 195:   constexpr static bool supports_recursive_fast_locking() { return false; }

Next cleanup: this is supported on all platforms now, so we can get rid of this migration aid.

src/hotspot/share/runtime/synchronizer.cpp line 287:

> 285:   _last_async_deflation_time_ns = os::javaTimeNanos();
> 286: 
> 287:   ObjectSynchronizer::create_om_table();

The original code should effectively be inlined here:

if (UseObjectMonitorTable) {
  ObjectMonitorTable::create();
}

src/hotspot/share/runtime/synchronizer.cpp line 655:

> 653:     // installed in the object header.
> 654:     return install_hash_code(current, obj);
> 655:   }

This merging of the OMT and non-OMT `FastHashCode` does not seem related to the `lightweight` renaming. I found it very hard to convince myself of the equivalences here so would prefer to see this in a separate PR.

src/hotspot/share/runtime/synchronizer.cpp line 1838:

> 1836:   if (!UseObjectMonitorTable) {
> 1837:     return;
> 1838:   }

This should move to the caller and be replaced with an assertion at this level. Though you don't need to introduce this method at all as the caller can call `OMT::create` directly.

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27915#pullrequestreview-3381506352
PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2464180623
PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2464188744
PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2464195475
PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2464191200


More information about the hotspot-dev mailing list