On Fri, 12 May 2023 17:27:25 GMT, Roman Kennke <rken...@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
These changes are an improvement.
src/hotspot/cpu/x86/c1_MacroAssembler_x86.cpp line 193:
> 191: movl(Address(obj, arrayOopDesc::length_offset_in_bytes() +
> sizeof(jint)), t1);
> 192: }
> 193: #endif
This endif should go after UseCompressedClassPointers conditional, and
consolidate to one set of #ifdef _LP64.
src/hotspot/cpu/x86/macroAssembler_x86.cpp line 5126:
> 5124: assert(UseCompactObjectHeaders, "expect compact object headers");
> 5125:
> 5126: if (!UseCompactObjectHeaders) {
Now this isn't needed, right?
src/hotspot/share/cds/archiveBuilder.cpp line 726:
> 724:
> k->set_prototype_header(markWord::prototype().set_narrow_klass(nk));
> 725: }
> 726: #endif //_LP64
If CDS is turned off for UseCompactObjectHeaders, I don't understand this
change or the one to archiveHeapWriter. -Xshare:dump objects would be the
wrong size. If CDS is not supported, then there should be something in
arguments.cpp that gives an error for that. And write a test for that error of
mixing and matching.
-------------
PR Review: https://git.openjdk.org/jdk/pull/13961#pullrequestreview-1424925882
PR Review Comment: https://git.openjdk.org/jdk/pull/13961#discussion_r1192648245
PR Review Comment: https://git.openjdk.org/jdk/pull/13961#discussion_r1192646637
PR Review Comment: https://git.openjdk.org/jdk/pull/13961#discussion_r1192661859
