RFR: 8305895: Implementation: JEP 450: Compact Object Headers (Experimental) [v7]

[Aleksey Shipilev]

On Wed, 10 May 2023 19:16:58 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> This is the main body of the JEP 450: Compact Object Headers (Experimental).
>> 
>> Main changes:
>>  - Introduction of the (experimental) flag UseCompactObjectHeaders. All changes in this PR are protected by this flag.
>>  - The compressed Klass* can now be stored in the mark-word of objects. In order to be able to do this, we are building on #10907, #13582 and #13779 to protect the relevant (upper 32) bits of the mark-word. Significant parts of this PR deal with loading the compressed Klass* from the mark-word, and dealing with (monitor-)locked objects. When the object is monitor-locked, we load the displaced mark-word from the monitor, and load the compressed Klass* from there. 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, and/or reach through to the monitor when the object is locked by a monitor.
>>  - The identity hash-code is narrowed to 25 bits.
>>  - 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 can now store their length at offset 8. Due to alignment restrictions, array elements will still start at offset 16. #11044 will resolve that restriction and allow array elements to start at offset 12 (except for long, double and uncompressed oops, which are still required to start at an element-aligned offset).
>>  - 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.
>> 
>> Testing:
>>  (+UseCompactObjectHeaders tests are run with the flag hard-patched into the build, to also catch @flagless tests, and to avoid mismatches with CDS - see above.)
>>  - [x] tier1 (x86_64)
>>  - [x] tier2 (x86_64)
>>  - [x] tier3 (x86_64)
>>  - [ ] tier4 (x86_64)
>>  - [x] tier1 (aarch64)
>>  - [x] tier2 (aarch64)
>>  - [x] tier3 (aarch64)
>>  - [ ] tier4 (aarch64)
>>  - [ ] tier1 (x86_64) +UseCompactObjectHeaders
>>  - [ ] tier2 (x86_64) +UseCompactObjectHeaders
>>  - [ ] tier3 (x86_64) +UseCompactObjectHeaders
>>  - [ ] tier4 (x86_64) +UseCompactObjectHeaders
>>  - [ ] tier1 (aarch64) +UseCompactObjectHeaders
>>  - [ ] tier2 (aarch64) +UseCompactObjectHeaders
>>  - [ ] tier3 (aarch64) +UseCompactObjectHeaders
>>  - [ ] tier4 (aarch64) +UseCompactObjectHeaders
>
> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 28 commits:
> 
>  - Merge branch 'JDK-8305898' into JDK-8305895
>  - @shipilev review, round 2
>  - Fix build
>  - @shipilev comments, round 1
>  - Allow to resolve mark with LW locking
>  - Use new lightweight locking with compact headers
>  - Merge branch 'JDK-8305898' into JDK-8305895
>  - Imporve GetObjectSizeIntrinsicsTest
>  - Some GC fixes
>  - Add BaseOffsets test
>  - ... and 18 more: https://git.openjdk.org/jdk/compare/39c33727...58046e58

Another quick read.

src/hotspot/cpu/aarch64/aarch64.ad line 7370:

> 7368:     __ br(Assembler::NE, stub->entry());
> 7369:     __ bind(stub->continuation());
> 7370:     __ lsr(dst, dst, markWord::klass_shift);

Like for x86, should probably go in `macroAssembler`?

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 4325:

> 4323:     ldrw(dst, Address(src, oopDesc::klass_offset_in_bytes()));
> 4324:     return;
> 4325:   }

Long-shot: maybe we should be doing `MacroAssembler::load_compact_klass` specifically for `UseCompactObjectHeaders`, and keep the legacy `UseCompressedClassPointers` paths in the callers. This would limit the exposure to the new code here.

src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp line 3542:

> 3540:     __ decode_klass_not_null(result, tmp);
> 3541:   } else
> 3542:   if (UseCompressedClassPointers) {

Suggestion:

  } else if (UseCompressedClassPointers) {

src/hotspot/share/oops/objArrayKlass.inline.hpp line 73:

> 71: template <typename T, typename OopClosureType>
> 72: void ObjArrayKlass::oop_oop_iterate(oop obj, OopClosureType* closure) {
> 73:   // We cannot safely access the Klass* with compact headers.

"// In this assert, ...

src/hotspot/share/oops/oop.inline.hpp line 88:

> 86: markWord oopDesc::resolve_mark() const {
> 87:   assert(LockingMode != LM_LEGACY, "Not safe with legacy stack-locking");
> 88:   markWord hdr = mark();

Convention `hdr` -> `m`? Everywhere in new code in this file, I think.

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

PR Review: https://git.openjdk.org/jdk/pull/13844#pullrequestreview-1422448723
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1191062237
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1191074323
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1191078669
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1191095811
PR Review Comment: https://git.openjdk.org/jdk/pull/13844#discussion_r1191097686