RFR: 8373595: A new ObjectMonitorTable implementation [v5]

[Fredrik Bredberg]

On Mon, 16 Feb 2026 06:02:11 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> Fredrik Bredberg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Updated for Andrew
>
> 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).

Yea, that is the idea.

> 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.

I agree, will fix 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) {

Fixed

> 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);

Fixed

> 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()`.

Fixed

> 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);

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2817132038
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2817117931
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2817122541
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2817123491
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2817126358
PR Review Comment: https://git.openjdk.org/jdk/pull/29383#discussion_r2817125013