RFR: 8315884: New Object to ObjectMonitor mapping [v9]
Axel Boldt-Christmas
aboldtch at openjdk.org
Tue Aug 13 14:01:56 UTC 2024
On Tue, 23 Jul 2024 16:36:18 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> src/hotspot/share/runtime/lightweightSynchronizer.cpp line 102:
>>
>>> 100: assert(*value != nullptr, "must be");
>>> 101: return (*value)->object_is_cleared();
>>> 102: }
>>
>> The `is_dead` functions seem oddly placed given they do not relate to the object stored in the wrapper. Why are they here? And what is the difference between `object_is_cleared` and `object_is_dead` (as used by `LookupMonitor`) ?
>
> This is a good question. When we look up the Monitor, we don't want to find any that the GC has marked dead, so that's why we call object_is_dead. When we look up with the object to find the Monitor, the object won't be dead (since we're using it to look up). But we don't want to find one that we've cleared because the Monitor was deflated? I don't see where we would clear it though. We clear the WeakHandle in the destructor after the Monitor has been removed from the table.
What @coleenp said is correct. One is just a null check, the other interacts with the GC and the objects lifecycle.
But as also mentioned we do not ever clear these any WeakHandle, so this is currently always returning false. (At least from what I can see). This load should be cached or in a register because it always load this to check if it is the value when doing a lookup. So there should be no performance cost here, but it is unnecessary. I'll remove this.
The `object_is_dead` is only used when removing, where we can take the extra cost.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20067#discussion_r1715347334
More information about the core-libs-dev
mailing list