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

[Thomas Schatzl]

On Wed, 24 Sep 2025 01:17:19 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
>   
>   fix a small error

Changes requested by tschatzl (Reviewer).

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

> 65:                               recent_load, G1PeriodicGCSystemLoadThreshold);
> 66:       return false;
> 67:     }

Suggestion:

  if (G1PeriodicGCSystemLoadThreshold > 0.0) {
    if (os::loadavg(&recent_load, 1) == -1) {
      G1PeriodicGCSystemLoadThreshold = 0.0;
      log_warning(gc, periodic)("System loadavg() call failed, disabling G1PeriodicGCSystemLoadThreshold check.");
      // Fall through and start the periodic GC.
    } else if (recent_load > G1PeriodicGCSystemLoadThreshold) {
      log_debug(gc, periodic)("Load %1.2f is higher than threshold %1.2f. Skipping.",
                              recent_load, G1PeriodicGCSystemLoadThreshold);
      return false;
    }


After detecting that `loadavg` is not supported, it would be preferable (more consistent) imo that the periodic gc would be started the first time too.
Another option is to check `loadavg` support by a dummy call in argument processing, but here is good too.

As for the other change, I would prefer to have the "disabling" in active voice, making it clear that this code disables the check.

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

PR Review: https://git.openjdk.org/jdk/pull/27413#pullrequestreview-3263079661
PR Review Comment: https://git.openjdk.org/jdk/pull/27413#discussion_r2375909307