diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 34043c6432..8b323c5bf7 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1324,8 +1324,13 @@ func gcBgMarkWorker() { }) // Account for time and mark us as stopped. - duration := nanotime() - startTime + now := nanotime() + duration := now - startTime gcController.markWorkerStop(pp.gcMarkWorkerMode, duration) + if pp.gcMarkWorkerMode == gcMarkWorkerIdleMode { + gcCPULimiter.addIdleMarkTime(duration) + gcCPULimiter.update(now) + } if pp.gcMarkWorkerMode == gcMarkWorkerFractionalMode { atomic.Xaddint64(&pp.gcFractionalMarkTime, duration) } diff --git a/src/runtime/mgclimit.go b/src/runtime/mgclimit.go index cbe5500be6..12ff0a7e68 100644 --- a/src/runtime/mgclimit.go +++ b/src/runtime/mgclimit.go @@ -60,6 +60,9 @@ type gcCPULimiterState struct { // assistTimePool is the accumulated assist time since the last update. assistTimePool atomic.Int64 + // idleMarkTimePool is the accumulated idle mark time since the last update. + idleMarkTimePool atomic.Int64 + // lastUpdate is the nanotime timestamp of the last time update was called. // // Updated under lock, but may be read concurrently. @@ -143,6 +146,12 @@ func (l *gcCPULimiterState) addAssistTime(t int64) { l.assistTimePool.Add(t) } +// addIdleMarkTime notifies the limiter of additional idle mark worker time. It will be +// subtracted from the total CPU time in the next update. +func (l *gcCPULimiterState) addIdleMarkTime(t int64) { + l.idleMarkTimePool.Add(t) +} + // update updates the bucket given runtime-specific information. now is the // current monotonic time in nanoseconds. // @@ -177,11 +186,38 @@ func (l *gcCPULimiterState) updateLocked(now int64) { l.assistTimePool.Add(-assistTime) } - // Accumulate. + // Drain the pool of idle mark time. + idleMarkTime := l.idleMarkTimePool.Load() + if idleMarkTime != 0 { + l.idleMarkTimePool.Add(-idleMarkTime) + } + + // Compute total GC time. windowGCTime := assistTime if l.gcEnabled { windowGCTime += int64(float64(windowTotalTime) * gcBackgroundUtilization) } + + // Subtract out idle mark time from the total time. Do this after computing + // GC time, because the background utilization is dependent on the *real* + // total time, not the total time after idle time is subtracted. + // + // Idle mark workers soak up time that the application spends idle. Any + // additional idle time can skew GC CPU utilization, because the GC might + // be executing continuously and thrashing, but the CPU utilization with + // respect to GOMAXPROCS will be quite low, so the limiter will otherwise + // never kick in. By subtracting idle mark time, we're removing time that + // we know the application was idle giving a more accurate picture of whether + // the GC is thrashing. + // + // TODO(mknyszek): Figure out if it's necessary to also track non-GC idle time. + // + // There is a corner case here where if the idle mark workers are disabled, such + // as when the periodic GC is executing, then we definitely won't be accounting + // for this correctly. However, if the periodic GC is running, the limiter is likely + // totally irrelevant because GC CPU utilization is extremely low anyway. + windowTotalTime -= idleMarkTime + l.accumulate(windowTotalTime-windowGCTime, windowGCTime) }