From f99e40aac023d818e8c2594e5b8c075786087132 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 8 Apr 2021 22:01:13 +0000 Subject: [PATCH] runtime: detangle gcPaceScavenger from the pacer Currently gcPaceScavenger is called by gcControllerState.commit, but it manipulates global state which precludes testing. This change detangles the two. Change-Id: I10d8ebdf426d99ba49d2f2cb4fb64891e9fd6091 Reviewed-on: https://go-review.googlesource.com/c/go/+/309272 Reviewed-by: Michael Pratt Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot --- src/runtime/mgc.go | 1 + src/runtime/mgcpacer.go | 5 +++-- src/runtime/mgcscavenge.go | 13 +++++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index f937287281..429b907322 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -971,6 +971,7 @@ func gcMarkTermination(nextTriggerRatio float64) { // Update GC trigger and pacing for the next cycle. gcController.commit(nextTriggerRatio) + gcPaceScavenger(gcController.heapGoal, gcController.lastHeapGoal) // Update timing memstats now := nanotime() diff --git a/src/runtime/mgcpacer.go b/src/runtime/mgcpacer.go index 44b870446f..73fe6e15e4 100644 --- a/src/runtime/mgcpacer.go +++ b/src/runtime/mgcpacer.go @@ -769,8 +769,6 @@ func (c *gcControllerState) commit(triggerRatio float64) { mheap_.pagesSweptBasis.Store(pagesSwept) } } - - gcPaceScavenger() } // effectiveGrowthRatio returns the current effective heap growth @@ -796,6 +794,8 @@ func (c *gcControllerState) effectiveGrowthRatio() float64 { // setGCPercent updates gcPercent and all related pacer state. // Returns the old value of gcPercent. // +// Calls gcControllerState.commit. +// // The world must be stopped, or mheap_.lock must be held. func (c *gcControllerState) setGCPercent(in int32) int32 { assertWorldStoppedOrLockHeld(&mheap_.lock) @@ -819,6 +819,7 @@ func setGCPercent(in int32) (out int32) { systemstack(func() { lock(&mheap_.lock) out = gcController.setGCPercent(in) + gcPaceScavenger(gcController.heapGoal, gcController.lastHeapGoal) unlock(&mheap_.lock) }) diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 2bb19985db..fb9b5c8694 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -105,7 +105,8 @@ func heapRetained() uint64 { } // gcPaceScavenger updates the scavenger's pacing, particularly -// its rate and RSS goal. +// its rate and RSS goal. For this, it requires the current heapGoal, +// and the heapGoal for the previous GC cycle. // // The RSS goal is based on the current heap goal with a small overhead // to accommodate non-determinism in the allocator. @@ -113,18 +114,22 @@ func heapRetained() uint64 { // The pacing is based on scavengePageRate, which applies to both regular and // huge pages. See that constant for more information. // +// Must be called whenever GC pacing is updated. +// // mheap_.lock must be held or the world must be stopped. -func gcPaceScavenger() { +func gcPaceScavenger(heapGoal, lastHeapGoal uint64) { + assertWorldStoppedOrLockHeld(&mheap_.lock) + // If we're called before the first GC completed, disable scavenging. // We never scavenge before the 2nd GC cycle anyway (we don't have enough // information about the heap yet) so this is fine, and avoids a fault // or garbage data later. - if gcController.lastHeapGoal == 0 { + if lastHeapGoal == 0 { mheap_.scavengeGoal = ^uint64(0) return } // Compute our scavenging goal. - goalRatio := float64(atomic.Load64(&gcController.heapGoal)) / float64(gcController.lastHeapGoal) + goalRatio := float64(heapGoal) / float64(lastHeapGoal) retainedGoal := uint64(float64(memstats.last_heap_inuse) * goalRatio) // Add retainExtraPercent overhead to retainedGoal. This calculation // looks strange but the purpose is to arrive at an integer division