On Mon, 9 Feb 2026 21:33:37 GMT, Thomas Stuefe <[email protected]> wrote:

>> _This patch is not intended for JDK 26_. 
>> 
>> I'm posting it now to collect feedback and, barring any objections, plan to 
>> push it once JDK 27 opens.
>> 
>> This change removes the uncompressed Klass pointer mode and, with compressed 
>> Klass pointers remaining as the only option, the 
>> `UseCompressedClassPointers` switch.
>> 
>> For motivation, please take a look at CSR associated with the deprecation 
>> (which we did for JDK 25) and the preparatory discussion we had at the start 
>> of the year around this topic [2].
>> 
>> This patch is quite invasive and touches many parts of the JVM, since its 
>> goal is to remove most traces of the uncompressed Klass path and to take 
>> advantage of opportunities for simplification. In some cases, I did not take 
>> opportunities for further simplification to keep the patch somewhat legible; 
>> it will be onerous enough to review.
>> 
>> ### Implementation Notes
>> 
>> With uncompressed Klass pointers removed, we have three modes of operation 
>> left (including 32-bit):
>>   a) 64-bit, COH off - this is the old `+UseCompressedClassPointers` mode. 
>> This is now the standard mode until we run with COH by default.
>>   b) 64-bit, COH on
>>   c) 32-bit - Here, we run with a "fake" narrow Klass pointer mode. We run 
>> with hardcoded narrowKlass base == NULL and shift = 0, so nKlass == Klass*. 
>> The difference to (a, b) is that we don't use a class space. This was 
>> implemented with JDK-8363998 [3] - for more details, please see that issue 
>> and its PR.
>> 
>> I ensured *arm32* builds and I performed some rudimentary checks (selected 
>> metaspace/gc tests, and a simple Spring PetClinic run). Vendors with an 
>> interest in arm32 will have to step up and do their own, more thorough unit 
>> testing. Also, I did not see anyone doing follow-up work after JDK-8363998 
>> [3] - so some issues may still lurk from that patch as well (but maybe 
>> JDK-8363998 was just not breaking anything).
>> 
>> I did not check *zero 32-bit*, the only other platform supporting 32-bit. 
>> Anyone with an interest in 32-bit zero should chip in.
>>   
>> Pre-existing errors: While working on this patch, I stumbled over a few 
>> occurrences of old but benign bugs. Mostly old code assuming 
>> CompressedClassPointers and CompressedOops were still tied together 
>> (example: Arguments::set_heap_size()). These bugs are implicitly fixed with 
>> this patch.
>> 
>> ### Testing
>> 
>> - tier 1 2 3 locally on Linux x64
>> - SAP ran their whole set of tests for all the platforms they support.
>> 
>> 
>> [1] https://bugs.openjdk.org/browse/JDK-8350754
>> [2...
>
> Thomas Stuefe has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 62 commits:
> 
>  - Replace Klass::_metadata union with narrowKlass member
>  - Ivan: fix various instances of ObjLayout::undefined should assert
>  - Ivan: Update src/hotspot/share/oops/instanceKlass.cpp
>    
>    Co-authored-by: Ivan Walulya <[email protected]>
>  - David: reduce diff in ObjectCountEventVerifier.java
>  - David: minimize change in GetObjectSizeIntrinsicsTest.java
>  - David: minimize diffs in TestZGCWithCDS.java
>  - David: minimize diffs for 
> runtime/ErrorHandling/TestVMConfigInHsErrFile.java
>  - David: revert type change in arguments.cpp
>  - David: fix comment in metaspace
>  - Merge branch 'master' into JDK-8363996-Obsolete-UseCompressedClassPointers
>  - ... and 52 more: https://git.openjdk.org/jdk/compare/ffb6279c...642604ac

A couple of minor follow up comments below, but otherwise this all seems okay. 
Thanks

src/hotspot/cpu/x86/macroAssembler_x86.hpp line 354:

> 352:   void store_klass(Register dst, Register src, Register tmp);
> 353: 
> 354:   // Compares the narrow Klass pointer of an object to a given narrow 
> Klass (which might be narrow

Suggestion:

  // Compares the narrow Klass pointer of an object to a given narrow Klass.

??

test/jdk/jdk/jfr/event/gc/objectcount/ObjectCountEventVerifier.java line 73:

> 71:         boolean runsOn32Bit = 
> System.getProperty("sun.arch.data.model").equals("32");
> 72:         int bytesPerWord = runsOn32Bit ? 4 : 8;
> 73:         int objectHeaderSize = runsOn32Bit ? 12 : 16;

This seems a separate bug fix. ??

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

PR Review: https://git.openjdk.org/jdk/pull/28366#pullrequestreview-3776771990
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2785847854
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2785905985

Reply via email to