On Fri, 24 Oct 2025 14:20:36 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. > > 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
