RFR: 8357445: G1: Time-Based Heap Uncommit During Idle Periods [v2]

[Monica Beckwith]

On Wed, 16 Jul 2025 18:20:30 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Monica Beckwith has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Normalize encoding and line endings for G1 GC time-based tests
>
> src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2962:
> 
>> 2960:   assert(alloc_region->is_eden(), "all mutator alloc regions should be eden");
>> 2961: 
>> 2962:   alloc_region->record_activity();  // Record the activity of the alloc region
> 
> Since we only uncommit free regions, we only need to `record_activity` when the region is freed which is done in `G1HeapRegion::hr_clear` before the region is added to the free-list

Thanks for pointing this out—you're right, and to clarify, the logic explicitly ensures that we only ever uncommit free regions. When I designed this, I tracked both allocation activities and retirements to record region idle times as accurately and minimally as possible.

For precise time-based tracking, I believe we need to update the timestamp at both `hr_clear()` and retirement. Here’s the timing consideration:

**The timing issue:**
1. Region cleared → `hr_clear()` → `record_activity()` → sets timestamp = T1
2. Region is allocated and used for extended period
3. Region is retired → if we only update the timestamp at clear, not at retirement, there’s no new timestamp
4. Time-based evaluation → sees timestamp T1 (from clear, not retire)
5. Result: Region appears "idle" since T1, even though it was just recently retired

So, we need `record_activity()` at both lifecycle points:

- **`hr_clear()`**: When a region becomes available (initialization)
- **`retire_*()`**: When a region finishes active use (retirement)

This way we always track the **most recent activity** for each region, whether that was clearing or retirement, giving us accurate idle time calculations for time-based uncommit decisions.

Since the logic guarantees we only uncommit regions that are truly free and have been idle for the configured delay, having an accurate “last active” timestamp is critical for the feature’s effectiveness.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26240#discussion_r2211619736