On Tue, 21 Oct 2025 13:11:45 GMT, Fredrik Bredberg <[email protected]> 
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

Reply via email to