RFR: 8363996: Obsolete UseCompressedClassPointers [v4]
David Holmes
dholmes at openjdk.org
Mon Dec 8 05:46:11 UTC 2025
On Thu, 4 Dec 2025 13:36:43 GMT, Thomas Stuefe <stuefe at openjdk.org> 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 incrementally with one additional commit since the last revision:
>
> Update src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp
>
> Co-authored-by: Andrew Haley <aph-open at littlepinkcloud.com>
I've completed a pass through everything and have a couple of comments on the functional changes.
The tests were quite hard to follow in places and I've flagged a few where you seemed to have done a more extensive rewrite than what was actually needed for the obsoletion - making it hard to assess the changes.
Thanks
src/hotspot/share/memory/metaspace.cpp line 661:
> 659: MaxMetaspaceSize = MAX2(MaxMetaspaceSize, commit_alignment());
> 660:
> 661: if (using_class_space()) {
Shouldn't this now just be a build-time `ifdef _LP64` check?
src/hotspot/share/memory/metaspace.cpp line 728:
> 726: }
> 727:
> 728: // a) if CDS is active (runtime, Xshare=on), it will create the class space
You have left descriptions of a) and b) but now we have no idea what they are referring to. ???
src/hotspot/share/runtime/arguments.cpp line 1591:
> 1589:
> 1590: size_t heap_end = HeapBaseMinAddress + MaxHeapSize;
> 1591: size_t max_coop_heap = max_heap_for_compressed_oops();
Please don't change the types in this PR.
test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointers.java line 294:
> 292: }
> 293:
> 294: public static void heapBaseMinAddressTestNoCoop() throws Exception {
It is not obvious to me why this test case was deleted.
test/hotspot/jtreg/runtime/ErrorHandling/TestVMConfigInHsErrFile.java line 59:
> 57: switch (args[0]) {
> 58: case "coh-on" -> testCompactObjectHeaders();
> 59: case "coh-off" -> testCompressedClassPointers();
You seem to have done a complete test rewrite here and it is not obvious to me it follows as part of the obsoletion of UCCP.
test/hotspot/jtreg/runtime/cds/appcds/TestCombinedCompressedFlags.java line 67:
> 65: private void initExecArgs() {
> 66: // We fail this test if the UseCompressedOops setting used at dumptime differs from runtime,
> 67: // succeed if it is identical
This test seems to reduce to pretty much nothing. ??
test/hotspot/jtreg/runtime/cds/appcds/TestZGCWithCDS.java line 44:
> 42: private final static String UNABLE_TO_USE_ARCHIVE = "Unable to use shared archive.";
> 43: private final static String ERR_MSG = "The saved state of UseCompressedOops (0) is different from runtime (1), CDS will be disabled.";
> 44: private final static String COMPACT_OBJECT_HEADERS = "-XX:+UseCompactObjectHeaders";
Again this test seems to have a lot of unrelated changes. This is obscuring the actual differences and makes the review harder.
test/jdk/java/lang/instrument/GetObjectSizeIntrinsicsTest.java line 318:
> 316:
> 317: // Includes length, excludes alignment gap to base
> 318: static final int ARRAY_HEADER_SIZE = Platform.is64bit() ? 16 : 12;
Again unrelated change.
test/jdk/jdk/jfr/event/gc/objectcount/ObjectCountEventVerifier.java line 75:
> 73: final int bytesPerWord = runsOn32Bit ? 4 : 8;
> 74: final int objectHeaderSize = runsOn32Bit ? 12 : 16;
> 75: final int alignmentGap = runsOn32Bit ? 4 : 0;
Again unrelated changes.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28366#pullrequestreview-3550086639
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597013211
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597014493
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597023027
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597045861
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597050708
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597102266
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597107079
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597110703
PR Review Comment: https://git.openjdk.org/jdk/pull/28366#discussion_r2597114451
More information about the core-libs-dev
mailing list