On Fri, 12 Dec 2025 14:02:17 GMT, Joel Sikström <[email protected]> wrote:

>> src/hotspot/share/gc/parallel/psParallelCompact.cpp line 1455:
>> 
>>> 1453:   precond(words > 0);
>>> 1454: 
>>> 1455:   const bool safe = words >= (size_t)oopDesc::header_size();
>> 
>> I don't know what this means.  Does header_size() include the markWord and 
>> klass pointer? ie 2 words for !UseCompactObjectHeaders? 
>> Is it safe if the number of words includes a klass pointer?
>> Can you enclose EnableValhalla in this method rather than asking in these 
>> callers?
>
> Yes, header_size() includes both markWord and Klass* and is 2 words for 
> !UseCOH and 1 word for UseCOH. The header is safe to read if we have all of 
> it, i.e. the full size. Otherwise it is not safe to read the Klass*, as it 
> could be it the second word.
> 
> I'm not totally opposed to move the EnableValhalla check, but what we have 
> right now is pretty similar to other places working with markWord's, like 
> oopDesc::init_mark(), which I think is a plus. 
> 
> Also, moving the EnableValhalla check inside safe_to_read_header should give 
> it another name, like (very explicit name) "need_to_read_header_and_safe". 
> Keeping it the way it is allows a much easier refactor in the future once the 
> EnableValhalla check is gone.

FYI, I accidentally sent my last comment twice and I deleted the other one so 
the mail-link got broken :)

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1785#discussion_r2615133024

Reply via email to