On Mon, 31 Aug 2026 05:13:00 GMT, Axel Boldt-Christmas <[email protected]> wrote:
>> src/hotspot/share/runtime/objectMonitorTable.cpp line 571: >> >>> 569: Table* curr = _curr.load_acquire(); >>> 570: >>> 571: assert(hash != 0, "must be"); >> >> Seems unrelated to the cleanup. Why must it be? > > All our `put` methods expect the hash to have been installed in the markWord > (I think the get has the same expectation, but there it is not a correctness > issue if it is missing) > > Right now we only ever interact with the ObjectMonitorTable if we have > encountered an object with the monitor lock bits. And it is invariant that an > object where we once have seen the monitor lock bits also must have a hash > installed in the header. > > I think we could and probably should rewrite this so that our `get` > short-circuits objects without a hash, and that our `put` uses > `oopDesc::identity_hash` which ensures that we have an identity hash for the > object. And remove the `(void)object->identity_hash(...);` calls inside > `ObjectSynchronizer::inflate_*` First, I agree with Axel's comment. Then to expand a bit on the unrelated, must it be, comment. The `assert()` was part of the `set_hash()` code in `ObjectSynchronizer::add_monitor()`. Since I removed the `set_hash()` part, only the `assert()` part remained, and I felt that it didn't belong in the general `ObjectSynchronizer::add_monitor()`, which doesn't care about any hash at all. Who knows if we'll add another mechanism instead of the `ObjectMonitorTable` in the future, that doesn't need the hash. So I moved it to `ObjectMonitorTable::monitor_put_get()`, because the `ObjectMonitorTable` really do need the hash. Bottom line, must it be? Strictly speaking, no. But it felt natural to move it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32573#discussion_r3893072616
