RFR: 8305895: Implementation: JEP 450: Compact Object Headers (Experimental) [v3]

[Roman Kennke]

On Wed, 10 May 2023 11:13:30 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Allow to resolve mark with LW locking
>
> src/hotspot/share/gc/parallel/psOldGen.cpp line 398:
> 
>> 396: 
>> 397:   virtual void do_object(oop obj) {
>> 398:     HeapWord* test_addr = cast_from_oop<HeapWord*>(obj);
> 
> I thought this `+1` is specifically to test that `object_start` is able to find the object header when given the interior pointer. See the `guarantee`-s in the next lines.

Yes, but with compact headers, we could now have 1-word-sized objects, in which case this would fail. I am not sure how to deal with that, TBH. Maybe do the whole test only when !UseCompactObjectHeaders or when object-size is > 1?

> src/hotspot/share/gc/shenandoah/shenandoahHeap.inline.hpp line 326:
> 
>> 324:   oop copy_val = cast_to_oop(copy);
>> 325:   if (!copy_val->mark().is_marked()) {
>> 326:     // If we copied a mark-word that indicates 'forwarded' state, then
> 
> Ouch. This is only the problem with `UseCompactObjectHeaders`, right? Can additionally conditionalize on that, so that legacy code path stays the same.

I'm actually thinking to maybe upstream these parts separately? Because it seems a nice improvement to not even try to relativize a stack-chunk if object is not reachable anyway?

> src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp line 309:
> 
>> 307:     _loc = obj;
>> 308:     Klass* klass = obj->forward_safe_klass();
>> 309:     obj->oop_iterate_backwards(this, klass);
> 
> Why `backwards`?

Because that's the only oop_iterate() variant that takes a Klass* :-) It seems trivial to add such a forward-variant, though. Prefer that?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1189990477
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1189993638
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1189994714