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 <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index f937287..429b907 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -971,6 +971,7 @@
// 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 44b8704..73fe6e1 100644
--- a/src/runtime/mgcpacer.go
+++ b/src/runtime/mgcpacer.go
@@ -769,8 +769,6 @@
mheap_.pagesSweptBasis.Store(pagesSwept)
}
}
-
- gcPaceScavenger()
}
// effectiveGrowthRatio returns the current effective heap growth
@@ -796,6 +794,8 @@
// 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 @@
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 2bb1998..fb9b5c8 100644
--- a/src/runtime/mgcscavenge.go
+++ b/src/runtime/mgcscavenge.go
@@ -105,7 +105,8 @@
}
// 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 @@
// 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
