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. 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. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1785#discussion_r2614316345
