[master] RFR: JDK-8325104: Lilliput: Shrink Classpointers [v2]

Thomas Stuefe stuefe at openjdk.org
Sat Feb 10 14:55:15 UTC 2024


On Wed, 7 Feb 2024 18:49:36 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
>> 
>>  - Merge branch 'master' into Smaller-ClassPointers
>>  - Merge
>>  - Merge commit 'c1281e6b45ed167df69d29a6039d81854c145ae6~1' into Smaller-ClassPointers
>>  - Fix Typo
>>  - Better CDS arch generation
>>  - Fix error in COH archive generation
>>  - Fixes
>>  - Fix Windows-Only bug that was caused by !INCLUDE_CDS_JAVA_HEAP
>>  - fix runtime/cds/appcds/TestCombinedCompressedFlags and runtime/cds/appcds/CommandLineFlagComboNegative.java
>>  - Fix CdsDifferentCompactObjectHeaders
>>  - ... and 6 more: https://git.openjdk.org/lilliput/compare/da4ffa35...f665504c
>
> src/hotspot/cpu/x86/sharedRuntime_x86.cpp line 80:
> 
>> 78:   // because it could be larger than 32 bits in a 64-bit vm. See markWord.hpp.
>> 79:   if (UseCompactObjectHeaders) {
>> 80:     STATIC_ASSERT(markWord::hash_mask_compact < nth_bit(32));
> 
> This makes me think if the i-hash should go/stay in its original position, and we could get rid of hash_code_compact and related changes in Lilliput? Or would that not work for some reason?

It would, but we would have one unused bit in the front, three in the back. I am not sure that is an improvement.

> src/hotspot/share/cds/archiveBuilder.cpp line 642:
> 
>> 640:     // Allocate space for the future InstanceKlass with proper alignment
>> 641: #ifndef _LP64
>> 642:     const size_t al = SharedSpaceObjectAlignment;
> 
> 'al' seems a weird variable name ('al Bundy'? ;-) ). Maybe 'align' or 'alignment'?

okay, align it is :)

> src/hotspot/share/gc/shared/c2/barrierSetC2.cpp line 661:
> 
>> 659:   if (base_off % BytesPerLong != 0) {
>> 660:     assert(UseCompressedClassPointers, "");
>> 661:     assert(!UseCompactObjectHeaders, "");
> 
> What is that for? Is it important?

Mostly for documentation. The various asserts like this helped me understand a lot of code.

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

PR Review Comment: https://git.openjdk.org/lilliput/pull/128#discussion_r1485148600
PR Review Comment: https://git.openjdk.org/lilliput/pull/128#discussion_r1485149490
PR Review Comment: https://git.openjdk.org/lilliput/pull/128#discussion_r1485151455


More information about the lilliput-dev mailing list