On Thu, 29 Aug 2024 09:28:50 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fix bit counts in GCForwarding
>
> src/hotspot/share/cds/archiveHeapWriter.cpp line 214:
> 
>> 212:       oopDesc::set_mark(mem, markWord::prototype());
>> 213:       oopDesc::release_set_klass(mem, k);
>> 214:     }
> 
> The `UseCompactObjectHeaders` path calls `get_requested_narrow_klass`, while 
> the `else` part directly uses `k`. Is one of these paths incorrect?

This seems odd. The original code sets `Universe::objectArrayKlass()` into the 
object header. This is the value of this class in the current JVM lifetime.

Later, `ArchiveHeapWriter::update_header_for_requested_obj()` would change the 
object's klass to the "requested" address. I.e., where this class will be 
loaded in a future JVM lifetime when the CDS archive is loaded into memory.

It seems the same logic should be used in the `UseCompactObjectHeaders==true` 
case.

BTW (unrelated to this PR) the comment a few lines up is outdated and wrong:


Klass* k = Universe::objectArrayKlass(); // already relocated to point to 
archived klass


`k` is the value of the *actual* location of this class in the current JVM 
lifetime. Please ignore this comment when trying to understand this function.

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

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

Reply via email to