RFR: 8354145: G1GC: keep the CompressedOops same as before when not setting HeapRegionSize explicitly [v2]

[Leonid Mesnik]

On Mon, 14 Apr 2025 13:16:24 GMT, Tongbao Zhang <duke at openjdk.org> wrote:

>> After [JDK-8275056](https://bugs.openjdk.org/browse/JDK-8275056), The max heap region size became 512M, and the calculation of CompressedOops based on the max_heap_size - max_heap_region_size.
>> So before this patch, the CompressedOops will turn on below 32G - 32m, After this patch is 32G -512m.
>> 
>> When our Apps migrating from JDK11 to JDK21, the heap size parameters(Xmx32736m) will turn off the CompressedOops.
>> 
>> Since the current max ergonomics size is still 32m, We hoped that the original behavior will not be changed if HeapRegionSize is not explicitly set.
>> 
>> before this patch:
>> 
>> ./build/linux-x86_64-server-release/images/jdk/bin/java -Xmx32736m -XX:+PrintFlagsFinal -version | grep CompressedOops
>>      bool UseCompressedOops                        = false                          {product lp64_product} {default}
>> openjdk version "25-internal" 2025-09-16
>> OpenJDK Runtime Environment (build 25-internal-adhoc.root.jdk)
>> OpenJDK 64-Bit Server VM (build 25-internal-adhoc.root.jdk, mixed mode, sharing)
>> 
>> 
>> after this patch:
>> 
>> ./build/linux-x86_64-server-release/images/jdk/bin/java -Xmx32736m -XX:+PrintFlagsFinal -version | grep CompressedOops
>>      bool UseCompressedOops                        = true                           {product lp64_product} {ergonomic}
>> openjdk version "25-internal" 2025-09-16
>> OpenJDK Runtime Environment (build 25-internal-adhoc.root.jdk)
>> OpenJDK 64-Bit Server VM (build 25-internal-adhoc.root.jdk, mixed mode, sharing)
>
> Tongbao Zhang has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   G1GC: keep the CompressedOops same as before when not setting HeapRegionSize explicitly

Marked as reviewed by lmesnik (Reviewer).

Sorry, I wanted to ask you to change test, not approve it yet.

test/hotspot/jtreg/gc/arguments/TestG1CompressedOops.java line 30:

> 28:  * @test TestG1CompressedOops
> 29:  * @bug 8354145
> 30:  * @requires vm.gc.G1 & vm.opt.G1HeapRegionSize == null

The test ignores external VM flags, so vm.opt.G1HeapRegionSize is not needed.
But it is needed to add
`* @requires vm.flagless`

test/hotspot/jtreg/gc/arguments/TestG1CompressedOops.java line 32:

> 30:  * @requires vm.gc.G1 & vm.opt.G1HeapRegionSize == null
> 31:  * @summary Verify that the flag TestG1CompressedOops is updated properly
> 32:  * @modules java.base/jdk.internal.misc

Is any of those 2 modules is used by tests? I don't see it in the test.

test/hotspot/jtreg/gc/arguments/TestG1CompressedOops.java line 35:

> 33:  * @modules java.management/sun.management
> 34:  * @library /test/lib
> 35:  * @library /

Why this line is needed? I don't see any dependencies on "/"
If you use some test code outside directory, better to build them.

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

PR Review: https://git.openjdk.org/jdk/pull/24541#pullrequestreview-2766273464
Changes requested by lmesnik (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24541#pullrequestreview-2766313637
PR Review Comment: https://git.openjdk.org/jdk/pull/24541#discussion_r2043328713
PR Review Comment: https://git.openjdk.org/jdk/pull/24541#discussion_r2043315584
PR Review Comment: https://git.openjdk.org/jdk/pull/24541#discussion_r2043311695