RFR: 8368089: G1: G1PeriodicGCTask::should_start_periodic_gc may use uninitialised value if os::loadavg is unsupported [v2]

[Albert Mingkun Yang]

On Mon, 22 Sep 2025 13:23:56 GMT, Guanqiang Han <ghan at openjdk.org> wrote:

>> Please review this change. Thanks!
>> 
>> **Description:**
>> 
>> Previously the check combined loadavg() failure and threshold comparison in a single condition. When os::loadavg() returned -1, `recent_load` contained an undefined value but was still logged as if it was a real load average, leading to misleading log output.
>> 
>> **Fix:**
>> 
>> This change separates the failure case from the combined check:
>> - If os::loadavg() returns -1, log "System loadavg not supported" and skip.
>> - Only compare `recent_load` to the threshold when loadavg() succeeds.
>> 
>> **Test:**
>> 
>> GHA
>
> Guanqiang Han has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update g1PeriodicGCTask.cpp
>   
>   Disable the functionality when os::loadavg() is not supported.

src/hotspot/share/gc/g1/g1PeriodicGCTask.cpp line 61:

> 59:       G1PeriodicGCInterval = 0;
> 60:       log_warning(gc, periodic)("System loadavg not supported. Periodic GC is disabled "
> 61:                                 "by setting G1PeriodicGCInterval to 0.");

This disables periodic-gc. It's not super obvious that this is desirable. My expectation would be if `G1PeriodicGCSystemLoadThreshold`, a fine tuning knob in controlling periodic-gc, is not supported, this tuning knob will just be ignored with a warning, instead of disabling periodic-gc completely.

However, I can also imagine starting periodic-gc when system-load is high can be detrimental, so disabling periodic-gc is safer.

(Just raising it for discussion.)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27413#discussion_r2371487036