RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v6]

[Roman Kennke]

On Tue, 10 Sep 2024 07:53:23 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/collectedHeap.cpp line 232:
>> 
>>> 230:   }
>>> 231: 
>>> 232:   // With compact headers, we can't safely access the class, due
>> 
>> Suggestion:
>> 
>>   // With compact headers, we can't safely access the klass, due
>> 
>> 
>> This is the case why? Because we might not have copied the header yet? Is this method actually ever used while the forwarded object is unstable?
>> Given this is used for verification only afaik, we should make an effort to provide that check.
>
> With compact headers, we can't safely access the Klass* when the object has been forwarded, because non-full-GC-forwarding temporarily overwrites the mark-word, and thus the Klass*, with the forwarding pointer, and here we have no way to make a distinction between Full-GC and regular GC forwarding.
> 
> I improved the code to make the check when the object is not forwarded. Not sure if we could/should do more (e.g. pass around is_full argument to make the distinction, or find the - possibly few - places where we might call is_oop() on from-space objects in regular GC and do the check in a forwardee-safe way?).

Ah, I found it! It seems only the ShenandoahVerifier calls oop_iterate() on from_space objects, which can have a forwarding, which would mess with the object's Klass*. We're lucky because that iterator doesn't visit the Klass*. I see the following ways out:
- The caller must ensure that the oop is ok and Klass* is accessible. I could do that in the ShenandoahVerifier. It kinda defeats the point, though, we want the verifier operate on the 'raw' object, not necessarily the forwardee.
- Next easy way out would be to use 'this' instead of obj->klass(). Should makes sense, because it should always be the same. Using 'this' in the assert (this->is_array_klass()) is kinda bogus, though. And asserting (this == obj->klass()) would be nice, but would have the same problem as before where we would need to exclude UCOH for the case where Shenandoah needs it. In-fact, this is done already in oopDesc::oop_iterate_backwards(), but also excluding UCOH.
- We could add a hook in the iterator that gives the Klass* for a given oop, which can then be overridden by the actual iterator to do the right thing, e.g. load the Klass* from the forwardee.

WDYT?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1751770293