On Fri, 12 Dec 2025 18:06:10 GMT, Joel Sikström <[email protected]> wrote:
>> 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 :) If you move EnableValhalla to inside safe_to_read_header(), then we'll only have to fix it once when it's only set when Arguments::is_valhalla_enabled() is set. Right now EnablePreview is true by default. EnableValhalla is a property that determines the answer to safe_to_read_header(), so I think it should go there. Can you add a comment to safe_to_read_header() - the second sentence to the first paragraph above? ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1785#discussion_r2615336124
