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