RFR: 8373595: A new ObjectMonitorTable implementation [v5]

[Axel Boldt-Christmas]

On Fri, 13 Feb 2026 13:08:22 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> Me and Anton Artemov (@toxaart) investigated a quite large regression that occurred in Pet Clinic that happened if you turned on Compact Object Headers. It was found that a large portion of that regression could be attributed to not finding the monitor in the Object Monitor Cache, and because we couldn't access the Object Monitor Table from C2 generated code, we often had to take the slow path.
>> 
>> By making the object monitor cache larger and make it use the object's hash value as a key, we managed to mitigate the regression.
>> 
>> Erik Österlund (@fisk) took that idea and elevated it to the next level, which means that he rewrote the object monitor table code so that we can now search for an object in the global object monitor table from C2 generated code. I.e. from `C2_MacroAssembler::fast_lock()`. As in my and Anton's version, the key is the hash value from the object.
>> 
>> Erik also provided new barrier code needed for ZGC for x86 and aarch64. Roman Kennke (@rkennke) provided the same for Shenandoah, also for x86 and aarch64.
>> 
>> We decided to keep the Object Monitor Cache, since we found that for most programs (but not Pet Clinic) the monitor you are looking for is likely found in the first positions of the cache (it's sorted in most recently used order). However we decresed the size from 8 to 2 elements.
>> 
>> After running extensive performance tests we can say that this has improved the performance in many of them, not only mitigated the regression in Pet Clinic.
>> 
>> Tests are running okay tier1-7 on supported platforms.
>> 
>> The rest of the platforms (`ppc`, `riscv` and `s390`) have been smoke tested using QEMU.
>> I mainly used this test for smoke testing with QEMU: `-XX:+UnlockDiagnosticVMOptions -XX:+UseObjectMonitorTable ./test/hotspot/jtreg/runtime/Monitor/UseObjectMonitorTableTest.java `
>
> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Updated for Andrew

Thanks for typing up the table and thanks again for implementing this!

Looks good, just some small comments.

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 257:

> 255: 
> 256:       // Read the monitor from the bucket.
> 257:       ldr(t1_monitor, Address(t3, t1_hash, Address::lsl(LogBytesPerWord)));

Just for my own understanding. The reason we do not need a `ldar` here is because all of other reads are data dependent (OopHandle, owner, recursion) and we have release semantics when inserting the monitor in the table. (And all other reads have no synch requirements with respect to object monitor inflation.)

We have the same (non-acquire) thing without the table when reading the monitor from the header (just we do not use the OopHandle).

src/hotspot/cpu/aarch64/gc/shared/barrierSetAssembler_aarch64.hpp line 138:

> 136:   OptoReg::Name refine_register(const Node* node,
> 137:                                 OptoReg::Name opto_reg);
> 138:   virtual void try_resolve_weak_handle_in_c2(MacroAssembler* masm, Register obj, Register tmp, Label& slow_path);

I wonder if this should have been 
Suggestion:

  virtual void try_resolve_weak_handle_in_c2(MacroAssembler* masm, Register handle, Register obj, Register tmp, Label& slow_path);


Because right now `obj` really should be named `handle_in_obj_out` or something. It did trip me up at first until looking at the definition of the function.

But I think this can be fixed in a follow up RFE.

src/hotspot/share/runtime/objectMonitorTable.cpp line 135:

> 133:   DEFINE_PAD_MINUS_SIZE(2, DEFAULT_CACHE_LINE_SIZE, sizeof(_items_count));
> 134: 
> 135:   static Entry as_entry(ObjectMonitor *monitor) {

Suggestion:

  static Entry as_entry(ObjectMonitor* monitor) {

src/hotspot/share/runtime/objectMonitorTable.cpp line 143:

> 141:   static ObjectMonitor* as_monitor(Entry entry) {
> 142:     assert(entry >= Entry::below_is_special, "Must be! (entry: " PTR_FORMAT ")", intptr_t(entry));
> 143:     return reinterpret_cast<ObjectMonitor *>(entry);

Suggestion:

    return reinterpret_cast<ObjectMonitor*>(entry);

src/hotspot/share/runtime/objectMonitorTable.cpp line 372:

> 370:       }
> 371: 
> 372:       if (entry != removed() && entry == old_monitor) {

While we do not assert it in this function, we expect that `old_monitor >= below_is_special` and as such `entry == old_monitor` implies `entry != removed()`.

src/hotspot/share/runtime/objectMonitorTable.cpp line 487:

> 485:       if (entry != tombstone() && entry != removed()) {
> 486:         // A monitor
> 487:         ObjectMonitor *monitor = as_monitor(entry);

Suggestion:

        ObjectMonitor* monitor = as_monitor(entry);

-------------

Marked as reviewed by aboldtch (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/29383#pullrequestreview-3797956246
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2810804374
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2804823307
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2805003929
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2805005028
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2804908590
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2805008736