RFR: 8367982: Unify ObjectSynchronizer and LightweightSynchronizer

Coleen Phillimore coleenp at openjdk.org
Fri Oct 24 11:19:04 UTC 2025


On Tue, 21 Oct 2025 13:11:45 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.

Never say "last" cleanup.  The renaming to remove "lightweight" looks good.  I wasn't sure if we wanted to keep that name or not.  There's a lightweight NMT coming so this won't be confused with that anymore.  I guess that's good.

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

> 1452: // -----------------------------------------------------------------------------
> 1453: // ConcurrentHashTable storing links from objects to ObjectMonitors
> 1454: class ObjectMonitorTable : AllStatic {

I guess after looking at this, it made sense to combine the lightweightSynchronizer code into synchronizer.cpp (should be ObjectSynchronizer.hpp/cpp).  I wonder if the OM table code could be split out into its own file objectMontitorTable.hpp/cpp.  I feel like synchronzer.hpp/cpp again does too many different things.

src/hotspot/share/runtime/synchronizer.inline.hpp line 40:

> 38:     return read_monitor(mark);
> 39:   } else {
> 40:     return ObjectSynchronizer::get_monitor_from_table(current, obj);

I don't think there's a need for this file anymore.  read_monitor is mostly called inside synchronizer.cpp, so it can be inlined there.

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

PR Review: https://git.openjdk.org/jdk/pull/27915#pullrequestreview-3375815186
PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2459848893
PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2459863994


More information about the serviceability-dev mailing list