On Thu, 22 Aug 2024 18:18:01 GMT, Roman Kennke <rken...@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 one additional 
> commit since the last revision:
> 
>   Fix hash shift for 32 bit builds

src/hotspot/share/gc/shared/gcForwarding.cpp line 37:

> 35:   size_t max_narrow_heap_size = right_n_bits(NumLowBitsNarrow - Shift);
> 36:   if (UseCompactObjectHeaders && max_heap_size > max_narrow_heap_size * 
> HeapWordSize) {
> 37:     FLAG_SET_DEFAULT(UseCompactObjectHeaders, false);

Maybe a log-info/warning would be nice.

src/hotspot/share/gc/shared/gcForwarding.hpp line 36:

> 34:  * Implements forwarding for the full-GCs of Serial, Parallel, G1 and 
> Shenandoah in
> 35:  * a way that preserves upper N bits of object mark-words, which contain 
> crucial
> 36:  * Klass* information when running with compact headers. The encoding is 
> similar to

This doc suggests this forwarding is only for compact-header so I wonder if we 
can check `UseCompactObjectHeaders` directly instead of heap-size in 
`GCForwarding::initialize`.

src/hotspot/share/gc/shared/gcForwarding.hpp line 40:

> 38:  * heap-base, shifts that difference into the right place, and sets the 
> lowest two
> 39:  * bits (to indicate 'forwarded' state as usual).
> 40:  */

> "can use 40 bits for forwardee encoding. That's enough for 8TB of heap."

I feel this 8T-constraint is significant and should be in the doc.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1727708193
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1727727638
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1727732496

Reply via email to