On Mon, 9 Sep 2024 18:30:21 GMT, Coleen Phillimore <[email protected]> wrote:
>> Roman Kennke has updated the pull request incrementally with six additional
>> commits since the last revision:
>>
>> - Print as warning when UCOH doesn't match in CDS archive
>> - Improve initialization of mark-word in CDS ArchiveHeapWriter
>> - Simplify getKlass() in SA
>> - Simplify oopDesc::init_mark()
>> - Get rid of forward_safe_* methods
>> - GCForwarding touch-ups
>
> src/hotspot/share/oops/markWord.inline.hpp line 90:
>
>> 88: ShouldNotReachHere();
>> 89: return markWord();
>> 90: #endif
>
> Is the ifdef _LP64 necessary, since UseCompactObjectHeaders should always be
> false for 32 bits?
Kindof. The problem is that klass_shift is larger than 31, and shifting with it
would thus be UB and generate a compiler warning. I opted to simply not compile
any of that code in 32bit builds. We could also define klass_shift differently
on 32bit.
Long-term (maybe with Lilliput2/4-byte-headers?) it would be nice to
consolidate the header layout between 32 and 64 bit builds and not make any
distinction anywhere. E.g. define markWord (or objectHeader?) in a single way,
and use that to extract all the relevant stuff. It's not totally unlikely that
we deprecate 32-bit builds before that can happen, though.
> src/hotspot/share/oops/oop.inline.hpp line 90:
>
>> 88: } else {
>> 89: return markWord::prototype();
>> 90: }
>
> Could this be unconditional since prototoype_header is initialized for all
> Klasses?
yes, but there is ongoing effort (at Oracle) to get rid of
```Klass::_prototype_header``` altogether. Let's wait for that and see how it
looks then.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1765003983
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1765006669
