RFR: 8357445: G1: Time-Based Heap Uncommit During Idle Periods [v7]
Monica Beckwith
mbeckwit at openjdk.org
Tue Oct 7 21:01:39 UTC 2025
On Tue, 2 Sep 2025 09:38:01 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:
>> Monica Beckwith has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Remove unused static _uncommit_delay member and accessor
>
> src/hotspot/share/gc/g1/g1VMOperations.cpp line 175:
>
>> 173:
>> 174: void VM_G1ShrinkHeap::doit() {
>> 175: _g1h->shrink(_bytes);
>
> I strongly believe that the shrink amount needs to be re-evaluated in the shrink pause again (ideally it would not need a VM operation, but I think this is not necessary for now). Between posting the VM operation and actual execution there can be any number of more (GC) VM operations that can change the shape of the heap, so the amount of necessary shrinking can change substantially.
>
> Also, there needs to be some synchronization between the selection of regions made during determining that we should do this and the actual shrinking.
>
> There is the risk that the region selection bases its decision based on e.g. region A, B, C being too old, and then uncommits regions D, E, and F here. Next time the idleness will be evaluated, regions A, B, and C will be found again as too old, so keeping on shrinking until eventually A, B, and C are uncommitted.
>
> However the original goal of the method would have been to _only_ uncommit A, B and C. Which means this mechanism uncommitted way too many regions.
I have added execution-time handling that uses a pre-evaluated shrink amount but performs fresh region selection at VM operation execution time:
`_g1h->shrink_with_time_based_selection(_bytes);`
At execution time, we do the calculation/fresh eval by clearing the cache and applying the time-based criteria:
void G1CollectedHeap::shrink_with_time_based_selection(size_t shrink_bytes) {
// Fresh region scanning and validation at VM operation execution time
_hrm.remove_all_free_regions();
shrink_helper_with_time_based_selection(aligned_shrink_bytes);
rebuild_region_sets(true);
}
The actual region selection:
`num_regions_removed = _hrm.shrink_by(num_regions_to_remove, true /* use_time_based_selection */);`
This should address the stale region selection concern (A,B,C → D,E,F) by ensuring region eligibility is determined fresh at VM operation execution time rather than using potentially outdated information from the scheduling phase.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26240#discussion_r2411912340
More information about the hotspot-dev
mailing list