On Thu, 27 Aug 2026 13:03:33 GMT, Axel Boldt-Christmas <[email protected]> wrote:
>> The interface on oopDesc and the markWord w.r.t. the locking bits have grown >> overtime the names do not reflect what they actually do, there are multiple >> ways of asking for the same property. >> >> The properties `is_locked` and `is_unlocked` are misleading. As the answer >> true of false does not necessarily reflect the locking state of the object. >> I suggest we use a single terminology `is_fast_locked` to mean the locking >> bits are locked using lightweight non-monitor locking and `is_neutral` to >> mean the locking bits are in the prototype state. >> >> Using `is_fast_unlocked` could be an alternative to `is_neutral`, but >> `is_neutral` captures the state better of being an object which is currently >> not taking part in locking. However the name does not make it obvious that >> it is referring to the locking state / mark state. Not 100% on this naming, >> and how the comments and code which uses these constants in the >> MacroAssembler should name and deal with this. >> >> Also cleaned up the C1 and C2 Valhalla header bits checks which were gated >> on the locking bits. There is not more displaced header so conditionally >> checking the prototype header in the Klass* is not needed. >> >> Testing (in progress): >> * Tier 1-5 Oracle supported platforms >> * GHA >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Axel Boldt-Christmas has updated the pull request incrementally with one > additional commit since the last revision: > > Removed the assert, we have other checks for inconsistent locking src/hotspot/share/oops/markWord.hpp line 52: > 50: // klass:22 hash:31 valhalla:4 age:4 self-fwd:1 lock:2 > 51: // > 52: // - lock bits are used to describe lock states: > fast-locked/lock-neutral/inflated It would be nice to get a little bit more info about the lock-neutral concept. I wonder if the following conveys the information, and at the same time is terse enough that it gets read? // - lock bits are used to describe lock states and GC marking / fowarding // // [header | 00] fast_locked object has been fast-locked // [header | 01] lock_neutral object has no monitor and is not locked // [header | 10] monitor object has a monitor (lock state recorded there) // [ptr | 11] marked used to mark an object (header is swapped out) src/hotspot/share/oops/markWord.hpp line 57: > 55: // [header | 00] fast-locked locked regular object > header (fast-locking in use) > 56: // [header | 01] lock-neutral regular object header > 57: // [header | 10] monitor inflated lock Since the monitor-table we also have a "regular object header" when we have the monitor pattern. I would suggest that we remove the "regular object header" from the lines above. src/hotspot/share/oops/markWord.hpp line 225: > 223: LP64_ONLY(assert(mask_bits(value(), valhalla_reserved_bit_in_place) > == 0, > 224: "Reserved bits should not be used. _value: " > PTR_FORMAT, _value)); > 225: return !is_lock_neutral() || !has_no_hash(); Pre-existing: The lock-bits is stale after the object monitor table removal. I don't remember if you update this in the hash PR. Otherwise, we should investigate these usages (in another PR) to see if they can be tweaked. src/hotspot/share/runtime/vmStructs.cpp line 1819: > 1817: > \ > 1818: declare_constant(markWord::fast_locked_value) > \ > 1819: declare_constant(markWord::lock_neutral_value) > \ Indentation src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Mark.java line 98: > 96: > 97: private static long fastLockedValue; > 98: private static long neutralValue; lockNeutralValue? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32544#discussion_r3872629920 PR Review Comment: https://git.openjdk.org/jdk/pull/32544#discussion_r3872508945 PR Review Comment: https://git.openjdk.org/jdk/pull/32544#discussion_r3872645469 PR Review Comment: https://git.openjdk.org/jdk/pull/32544#discussion_r3872487721 PR Review Comment: https://git.openjdk.org/jdk/pull/32544#discussion_r3872490885
