On Mon, 9 Sep 2024 11:55:52 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 two additional
> commits since the last revision:
>
> - Try to avoid lea in loadNklass (aarch64)
> - Fix release build error
src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp line 147:
> 145: #endif
> 146:
> 147: return true;
This should only be in the compressedKlass.cpp file.
src/hotspot/share/oops/compressedKlass.cpp line 214:
> 212: ss.print("Class space size (%zu) exceeds the maximum possible size
> (%zu)",
> 213: len, max_encoding_range_size());
> 214: vm_exit_during_initialization(ss.base());
Why does this exit and not turn off compressed klass pointers and compact
object headers?
src/hotspot/share/oops/compressedKlass.cpp line 222:
> 220: return;
> 221: }
> 222: #endif
Why not add null pd_initialize to zero to remove this conditional code?
src/hotspot/share/oops/compressedKlass.cpp line 224:
> 222: #endif
> 223:
> 224: if (tiny_classpointer_mode()) {
I kind of agree with Thomas Schatzl for this. Maybe it should be
compact_classpointer_mode(). It's nice to have a new string for grep, but
they're not really that tiny.
src/hotspot/share/oops/compressedKlass.cpp line 234:
> 232: _range = len;
> 233:
> 234: constexpr int log_cacheline = 6;
Is 6 the log of DEFAULT_CACHE_LINE_SIZE?
src/hotspot/share/oops/compressedKlass.cpp line 243:
> 241: } else {
> 242:
> 243: // In legacy mode, we try, in order of preference:
Can you not use the word 'legacy' here? Maybe in "non-compact object header
mode"...
src/hotspot/share/oops/compressedKlass.inline.hpp line 100:
> 98: check_valid_klass(k, base(), shift());
> 99: // Also assert that k falls into what we know is the valid Klass range.
> This is usually smaller
> 100: // than the encoding range (e.g. encoding range covers 4G, but we only
> have 1G class space and a
1G is the default CompressedClassSpaceSize but can be larger, right? So the
comment isn't quite accurate. Or with tiny class pointers can it only be 1G?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750527537
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750511912
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750513660
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750515923
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750520712
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750524690
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1750662637
