On Tue, 8 Sep 2026 06:36:47 GMT, Yunbo Zhang <[email protected]> wrote:
>> Please review this small fix.
>>
>> **Problem:**
>>
>> G1 only supports region sizes from 1 MiB up to 32 MiB on 32-bit platforms or
>> 512 MiB on 64-bit platforms. However, an invalid value such as `16` is
>> silently adjusted to 1 MiB before the constraint is checked:
>>
>>
>> $ java -XX:G1HeapRegionSize=16 -XX:+PrintFlagsFinal -version | grep
>> G1HeapRegionSize
>> size_t G1HeapRegionSize = 1048576
>> {product} {command line, ergonomic}
>> openjdk version "28-internal" 2027-03-23
>> OpenJDK Runtime Environment (build 28-internal-adhoc.zhangyunbo)
>> OpenJDK 64-Bit Server VM (build 28-internal-adhoc.zhangyunbo, mixed mode,
>> sharing)
>>
>>
>> **Fix:**
>>
>> - Move the `G1HeapRegionSize` constraint from `AfterMemoryInit` to
>> `AfterErgo`, so it is checked after GC selection but before the value is
>> adjusted during heap initialization.
>> - Keep `0` as the valid ergonomic value and reject non-zero values below 1
>> MiB.
>> - Update `MemoryManagement.java` to use the valid minimum value `1m` instead
>> of relying on an invalid value being adjusted.
>>
>> **Testing:**
>>
>> - gc/arguments：47 passed, 0 failed, 2 skipped
>> - tier1:all pass
>> - MemoryManagement.java & TestG1HeapRegionSize.java: pass
>>
>>
>> $ java -XX:G1HeapRegionSize=16 -version
>> G1HeapRegionSize (16) must be greater than or equal to ergonomic heap region
>> minimum size
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>>
>>
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Yunbo Zhang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8311029: Print minimum G1HeapRegionSize in constraint error
Good except for that typo.
src/hotspot/share/gc/g1/g1_globals.hpp line 270:
> 268: "Size of the G1 regions.")
> \
> 269: range(0, NOT_LP64(32*M) LP64_ONLY(512*M))
> \
> 270: constraint(G1HeapRegionSizeConstraintFunc,AfterErgo)
> \
Suggestion:
constraint(G1HeapRegionSizeConstraintFunc, AfterErgo) \
src/hotspot/share/gc/g1/jvmFlagConstraintsG1.cpp line 88:
> 86: "G1HeapRegionSize (%zu) must be "
> 87: "greater than or equal to ergonomic heap region
> minimum size (%zu)\n",
> 88: value, G1HeapRegionBounds::min_size());
Maybe convert this to MB? The minimum is always supposed to be a multiple of
MBs, and the endlessly long in-bytes number is awkward to use.
Suggestion:
"greater than or equal to ergonomic heap region minimum
size (%zuM)\n",
value, G1HeapRegionBounds::min_size() / M);
(Did not check if that M value is in scope).
-------------
Changes requested by tschatzl (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/32749#pullrequestreview-5167099898
PR Review Comment: https://git.openjdk.org/jdk/pull/32749#discussion_r3979074091
PR Review Comment: https://git.openjdk.org/jdk/pull/32749#discussion_r3979187616
