RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v9]

[Coleen Phillimore]

On Mon, 9 Sep 2024 17:45:47 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> This is the main body of the JEP 450: Compact Object Headers (Experimental).
>> 
>> It is also a follow-up to #20640, which now also includes (and supersedes) #20603 and #20605, plus the Tiny Class-Pointers parts that have been previously missing.
>> 
>> Main changes:
>>  - Introduction of the (experimental) flag UseCompactObjectHeaders. All changes in this PR are protected by this flag. The purpose of the flag is to provide a fallback, in case that users unexpectedly observe problems with the new implementation. The intention is that this flag will remain experimental and opt-in for at least one release, then make it on-by-default and diagnostic (?), and eventually deprecate and obsolete it. However, there are a few unknowns in that plan, specifically, we may want to further improve compact headers to 4 bytes, we are planning to enhance the Klass* encoding to support virtually unlimited number of Klasses, at which point we could also obsolete UseCompressedClassPointers.
>>  - The compressed Klass* can now be stored in the mark-word of objects. In order to be able to do this, we are add some changes to GC forwarding (see below) to protect the relevant (upper 22) bits of the mark-word. Significant parts of this PR deal with loading the compressed Klass* from the mark-word. This PR also changes some code paths (mostly in GCs) to be more careful when accessing Klass* (or mark-word or size) to be able to fetch it from the forwardee in case the object is forwarded.
>>  - Self-forwarding in GCs (which is used to deal with promotion failure) now uses a bit to indicate 'self-forwarding'. This is needed to preserve the crucial Klass* bits in the header. This also allows to get rid of preserved-header machinery in SerialGC and G1 (Parallel GC abuses preserved-marks to also find all other relevant oops).
>>  - Full GC forwarding now uses an encoding similar to compressed-oops. We have 40 bits for that, and can encode up to 8TB of heap. When exceeding 8TB, we turn off UseCompressedClassPointers (except in ZGC, which doesn't use the GC forwarding at all).
>>  - Instances can now have their base-offset (the offset where the field layouter starts to place fields) at offset 8 (instead of 12 or 16).
>>  - Arrays will now store their length at offset 8.
>>  - CDS can now write and read archives with the compressed header. However, it is not possible to read an archive that has been written with an opposite setting of UseCompactObjectHeaders. Some build machinery is added so that _co...
>
> 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

I reviewed the oops code so far.

src/hotspot/share/oops/compressedKlass.cpp line 116:

> 114:   _range = end - _base;
> 115: 
> 116:   DEBUG_ONLY(assert_is_valid_encoding(addr, len, _base, _shift);)

Can you refactor so the aarch64 path runs this same code without duplication?

src/hotspot/share/oops/klass.hpp line 173:

> 171: 
> 172:   markWord _prototype_header;   // Used to initialize objects' header
> 173: 

I think you should move this up after ClassLoaderData, as there might be an alignment gap (you can run pahole to check).

src/hotspot/share/oops/klass.hpp line 718:

> 716: 
> 717:   markWord prototype_header() const {
> 718:     assert(UseCompactObjectHeaders, "only use with compact object headers");

Should this unconditionally return _prototype_header since it's initialized to markWord::prototype_header(), or would that decrease performance for the non-compact headers case?

src/hotspot/share/oops/klass.inline.hpp line 54:

> 52: }
> 53: 
> 54: inline void Klass::set_prototype_header(markWord header) {

Can you put a comment that this is only used when dumping the archive?  Because otherwise the Klass::_prototype_header field should always be initialized to the right thing (either with Klass encoded or as markWord::protoytpe_header()) and doesn't change.

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?

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?

src/hotspot/share/oops/typeArrayKlass.cpp line 175:

> 173: size_t TypeArrayKlass::oop_size(oop obj) const {
> 174:   // In this assert, we cannot safely access the Klass* with compact headers.
> 175:   assert(UseCompactObjectHeaders || obj->is_typeArray(),"must be a type array");

Why not?  I think I'm missing something.  Klass should be in the markWord and that should be ok (?)

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

PR Review: https://git.openjdk.org/jdk/pull/20677#pullrequestreview-2290316150
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750529270
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750727211
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750730078
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750736547
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750739441
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750842383
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750721069