RFR: 8319773: Avoid inflating monitors when installing hash codes for LM_LIGHTWEIGHT [v10]

Axel Boldt-Christmas aboldtch at openjdk.org
Thu Nov 30 08:10:10 UTC 2023


On Wed, 29 Nov 2023 18:25:56 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> LM_LEGACY and LM_MONITOR will. LM_LIGHTWEIGHT technically may. If deflation finishes between reading the mark word in FastHashCode and reading the mark word in `inflate`. It seems like a rare enough case that it does not need to be handled separately. The following change would avoid inflation all together.
>> 
>>     // An async deflation can race after the inflate() call and before we
>>     // can update the ObjectMonitor's header with the hash value below.
>> +   if (LockingMode == LM_LIGHTWEIGHT) {
>> +     assert(mark.has_monitor(), "must be");
>> +     monitor =  mark.monitor();
>> +   } else {
>> -   monitor = inflate(current, obj, inflate_cause_hash_code);
>> +     monitor = inflate(current, obj, inflate_cause_hash_code);
>> +   }
>>     // Load ObjectMonitor's header/dmw field and see if it has a hash.
>> 
>> 
>> Maybe I should change it to this. Given that there has been confusion here.
>> My ideal solution would be to separate the implementations for the different locking modes all together, all of these functions are littered with if (LockingMode == X).
>
> So what did you decide to do here?

For now I believe the extra code noise from trying to handle this race with deflation is not worth it. I creates some questionable code paths and head scratchers. If we were to add a separate FastHashCode just for LM_LIGHTWEIGHT it would be worth it as the while loop body would look quite a bit different and be easier to reason about. 

But I was looking for input if we should handle this case regardless of code complexity. Or maybe taking this all the way and create a separate FastHashCode with its own more understandable logic which does not have to try to fit in with the legacy locking/inflation protocol.

Regardless if we were to just go with it as it is now there should probably be a comment here along the line:
```c++
  // With LM_LIGHTWEIGHT FastHashCode may race with deflation here and cause a monitor to be re-inflated.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16603#discussion_r1410289581


More information about the hotspot-dev mailing list