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. Thanks for doing this cleanup. I only found some instances in comments where `lightweight` referred to the locking mode and not just the fast path, so the word should be removed rather than replaced with `fast`. src/hotspot/share/oops/markWord.hpp line 215: > 213: ObjectMonitor* monitor() const { > 214: assert(has_monitor(), "check"); > 215: assert(!UseObjectMonitorTable, "Fast locking with OM table does not > use markWord for monitors"); Suggestion: assert(!UseObjectMonitorTable, "Locking with OM table does not use markWord for monitors"); src/hotspot/share/oops/markWord.hpp line 241: > 239: } > 240: static markWord encode(ObjectMonitor* monitor) { > 241: assert(!UseObjectMonitorTable, "Fast locking with OM table does not > use markWord for monitors"); Suggestion: assert(!UseObjectMonitorTable, "Locking with OM table does not use markWord for monitors"); src/hotspot/share/runtime/objectMonitor.hpp line 164: > 162: // Because of frequent access, the metadata field is at offset zero > (0). > 163: // Enforced by the assert() in metadata_addr(). > 164: // * Fast locking with UseObjectMonitorTable: Suggestion: // * Locking with UseObjectMonitorTable: src/hotspot/share/runtime/objectMonitor.hpp line 166: > 164: // * Fast locking with UseObjectMonitorTable: > 165: // Contains the _object's hashCode. > 166: // * * Fast locking without UseObjectMonitorTable: Suggestion: // * Locking without UseObjectMonitorTable: src/hotspot/share/runtime/objectMonitor.inline.hpp line 77: > 75: > 76: inline markWord ObjectMonitor::header() const { > 77: assert(!UseObjectMonitorTable, "Fast locking with OM table does not use > header"); Suggestion: assert(!UseObjectMonitorTable, "Locking with OM table does not use header"); src/hotspot/share/runtime/objectMonitor.inline.hpp line 82: > 80: > 81: inline void ObjectMonitor::set_header(markWord hdr) { > 82: assert(!UseObjectMonitorTable, "Fast locking with OM table does not use > header"); Suggestion: assert(!UseObjectMonitorTable, "Locking with OM table does not use header"); src/hotspot/share/runtime/objectMonitor.inline.hpp line 87: > 85: > 86: inline intptr_t ObjectMonitor::hash() const { > 87: assert(UseObjectMonitorTable, "Only used by fast locking with OM > table"); Suggestion: assert(UseObjectMonitorTable, "Only used by locking with OM table"); src/hotspot/share/runtime/objectMonitor.inline.hpp line 92: > 90: > 91: inline void ObjectMonitor::set_hash(intptr_t hash) { > 92: assert(UseObjectMonitorTable, "Only used by fast locking with OM > table"); Suggestion: assert(UseObjectMonitorTable, "Only used by locking with OM table"); src/hotspot/share/runtime/synchronizer.cpp line 648: > 646: // stability and then install the hash. > 647: } else { > 648: assert(!mark.is_unlocked() && !mark.is_fast_locked(), "invariant"); Note that `mark.monitor()` below already asserts `mark.has_monitor()` which is stronger. ------------- PR Review: https://git.openjdk.org/jdk/pull/27915#pullrequestreview-3371295012 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456141768 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456143321 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456146275 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456190353 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456194705 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456195722 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456201765 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456204642 PR Review Comment: https://git.openjdk.org/jdk/pull/27915#discussion_r2456288423
