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

Thu Sep 26 09:07:56 UTC 2024

On Wed, 25 Sep 2024 12:53:17 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 one additional commit since the last revision:
> 
>   Allow LM_MONITOR on 32-bit platforms

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5692:

> 5690: 
> 5691: void MacroAssembler::load_klass(Register dst, Register src, Register tmp) {
> 5692:   BLOCK_COMMENT("load_klass");

I am not sure that the complexity of `MacroAssembler::load_klass` and the two `MacroAssembler::cmp_klass` functions warrant adding block comments, but if you prefer to leave them in, could you use opening and closing comments, as in the other functions in this file (e.g. `MacroAssembler::_verify_oop`)? In that case, please update the comment in the two `MacroAssembler::cmp_klass` functions with a more descriptive name than `cmp_klass 1` and `cmp_klass 2`.

src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5726:

> 5724: #ifdef _LP64
> 5725:   if (UseCompactObjectHeaders) {
> 5726:     load_nklass_compact(tmp, obj);

Suggestion: assert here that `tmp != noreg`, just like in `MacroAssembler::cmp_klass(Register src, Register dst, Register tmp1, Register tmp2)` below. Perhaps also assert that the input registers are different.

src/hotspot/cpu/x86/macroAssembler_x86.hpp line 379:

> 377:   // Uses tmp1 and tmp2 as temporary registers.
> 378:   void cmp_klass(Register src, Register dst, Register tmp1, Register tmp2);
> 379: 

The naming of these two functions could be made clearer and more consistent with their documentation. Please consider renaming the four-argument `cmp_klass` function to `cmp_klasses_from_objects` or similar. The notion of "source" and "destination" in the parameter names is unclear, I suggest to just call them `obj`, `obj1`, `obj2` etc. 
Please also make sure that the parameter names are consistent in the declaration and definition (e.g. `dst` vs `obj`).

src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 4008:

> 4006: #ifdef COMPILER2
> 4007:   if ((UseAVX == 2) && EnableX86ECoreOpts && !UseCompactObjectHeaders) {
> 4008:     generate_string_indexof(StubRoutines::_string_indexof_array);

This stub routine should be re-enabled if `UseCompactObjectHeaders` is to become non-experimental and enabled by default in the future. Is there a RFE for this task?

src/hotspot/share/opto/memnode.cpp line 1976:

> 1974:       // The field is Klass::_prototype_header.  Return its (constant) value.
> 1975:       assert(this->Opcode() == Op_LoadX, "must load a proper type from _prototype_header");
> 1976:       return TypeX::make(klass->prototype_header());

This code is dead, because by the time we call `load_array_final_field` from `LoadNode::Value` (its only caller) we know that if `UseCompactObjectHeaders`, then `tkls->offset() != in_bytes(Klass::prototype_header_offset()` (or else we would have returned from line 2161). Please remove it, or replace it with an assertion if you prefer.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1776676785
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1776628929
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1776644021
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1776663594
PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1776621766