RFR: 8232722: G1 archive region deallocation may shrink the heap below -Xms

[Thomas Schatzl]

On Thu, 25 May 2023 13:51:14 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this change that properly deallocates/uncommits CDS archive regions when failing to load the CDS archive?
>> 
>> In particular this caused the nuisance mentioned in the CR where even if -Xms==-Xmx, g1 uncommitted the heap memory anyway.
>> 
>> Testing: gha, manual testing as below
>> 
>> There is no (existing) way to induce CDS load errors easily, so what I did was adding `-XX:+UseNewCode` in `filemap.cpp:2202` to simulate failures when enabled. Obviously I removed the flag in this change.
>> 
>> Here's the problematic case:
>> 
>> 
>> $java -XX:GCCardSizeInBytes=128 -Xmx128m -Xms128m -Xlog:gc+region=trace,gc+ergo+heap=debug -XX:+UseNewCode -version
>> 
>> [0.050s][trace][gc,region   ] G1HR ALLOC(OLD) [0x00000000ffe00000, 0x00000000fff00000, 0x00000000fff00000]
>> [0.050s][trace][gc,region   ] G1HR ALLOC(OLD) [0x00000000fff00000, 0x00000000fff06278, 0x0000000100000000]
>> [0.050s][trace][gc,region   ] G1HR INACTIVE(FREE) [0x00000000ffe00000, 0x00000000ffe00000, 0x00000000fff00000]
>> [0.050s][trace][gc,region   ] G1HR INACTIVE(FREE) [0x00000000fff00000, 0x00000000fff00000, 0x0000000100000000]
>> [0.050s][debug][gc,ergo,heap] Attempt heap shrinking (CDS archive regions). Total size: 2097152B
>> [0.050s][trace][gc,region   ] G1HR UNCOMMIT(FREE) [0x00000000ffe00000, 0x00000000ffe00000, 0x00000000fff00000]
>> [0.050s][trace][gc,region   ] G1HR UNCOMMIT(FREE) [0x00000000fff00000, 0x00000000fff00000, 0x0000000100000000]
>> [0.057s][trace][gc,region   ] G1HR ALLOC(EDEN) [0x00000000ffd00000, 0x00000000ffd00000, 0x00000000ffe00000]
>> [0.129s][trace][gc,region   ] G1HR ALLOC(EDEN) [0x00000000ffc00000, 0x00000000ffc00000, 0x00000000ffd00000]
>> openjdk version "21-internal" 2023-09-19
>> 
>> 
>> (The `GCCardSizeInBytes` option is there to decrease the minimum heap alignment to 512kb/1M so that setting `-Xms` to an odd value in a later test works)
>> 
>> I.e. the CDS regions are unconditionally uncommitted even through `-Xms == -Xmx`.
>> 
>> The next case just illustrates current (pre-existing) behavior with `-Xms != -Xmx`, showing that CDS regions are always committed, leading to higher than `-Xms` memory usage. I will file an enhancement here, as it is acceptable behavior (to me).
>> 
>> 
>> $ java -XX:GCCardSizeInBytes=128 -Xmx128m -Xms126m -Xlog:gc+region=trace,gc+ergo+heap=debug -XX:+UseNewCode -version
>> 
>> [0.048s][trace][gc,region   ] G1HR COMMIT(FREE) [0x00000000fff00000, 0x00000000fff00000, 0x0000000100000000]
>> [0.048s][trace][gc,region   ] G1HR AC...
>
> src/hotspot/share/gc/g1/heapRegionManager.cpp line 568:
> 
>> 566:     }
>> 567:     HeapRegion* curr_region  = _regions.get_by_index(curr_index);
>> 568:     if (!curr_region->is_free()) {
> 
> What happens with regions committed before this false return?

They will be abandoned and in a weird state... I'll file a bug (this is pre-existing). It never happened because the only case where this is called is CDS loading which happens very early during execution.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14145#discussion_r1205609317