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

[Monica Beckwith]

On Wed, 16 Jul 2025 18:28:33 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/g1HeapEvaluationTask.cpp line 36:
> 
>> 34: #include "utilities/globalDefinitions.hpp"
>> 35: 
>> 36: G1HeapEvaluationTask::G1HeapEvaluationTask(G1CollectedHeap* g1h, G1HeapSizingPolicy* heap_sizing_policy) :
> 
> `G1HeapEvaluationTask` class name does not seem to clearly state the purpose of the class. But I cannot come up with a better name.

I can see that while 'G1HeapEvaluationTask' is generic, it kind of does describe what the class does (evaluates heap sizing decisions). The time-based nature is captured in the feature flag and comments. Open to suggestions if you have a preference.

> src/hotspot/share/gc/g1/g1HeapSizingPolicy.cpp line 46:
> 
>> 44: void G1HeapSizingPolicy::initialize() {
>> 45:   // Flag values are available at this point
>> 46:   _uncommit_delay_ms = (jlong)G1UncommitDelayMillis;
> 
> I think we can "safely" do this in the constructor for G1HeapSizingPolicy, then we don't need the `initialize` method. 
> 
> We also do not need to make `_uncommit_delay_ms`, `G1HeapSizingPolicy` relies on the singleton pattern through `G1HeapSizingPolicy::create`.

Good point! You're absolutely right - since G1HeapSizingPolicy uses the singleton pattern through `create()`, we can safely move the initialization into the constructor and eliminate the separate `initialize()` method. Made the change:

- Moved `_uncommit_delay_ms` initialization into the constructor
- Removed the `initialize()` method
- Removed the call to `initialize()` from G1CollectedHeap constructor

Much cleaner approach, thanks for the suggestion!

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

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