On Fri, 12 May 2023 18:03:13 GMT, Coleen Phillimore <cole...@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
>
> 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.

Yeah, we do have code in arguments.cpp that turns off CDS if the wrong setting 
is used (i.e. the opposite of the default setting). If you hard-code 
UseCompactObjectHeaders to be true, then the archives will be written in the 
compact layout, and can be read. That's what the changes in share/cds 
implement. (Note: I regularily hard-patch the UseCompactObjectHeaders flag to 
be true for testing, because that also catches all the @flagless tests, and I 
know that Daniel does that, too.)
We disabled CDS jtreg tests when passing +UseCompactObjectHeaders via cmd line, 
because we would see a lot of test failures because of archive format mismatch. 
I am not sure if that's a good way to deal with that.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13961#discussion_r1192698676

Reply via email to