runtime: revise assist ratio aggressively

At the start of a GC cycle, the garbage collector computes the assist
ratio based on the total scannable heap size. This was intended to be
conservative; after all, this assumes the entire heap may be reachable
and hence needs to be scanned. But it only assumes that the *current*
entire heap may be reachable. It fails to account for heap allocated
during the GC cycle. If the trigger ratio is very low (near zero), and
most of the heap is reachable when GC starts (which is likely if the
trigger ratio is near zero), then it's possible for the mutator to
create new, reachable heap fast enough that the assists won't keep up
based on the assist ratio computed at the beginning of the cycle. As a
result, the heap can grow beyond the heap goal (by hundreds of megs in
stress tests like in issue #11911).

We already have some vestigial logic for dealing with situations like
this; it just doesn't run often enough. Currently, every 10 ms during
the GC cycle, the GC revises the assist ratio. This was put in before
we switched to a conservative assist ratio (when we really were using
estimates of scannable heap), and it turns out to be exactly what we
need now. However, every 10 ms is far too infrequent for a rapidly
allocating mutator.

This commit reuses this logic, but replaces the 10 ms timer with
revising the assist ratio every time the heap is locked, which
coincides precisely with when the statistics used to compute the
assist ratio are updated.

Fixes #11911.

Change-Id: I377b231ab064946228378fa10422a46d1b50f4c5
Reviewed-on: https://go-review.googlesource.com/13047
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index b1fbdc91bb57a03650131de31ff45b7b055a4ae6..6d4799a9e299e2bb907a938917d85b0251ad5cf3 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -360,7 +360,8 @@ type gcControllerState struct {
 
        // assistRatio is the ratio of allocated bytes to scan work
        // that should be performed by mutator assists. This is
-       // computed at the beginning of each cycle.
+       // computed at the beginning of each cycle and updated every
+       // time heap_scan is updated.
        assistRatio float64
 
        // fractionalUtilizationGoal is the fraction of wall clock
@@ -379,10 +380,6 @@ type gcControllerState struct {
        // at the end of of each cycle.
        triggerRatio float64
 
-       // reviseTimer is a timer that triggers periodic revision of
-       // control variables during the cycle.
-       reviseTimer timer
-
        _ [_CacheLineSize]byte
 
        // fractionalMarkWorkersNeeded is the number of fractional
@@ -449,19 +446,11 @@ func (c *gcControllerState) startCycle() {
                        " workers=", c.dedicatedMarkWorkersNeeded,
                        "+", c.fractionalMarkWorkersNeeded, "\n")
        }
-
-       // Set up a timer to revise periodically
-       c.reviseTimer.f = func(interface{}, uintptr) {
-               gcController.revise()
-       }
-       c.reviseTimer.period = 10 * 1000 * 1000
-       c.reviseTimer.when = nanotime() + c.reviseTimer.period
-       addtimer(&c.reviseTimer)
 }
 
 // revise updates the assist ratio during the GC cycle to account for
-// improved estimates. This should be called periodically during
-// concurrent mark.
+// improved estimates. This should be called either under STW or
+// whenever memstats.heap_scan is updated (with mheap_.lock held).
 func (c *gcControllerState) revise() {
        // Compute the expected scan work. This is a strict upper
        // bound on the possible scan work in the current heap.
@@ -502,9 +491,6 @@ func (c *gcControllerState) endCycle() {
        // transient changes. Values near 1 may be unstable.
        const triggerGain = 0.5
 
-       // Stop the revise timer
-       deltimer(&c.reviseTimer)
-
        // Compute next cycle trigger ratio. First, this computes the
        // "error" for this cycle; that is, how far off the trigger
        // was from what it should have been, accounting for both heap

diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
index d190782580ddacce6ad618e6f20eaab6843bd26d..be4d61215632ecaa849ee6a31a6a4d57d5e93cfa 100644 (file)
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -420,6 +420,8 @@ func mHeap_Alloc_m(h *mheap, npage uintptr, sizeclass int32, large bool) *mspan
        memstats.tinyallocs += uint64(_g_.m.mcache.local_tinyallocs)
        _g_.m.mcache.local_tinyallocs = 0
 
+       gcController.revise()
+
        s := mHeap_AllocSpanLocked(h, npage)
        if s != nil {
                // Record span info, because gc needs to be
@@ -694,6 +696,7 @@ func mHeap_Free(h *mheap, s *mspan, acct int32) {
                if acct != 0 {
                        memstats.heap_objects--
                }
+               gcController.revise()
                mHeap_FreeSpanLocked(h, s, true, true, 0)
                if trace.enabled {
                        traceHeapAlloc()