RFR: 8305895: Implement JEP 450: Compact Object Headers (Experimental) [v30]

[Roman Kennke]

On Mon, 7 Oct 2024 08:44:16 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Roman Kennke has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 76 commits:
>> 
>>  - Merge remote-tracking branch 'rkennke/JDK-8305895-v4' into JDK-8305895-v4
>>  - Revert "Disable TestSplitPacks::test4a, failing on aarch64"
>>    
>>    This reverts commit 059b1573db26d1d2902ca6dadc8413f445234c2a.
>>  - Simplify object init code in interpreter
>>  - Disable some vectorization tests that fail with +UCOH and UseSSE<=3
>>  - Fix for CDSPluginTest.java
>>  - Merge tag 'jdk-24+18' into JDK-8305895-v4
>>    
>>    Added tag jdk-24+18 for changeset 19642bd3
>>  - Disable TestSplitPacks::test4a, failing on aarch64
>>  - @robcasloz review comments
>>  - Improve CollectedHeap::is_oop()
>>  - Allow LM_MONITOR on 32-bit platforms
>>  - ... and 66 more: https://git.openjdk.org/jdk/compare/19642bd3...8742f3c1
>
> src/hotspot/share/oops/markWord.cpp line 35:
> 
>> 33: STATIC_ASSERT(markWord::klass_shift + markWord::klass_bits == 64);
>> 34: // The hash (preceding nKlass) shall be a direct neighbor but not interleave
>> 35: STATIC_ASSERT(markWord::klass_shift == markWord::hash_bits + markWord::hash_shift);
> 
> The code is not consistent in it usage of the name for the klass bits. Here it says `nKlass` in the comment, but the fields are named `klass`. Maybe just change the comment to says `(preceding klass bits)`.
> 
> Note that the term `nklass` is not prevalent in the code base, but with this patch its starting to get a foot hold. It might be good to figure out what we do want to call these in field names and variables to at least a little bit more consistency in the code base. Currently we have `nklass`, `nKlass` `nk`, `narrow_klass`.
> 
>  In other places we have functions that are named `set_narrow_klass`, but the field is called `nklass` and other places call it `nk`. It would be good to stay consistent with the naming. FWIW, nklass has very little precedence in the code, so cleaning that away might be easiest.thing is to clean out all usages of nklass, because it isn't a

I renamed all occurrences of nklass and nKlass in shared code to something more useful. I left load_nklass* stuff in aarch64 and x86 code alone for now. Let me know if that addresses your concern.

https://github.com/openjdk/jdk/pull/20677/commits/1ab207746e4c4baaa6da162d7c1535c75342fa2e

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1790270819