[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