]> Cypherpunks.ru repositories - gostls13.git/commitdiff
runtime: check the heap goal and trigger dynamically
authorMichael Anthony Knyszek <mknyszek@google.com>
Mon, 21 Mar 2022 21:27:06 +0000 (21:27 +0000)
committerMichael Knyszek <mknyszek@google.com>
Tue, 3 May 2022 15:12:52 +0000 (15:12 +0000)
As it stands, the heap goal and the trigger are set once by
gcController.commit, and then read out of gcController. However with the
coming memory limit we need the GC to be able to respond to changes in
non-heap memory. The simplest way of achieving this is to compute the
heap goal and its associated trigger dynamically.

In order to make this easier to implement, the GC trigger is now based
on the heap goal, as opposed to the status quo of computing both
simultaneously. In many cases we just want the heap goal anyway, not
both, but we definitely need the goal to compute the trigger, because
the trigger's bounds are entirely based on the goal (the initial runway
is not). A consequence of this is that we can't rely on the trigger to
enforce a minimum heap size anymore, and we need to lift that up
directly to the goal. Specifically, we need to lift up any part of the
calculation that *could* put the trigger ahead of the goal. Luckily this
is just the heap minimum and minimum sweep distance. In the first case,
the pacer may behave slightly differently, as the heap minimum is no
longer the minimum trigger, but the actual minimum heap goal. In the
second case it should be the same, as we ensure the additional runway
for sweeping is added to both the goal *and* the trigger, as before, by
computing that in gcControllerState.commit.

There's also another place we update the heap goal: if a GC starts and
we triggered beyond the goal, we always ensure there's some runway.
That calculation uses the current trigger, which violates the rule of
keeping the goal based on the trigger. Notice, however, that using the
precomputed trigger for this isn't even quite correct: due to a bug, or
something else, we might trigger a GC beyond the precomputed trigger.

So this change also adds a "triggered" field to gcControllerState that
tracks the point at which a GC actually triggered. This is independent
of the precomputed trigger, so it's fine for the heap goal calculation
to rely on it. It also turns out, there's more than just that one place
where we really should be using the actual trigger point, so this change
fixes those up too.

Also, because the heap minimum is set by the goal and not the trigger,
the maximum trigger calculation now happens *after* the goal is set, so
the maximum trigger actually does what I originally intended (and what
the comment says): at small heaps, the pacer picks 95% of the runway as
the maximum trigger. Currently, the pacer picks a small trigger based
on a not-yet-rounded-up heap goal, so the trigger gets rounded up to the
goal, and as per the "ensure there's some runway" check, the runway ends
up at always being 64 KiB. That check is supposed to be for exceptional
circumstances, not the status quo. There's a test introduced in the last
CL that needs to be updated to accomodate this slight change in
behavior.

So, this all sounds like a lot that changed, but what we're talking about
here are really, really tight corner cases that arise from situations
outside of our control, like pathologically bad behavior on the part of
an OS or CPU. Even in these corner cases, it's very unlikely that users
will notice any difference at all. What's more important, I think, is
that the pacer behaves more closely to what all the comments describe,
and what the original intent was.

Another note: at first, one might think that computing the heap goal and
trigger dynamically introduces some raciness, but not in this CL: the heap
goal and trigger are completely static.

Allocation outside of a GC cycle may now be a bit slower than before, as
the GC trigger check is now significantly more complex. However, note
that this executes basically just as often as gcController.revise, and
that makes up for a vanishingly small part of any CPU profile. The next
CL cleans up the floating point multiplications on this path
nonetheless, just to be safe.

For #48409.

Change-Id: I280f5ad607a86756d33fb8449ad08555cbee93f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/397014
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/runtime/export_test.go
src/runtime/metrics.go
src/runtime/mgc.go
src/runtime/mgcpacer.go
src/runtime/mgcpacer_test.go
src/runtime/mstats.go
src/runtime/trace.go

index 6d1cf645d207ad91fa22d07698bb3dd9f03f0399..52d154bf903d3fbde3c98cfa665663a79177d9ef 100644 (file)
@@ -1281,10 +1281,11 @@ func NewGCController(gcPercent int) *GCController {
 }
 
 func (c *GCController) StartCycle(stackSize, globalsSize uint64, scannableFrac float64, gomaxprocs int) {
+       trigger, _ := c.trigger()
        c.scannableStackSize = stackSize
        c.globalsScan = globalsSize
-       c.heapLive = c.trigger
-       c.heapScan += uint64(float64(c.trigger-c.heapMarked) * scannableFrac)
+       c.heapLive = trigger
+       c.heapScan += uint64(float64(trigger-c.heapMarked) * scannableFrac)
        c.startCycle(0, gomaxprocs, gcTrigger{kind: gcTriggerHeap})
 }
 
@@ -1293,7 +1294,7 @@ func (c *GCController) AssistWorkPerByte() float64 {
 }
 
 func (c *GCController) HeapGoal() uint64 {
-       return c.heapGoal
+       return c.heapGoal()
 }
 
 func (c *GCController) HeapLive() uint64 {
@@ -1304,8 +1305,8 @@ func (c *GCController) HeapMarked() uint64 {
        return c.heapMarked
 }
 
-func (c *GCController) Trigger() uint64 {
-       return c.trigger
+func (c *GCController) Triggered() uint64 {
+       return c.triggered
 }
 
 type GCControllerReviseDelta struct {
@@ -1329,7 +1330,7 @@ func (c *GCController) EndCycle(bytesMarked uint64, assistTime, elapsed int64, g
        c.assistTime.Store(assistTime)
        c.endCycle(elapsed, gomaxprocs, false)
        c.resetLive(bytesMarked)
-       c.commit()
+       c.commit(false)
 }
 
 func (c *GCController) AddIdleMarkWorker() bool {
index ba0a920a5d16bc75d0e4d868702d1745f3da2770..763863e3584a5babee34e9968bd2c6d5d2953073 100644 (file)
@@ -431,7 +431,7 @@ func (a *sysStatsAggregate) compute() {
        a.buckHashSys = memstats.buckhash_sys.load()
        a.gcMiscSys = memstats.gcMiscSys.load()
        a.otherSys = memstats.other_sys.load()
-       a.heapGoal = atomic.Load64(&gcController.heapGoal)
+       a.heapGoal = gcController.heapGoal()
        a.gcCyclesDone = uint64(memstats.numgc)
        a.gcCyclesForced = uint64(memstats.numforcedgc)
 
index e6663b01ac3b93014070f3eeec14cefe89a8daab..93d090f6edc3f5c370321a0557bb1123db5b3b7a 100644 (file)
 // Next GC is after we've allocated an extra amount of memory proportional to
 // the amount already in use. The proportion is controlled by GOGC environment variable
 // (100 by default). If GOGC=100 and we're using 4M, we'll GC again when we get to 8M
-// (this mark is tracked in gcController.heapGoal variable). This keeps the GC cost in
+// (this mark is computed by the gcController.heapGoal method). This keeps the GC cost in
 // linear proportion to the allocation cost. Adjusting GOGC just changes the linear constant
 // (and also the amount of extra memory used).
 
@@ -401,7 +401,7 @@ var work struct {
        pauseStart int64 // nanotime() of last STW
 
        // debug.gctrace heap sizes for this cycle.
-       heap0, heap1, heap2, heapGoal uint64
+       heap0, heap1, heap2 uint64
 }
 
 // GC runs a garbage collection and blocks the caller until the
@@ -553,7 +553,8 @@ func (t gcTrigger) test() bool {
                // we are going to trigger on this, this thread just
                // atomically wrote gcController.heapLive anyway and we'll see our
                // own write.
-               return gcController.heapLive >= gcController.trigger
+               trigger, _ := gcController.trigger()
+               return atomic.Load64(&gcController.heapLive) >= trigger
        case gcTriggerTime:
                if gcController.gcPercent.Load() < 0 {
                        return false
@@ -674,7 +675,6 @@ func gcStart(trigger gcTrigger) {
        // Assists and workers can start the moment we start
        // the world.
        gcController.startCycle(now, int(gomaxprocs), trigger)
-       work.heapGoal = gcController.heapGoal
 
        // Notify the CPU limiter that assists may begin.
        gcCPULimiter.startGCTransition(true, 0, now)
@@ -985,10 +985,9 @@ func gcMarkTermination() {
        // Record heapInUse for scavenger.
        memstats.lastHeapInUse = gcController.heapInUse.load()
 
-       // Update GC trigger and pacing for the next cycle.
-       gcController.commit()
-       gcPaceSweeper(gcController.trigger)
-       gcPaceScavenger(gcController.heapGoal, gcController.lastHeapGoal)
+       // Update GC trigger and pacing, as well as downstream consumers
+       // of this pacing information, for the next cycle.
+       systemstack(gcControllerCommit)
 
        // Update timing memstats
        now := nanotime()
@@ -1111,7 +1110,7 @@ func gcMarkTermination() {
                }
                print(" ms cpu, ",
                        work.heap0>>20, "->", work.heap1>>20, "->", work.heap2>>20, " MB, ",
-                       work.heapGoal>>20, " MB goal, ",
+                       gcController.heapGoal()>>20, " MB goal, ",
                        gcController.stackScan>>20, " MB stacks, ",
                        gcController.globalsScan>>20, " MB globals, ",
                        work.maxprocs, " P")
index 40cf67c747e3372f27d8dc17cbc24c860644866b..44e45f2d09870732539c12609ea623c286a50604 100644 (file)
@@ -119,17 +119,11 @@ type gcControllerState struct {
        // debugging.
        heapMinimum uint64
 
-       // trigger is the heap size that triggers marking.
+       // runway is the amount of runway in heap bytes allocated by the
+       // application that we want to give the GC once it starts.
        //
-       // When heapLive ≥ trigger, the mark phase will start.
-       // This is also the heap size by which proportional sweeping
-       // must be complete.
-       //
-       // This is computed from consMark during mark termination for
-       // the next cycle's trigger.
-       //
-       // Protected by mheap_.lock or a STW.
-       trigger uint64
+       // This is computed from consMark during mark termination.
+       runway atomic.Uint64
 
        // consMark is the estimated per-CPU consMark ratio for the application.
        //
@@ -154,14 +148,33 @@ type gcControllerState struct {
 
        _ uint32 // Padding for atomics on 32-bit platforms.
 
-       // heapGoal is the goal heapLive for when next GC ends.
-       // Set to ^uint64(0) if disabled.
+       // gcPercentHeapGoal is the goal heapLive for when next GC ends derived
+       // from gcPercent.
+       //
+       // Set to ^uint64(0) if gcPercent is disabled.
+       gcPercentHeapGoal atomic.Uint64
+
+       // sweepDistMinTrigger is the minimum trigger to ensure a minimum
+       // sweep distance.
        //
-       // Read and written atomically, unless the world is stopped.
-       heapGoal uint64
+       // This bound is also special because it applies to both the trigger
+       // *and* the goal (all other trigger bounds must be based *on* the goal).
+       //
+       // It is computed ahead of time, at commit time. The theory is that,
+       // absent a sudden change to a parameter like gcPercent, the trigger
+       // will be chosen to always give the sweeper enough headroom. However,
+       // such a change might dramatically and suddenly move up the trigger,
+       // in which case we need to ensure the sweeper still has enough headroom.
+       sweepDistMinTrigger atomic.Uint64
+
+       // triggered is the point at which the current GC cycle actually triggered.
+       // Only valid during the mark phase of a GC cycle, otherwise set to ^uint64(0).
+       //
+       // Updated while the world is stopped.
+       triggered uint64
 
-       // lastHeapGoal is the value of heapGoal for the previous GC.
-       // Note that this is distinct from the last value heapGoal had,
+       // lastHeapGoal is the value of heapGoal at the moment the last GC
+       // ended. Note that this is distinct from the last value heapGoal had,
        // because it could change if e.g. gcPercent changes.
        //
        // Read and written with the world stopped or with mheap_.lock held.
@@ -169,10 +182,10 @@ type gcControllerState struct {
 
        // heapLive is the number of bytes considered live by the GC.
        // That is: retained by the most recent GC plus allocated
-       // since then. heapLive ≤ memstats.heapAlloc, since heapAlloc includes
-       // unmarked objects that have not yet been swept (and hence goes up as we
-       // allocate and down as we sweep) while heapLive excludes these
-       // objects (and hence only goes up between GCs).
+       // since then. heapLive ≤ memstats.totalAlloc-memstats.totalFree, since
+       // heapAlloc includes unmarked objects that have not yet been swept (and
+       // hence goes up as we allocate and down as we sweep) while heapLive
+       // excludes these objects (and hence only goes up between GCs).
        //
        // This is updated atomically without locking. To reduce
        // contention, this is updated only when obtaining a span from
@@ -377,6 +390,7 @@ type gcControllerState struct {
 
 func (c *gcControllerState) init(gcPercent int32, memoryLimit int64) {
        c.heapMinimum = defaultHeapMinimum
+       c.triggered = ^uint64(0)
 
        c.consMarkController = piController{
                // Tuned first via the Ziegler-Nichols process in simulation,
@@ -401,7 +415,10 @@ func (c *gcControllerState) init(gcPercent int32, memoryLimit int64) {
 
        c.setGCPercent(gcPercent)
        c.setMemoryLimit(memoryLimit)
-       c.commit()
+       c.commit(true) // No sweep phase in the first GC cycle.
+
+       // N.B. Don't bother calling traceHeapGoal. Tracing is never enabled at
+       // initialization time.
 }
 
 // startCycle resets the GC controller's state and computes estimates
@@ -418,17 +435,7 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
        c.idleMarkTime = 0
        c.markStartTime = markStartTime
        c.stackScan = atomic.Load64(&c.scannableStackSize)
-
-       // Ensure that the heap goal is at least a little larger than
-       // the current live heap size. This may not be the case if GC
-       // start is delayed or if the allocation that pushed gcController.heapLive
-       // over trigger is large or if the trigger is really close to
-       // GOGC. Assist is proportional to this distance, so enforce a
-       // minimum distance, even if it means going over the GOGC goal
-       // by a tiny bit.
-       if c.heapGoal < c.heapLive+64<<10 {
-               c.heapGoal = c.heapLive + 64<<10
-       }
+       c.triggered = c.heapLive
 
        // Compute the background mark utilization goal. In general,
        // this may not come out exactly. We round the number of
@@ -490,11 +497,12 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
        c.revise()
 
        if debug.gcpacertrace > 0 {
+               heapGoal := c.heapGoal()
                assistRatio := c.assistWorkPerByte.Load()
                print("pacer: assist ratio=", assistRatio,
                        " (scan ", gcController.heapScan>>20, " MB in ",
                        work.initialHeapLive>>20, "->",
-                       c.heapGoal>>20, " MB)",
+                       heapGoal>>20, " MB)",
                        " workers=", c.dedicatedMarkWorkersNeeded,
                        "+", c.fractionalUtilizationGoal, "\n")
        }
@@ -502,8 +510,9 @@ func (c *gcControllerState) startCycle(markStartTime int64, procs int, trigger g
 
 // revise updates the assist ratio during the GC cycle to account for
 // improved estimates. This should be called whenever gcController.heapScan,
-// gcController.heapLive, or gcController.heapGoal is updated. It is safe to
-// call concurrently, but it may race with other calls to revise.
+// gcController.heapLive, or if any inputs to gcController.heapGoal are
+// updated. It is safe to call concurrently, but it may race with other
+// calls to revise.
 //
 // The result of this race is that the two assist ratio values may not line
 // up or may be stale. In practice this is OK because the assist ratio
@@ -534,7 +543,7 @@ func (c *gcControllerState) revise() {
 
        // Assume we're under the soft goal. Pace GC to complete at
        // heapGoal assuming the heap is in steady-state.
-       heapGoal := int64(atomic.Load64(&c.heapGoal))
+       heapGoal := int64(c.heapGoal())
 
        // The expected scan work is computed as the amount of bytes scanned last
        // GC cycle, plus our estimate of stacks and globals work for this cycle.
@@ -555,7 +564,7 @@ func (c *gcControllerState) revise() {
                // growths. It's OK to use more memory this cycle to scan all the live heap,
                // because the next GC cycle is inevitably going to use *at least* that much
                // memory anyway.
-               extHeapGoal := int64(float64(heapGoal-int64(c.trigger))/float64(scanWorkExpected)*float64(maxScanWork)) + int64(c.trigger)
+               extHeapGoal := int64(float64(heapGoal-int64(c.triggered))/float64(scanWorkExpected)*float64(maxScanWork)) + int64(c.triggered)
                scanWorkExpected = maxScanWork
 
                // hardGoal is a hard limit on the amount that we're willing to push back the
@@ -630,7 +639,7 @@ func (c *gcControllerState) revise() {
 func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
        // Record last heap goal for the scavenger.
        // We'll be updating the heap goal soon.
-       gcController.lastHeapGoal = gcController.heapGoal
+       gcController.lastHeapGoal = c.heapGoal()
 
        // Compute the duration of time for which assists were turned on.
        assistDuration := now - c.markStartTime
@@ -642,11 +651,11 @@ func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
                utilization += float64(c.assistTime.Load()) / float64(assistDuration*int64(procs))
        }
 
-       if c.heapLive <= c.trigger {
+       if c.heapLive <= c.triggered {
                // Shouldn't happen, but let's be very safe about this in case the
                // GC is somehow extremely short.
                //
-               // In this case though, the only reasonable value for c.heapLive-c.trigger
+               // In this case though, the only reasonable value for c.heapLive-c.triggered
                // would be 0, which isn't really all that useful, i.e. the GC was so short
                // that it didn't matter.
                //
@@ -686,7 +695,7 @@ func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
        //
        // Note that because we only care about the ratio, assistDuration and procs cancel out.
        scanWork := c.heapScanWork.Load() + c.stackScanWork.Load() + c.globalsScanWork.Load()
-       currentConsMark := (float64(c.heapLive-c.trigger) * (utilization + idleUtilization)) /
+       currentConsMark := (float64(c.heapLive-c.triggered) * (utilization + idleUtilization)) /
                (float64(scanWork) * (1 - utilization))
 
        // Update cons/mark controller. The time period for this is 1 GC cycle.
@@ -716,11 +725,12 @@ func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
        }
 
        if debug.gcpacertrace > 0 {
+               heapGoal := c.heapGoal()
                printlock()
                goal := gcGoalUtilization * 100
                print("pacer: ", int(utilization*100), "% CPU (", int(goal), " exp.) for ")
                print(c.heapScanWork.Load(), "+", c.stackScanWork.Load(), "+", c.globalsScanWork.Load(), " B work (", c.lastHeapScan+c.stackScan+c.globalsScan, " B exp.) ")
-               print("in ", c.trigger, " B -> ", c.heapLive, " B (∆goal ", int64(c.heapLive)-int64(c.heapGoal), ", cons/mark ", oldConsMark, ")")
+               print("in ", c.triggered, " B -> ", c.heapLive, " B (∆goal ", int64(c.heapLive)-int64(heapGoal), ", cons/mark ", oldConsMark, ")")
                if !ok {
                        print("[controller reset]")
                }
@@ -867,6 +877,7 @@ func (c *gcControllerState) resetLive(bytesMarked uint64) {
        c.heapLive = bytesMarked
        c.heapScan = uint64(c.heapScanWork.Load())
        c.lastHeapScan = uint64(c.heapScanWork.Load())
+       c.triggered = ^uint64(0) // Reset triggered.
 
        // heapLive was updated, so emit a trace event.
        if trace.enabled {
@@ -931,41 +942,61 @@ func (c *gcControllerState) addGlobals(amount int64) {
        atomic.Xadd64(&c.globalsScan, amount)
 }
 
-// commit recomputes all pacing parameters from scratch, namely
-// absolute trigger, the heap goal, mark pacing, and sweep pacing.
-//
-// This can be called any time. If GC is the in the middle of a
-// concurrent phase, it will adjust the pacing of that phase.
-//
-// This depends on gcPercent, gcController.heapMarked, and
-// gcController.heapLive. These must be up to date.
+// heapGoal returns the current heap goal.
+func (c *gcControllerState) heapGoal() uint64 {
+       goal, _ := c.heapGoalInternal()
+       return goal
+}
+
+// heapGoalInternal is the implementation of heapGoal which returns additional
+// information that is necessary for computing the trigger.
 //
-// mheap_.lock must be held or the world must be stopped.
-func (c *gcControllerState) commit() {
-       if !c.test {
-               assertWorldStoppedOrLockHeld(&mheap_.lock)
+// The returned minTrigger is always <= goal.
+func (c *gcControllerState) heapGoalInternal() (goal, minTrigger uint64) {
+       // Start with the goal calculated for gcPercent.
+       goal = c.gcPercentHeapGoal.Load()
+       sweepDistTrigger := c.sweepDistMinTrigger.Load()
+
+       // Don't set a goal below the minimum heap size or the minimum
+       // trigger determined at commit time.
+       minGoal := c.heapMinimum
+       if sweepDistTrigger > minGoal {
+               minGoal = sweepDistTrigger
+       }
+       if goal < minGoal {
+               goal = minGoal
        }
 
-       // Compute the next GC goal, which is when the allocated heap
-       // has grown by GOGC/100 over where it started the last cycle,
-       // plus additional runway for non-heap sources of GC work.
-       goal := ^uint64(0)
-       if gcPercent := c.gcPercent.Load(); gcPercent >= 0 {
-               goal = c.heapMarked + (c.heapMarked+atomic.Load64(&c.stackScan)+atomic.Load64(&c.globalsScan))*uint64(gcPercent)/100
+       // Ensure that the heap goal is at least a little larger than
+       // the point at which we triggered. This may not be the case if GC
+       // start is delayed or if the allocation that pushed gcController.heapLive
+       // over trigger is large or if the trigger is really close to
+       // GOGC. Assist is proportional to this distance, so enforce a
+       // minimum distance, even if it means going over the GOGC goal
+       // by a tiny bit.
+       const minRunway = 64 << 10
+       if c.triggered != ^uint64(0) && goal < c.triggered+minRunway {
+               goal = c.triggered + minRunway
        }
+       return goal, sweepDistTrigger
+}
 
-       // Don't trigger below the minimum heap size.
-       minTrigger := c.heapMinimum
-       if !isSweepDone() {
-               // Concurrent sweep happens in the heap growth
-               // from gcController.heapLive to trigger, so ensure
-               // that concurrent sweep has some heap growth
-               // in which to perform sweeping before we
-               // start the next GC cycle.
-               sweepMin := atomic.Load64(&c.heapLive) + sweepMinHeapDistance
-               if sweepMin > minTrigger {
-                       minTrigger = sweepMin
-               }
+// trigger returns the current point at which a GC should trigger along with
+// the heap goal.
+//
+// The returned value may be compared against heapLive to determine whether
+// the GC should trigger. Thus, the GC trigger condition should be (but may
+// not be, in the case of small movements for efficiency) checked whenever
+// the heap goal may change.
+func (c *gcControllerState) trigger() (uint64, uint64) {
+       goal, minTrigger := c.heapGoalInternal()
+
+       // Invariant: the trigger must always be less than the heap goal.
+
+       // heapMarked is our absolute minumum, and it's possible the trigger
+       // bound we get from heapGoalinternal is less than that.
+       if minTrigger < c.heapMarked {
+               minTrigger = c.heapMarked
        }
 
        // If we let the trigger go too low, then if the application
@@ -1001,7 +1032,69 @@ func (c *gcControllerState) commit() {
                maxTrigger = minTrigger
        }
 
-       // Compute the trigger by using our estimate of the cons/mark ratio.
+       // Compute the trigger from our bounds and the runway stored by commit.
+       var trigger uint64
+       runway := c.runway.Load()
+       if runway > goal {
+               trigger = minTrigger
+       } else {
+               trigger = goal - runway
+       }
+       if trigger < minTrigger {
+               trigger = minTrigger
+       }
+       if trigger > maxTrigger {
+               trigger = maxTrigger
+       }
+       if trigger > goal {
+               print("trigger=", trigger, " heapGoal=", goal, "\n")
+               print("minTrigger=", minTrigger, " maxTrigger=", maxTrigger, "\n")
+               throw("produced a trigger greater than the heap goal")
+       }
+       return trigger, goal
+}
+
+// commit recomputes all pacing parameters needed to derive the
+// trigger and the heap goal. Namely, the gcPercent-based heap goal,
+// and the amount of runway we want to give the GC this cycle.
+//
+// This can be called any time. If GC is the in the middle of a
+// concurrent phase, it will adjust the pacing of that phase.
+//
+// isSweepDone should be the result of calling isSweepDone(),
+// unless we're testing or we know we're executing during a GC cycle.
+//
+// This depends on gcPercent, gcController.heapMarked, and
+// gcController.heapLive. These must be up to date.
+//
+// mheap_.lock must be held or the world must be stopped.
+func (c *gcControllerState) commit(isSweepDone bool) {
+       if !c.test {
+               assertWorldStoppedOrLockHeld(&mheap_.lock)
+       }
+
+       if isSweepDone {
+               // The sweep is done, so there aren't any restrictions on the trigger
+               // we need to think about.
+               c.sweepDistMinTrigger.Store(0)
+       } else {
+               // Concurrent sweep happens in the heap growth
+               // from gcController.heapLive to trigger. Make sure we
+               // give the sweeper some runway if it doesn't have enough.
+               c.sweepDistMinTrigger.Store(atomic.Load64(&c.heapLive) + sweepMinHeapDistance)
+       }
+
+       // Compute the next GC goal, which is when the allocated heap
+       // has grown by GOGC/100 over where it started the last cycle,
+       // plus additional runway for non-heap sources of GC work.
+       gcPercentHeapGoal := ^uint64(0)
+       if gcPercent := c.gcPercent.Load(); gcPercent >= 0 {
+               gcPercentHeapGoal = c.heapMarked + (c.heapMarked+atomic.Load64(&c.stackScan)+atomic.Load64(&c.globalsScan))*uint64(gcPercent)/100
+       }
+       c.gcPercentHeapGoal.Store(gcPercentHeapGoal)
+
+       // Compute the amount of runway we want the GC to have by using our
+       // estimate of the cons/mark ratio.
        //
        // The idea is to take our expected scan work, and multiply it by
        // the cons/mark ratio to determine how long it'll take to complete
@@ -1020,32 +1113,10 @@ func (c *gcControllerState) commit() {
        // As a result, this is basically just "weighing" the cons/mark ratio by
        // our desired division of resources.
        //
-       // Furthermore, by setting the trigger so that CPU resources are divided
+       // Furthermore, by setting the runway so that CPU resources are divided
        // this way, assuming that the cons/mark ratio is correct, we make that
        // division a reality.
-       var trigger uint64
-       runway := uint64((c.consMark * (1 - gcGoalUtilization) / (gcGoalUtilization)) * float64(c.lastHeapScan+c.stackScan+c.globalsScan))
-       if runway > goal {
-               trigger = minTrigger
-       } else {
-               trigger = goal - runway
-       }
-       if trigger < minTrigger {
-               trigger = minTrigger
-       }
-       if trigger > maxTrigger {
-               trigger = maxTrigger
-       }
-       if trigger > goal {
-               goal = trigger
-       }
-
-       // Commit to the trigger and goal.
-       c.trigger = trigger
-       atomic.Store64(&c.heapGoal, goal)
-       if trace.enabled {
-               traceHeapGoal()
-       }
+       c.runway.Store(uint64((c.consMark * (1 - gcGoalUtilization) / (gcGoalUtilization)) * float64(c.lastHeapScan+c.stackScan+c.globalsScan)))
 
        // Update mark pacing.
        if gcphase != _GCoff {
@@ -1066,8 +1137,8 @@ func (c *gcControllerState) effectiveGrowthRatio() float64 {
        if !c.test {
                assertWorldStoppedOrLockHeld(&mheap_.lock)
        }
-
-       egogc := float64(atomic.Load64(&c.heapGoal)-c.heapMarked) / float64(c.heapMarked)
+       heapGoal := c.heapGoal()
+       egogc := float64(heapGoal-c.heapMarked) / float64(c.heapMarked)
        if egogc < 0 {
                // Shouldn't happen, but just in case.
                egogc = 0
@@ -1100,9 +1171,7 @@ func setGCPercent(in int32) (out int32) {
        systemstack(func() {
                lock(&mheap_.lock)
                out = gcController.setGCPercent(in)
-               gcController.commit()
-               gcPaceSweeper(gcController.trigger)
-               gcPaceScavenger(gcController.heapGoal, gcController.lastHeapGoal)
+               gcControllerCommit()
                unlock(&mheap_.lock)
        })
 
@@ -1155,9 +1224,7 @@ func setMemoryLimit(in int64) (out int64) {
                        unlock(&mheap_.lock)
                        return
                }
-               gcController.commit()
-               gcPaceSweeper(gcController.trigger)
-               gcPaceScavenger(gcController.heapGoal, gcController.lastHeapGoal)
+               gcControllerCommit()
                unlock(&mheap_.lock)
        })
        return out
@@ -1321,3 +1388,28 @@ func (c *gcControllerState) setMaxIdleMarkWorkers(max int32) {
                }
        }
 }
+
+// gcControllerCommit is gcController.commit, but passes arguments from live
+// (non-test) data. It also updates any consumers of the GC pacing, such as
+// sweep pacing and the background scavenger.
+//
+// Calls gcController.commit.
+//
+// The heap lock must be held, so this must be executed on the system stack.
+//
+//go:systemstack
+func gcControllerCommit() {
+       assertWorldStoppedOrLockHeld(&mheap_.lock)
+
+       gcController.commit(isSweepDone())
+
+       // TODO(mknyszek): This isn't really accurate any longer because the heap
+       // goal is computed dynamically. Still useful to snapshot, but not as useful.
+       if trace.enabled {
+               traceHeapGoal()
+       }
+
+       trigger, heapGoal := gcController.trigger()
+       gcPaceSweeper(trigger)
+       gcPaceScavenger(heapGoal, gcController.lastHeapGoal)
+}
index fa60ddcc595efda6af86515cf1f4a1fbd829bee3..48b3477be7163dec9aa44f7d1215edf441041915 100644 (file)
@@ -300,11 +300,9 @@ func TestGcPacer(t *testing.T) {
                                        // room because we're probably going to be triggering early.
                                        assertInRange(t, "goal ratio", c[n-1].goalRatio(), 0.925, 1.025)
                                        // Next, let's make sure there's some minimum distance between the goal
-                                       // and the trigger.
-                                       //
-                                       // TODO(mknyszek): This is not quite intentional. For small heaps we should
-                                       // be within 5%.
-                                       assertInRange(t, "runway", c[n-1].runway(), 32<<10, 64<<10)
+                                       // and the trigger. It should be proportional to the runway (hence the
+                                       // trigger ratio check, instead of a check against the runway).
+                                       assertInRange(t, "trigger ratio", c[n-1].triggerRatio(), 0.925, 0.975)
                                }
                                if n > 25 {
                                        // Double-check that GC utilization looks OK.
@@ -529,7 +527,7 @@ func TestGcPacer(t *testing.T) {
                                        cycle:         i + 1,
                                        heapLive:      c.HeapMarked(),
                                        heapScannable: int64(float64(int64(c.HeapMarked())-bytesAllocatedBlackLast) * cycle.scannableFrac),
-                                       heapTrigger:   c.Trigger(),
+                                       heapTrigger:   c.Triggered(),
                                        heapPeak:      c.HeapLive(),
                                        heapGoal:      c.HeapGoal(),
                                        gcUtilization: float64(assistTime)/(float64(gcDuration)*float64(e.nCores)) + GCBackgroundUtilization,
index 90e5b9590953fafc5f8c942b7e92a3621a18ee4c..f4b2da03fc40bcdc1255f40af89f905eb208f633 100644 (file)
@@ -435,6 +435,8 @@ func readmemstats_m(stats *MemStats) {
                memstats.buckhash_sys.load() + memstats.gcMiscSys.load() + memstats.other_sys.load() +
                stackInUse + gcWorkBufInUse + gcProgPtrScalarBitsInUse
 
+       heapGoal := gcController.heapGoal()
+
        // The world is stopped, so the consistent stats (after aggregation)
        // should be identical to some combination of memstats. In particular:
        //
@@ -530,7 +532,7 @@ func readmemstats_m(stats *MemStats) {
        // at a more granular level in the runtime.
        stats.GCSys = memstats.gcMiscSys.load() + gcWorkBufInUse + gcProgPtrScalarBitsInUse
        stats.OtherSys = memstats.other_sys.load()
-       stats.NextGC = gcController.heapGoal
+       stats.NextGC = heapGoal
        stats.LastGC = memstats.last_gc_unix
        stats.PauseTotalNs = memstats.pause_total_ns
        stats.PauseNs = memstats.pause_ns
index b50e1b2ce0a16c3f45515b536caf0fa63476552f..dc26cfa25a04a81ed3c708767564415da116499f 100644 (file)
@@ -14,7 +14,6 @@ package runtime
 
 import (
        "internal/goarch"
-       "runtime/internal/atomic"
        "runtime/internal/sys"
        "unsafe"
 )
@@ -55,7 +54,7 @@ const (
        traceEvGoWaiting         = 31 // denotes that goroutine is blocked when tracing starts [timestamp, goroutine id]
        traceEvGoInSyscall       = 32 // denotes that goroutine is in syscall when tracing starts [timestamp, goroutine id]
        traceEvHeapAlloc         = 33 // gcController.heapLive change [timestamp, heap_alloc]
-       traceEvHeapGoal          = 34 // gcController.heapGoal (formerly next_gc) change [timestamp, heap goal in bytes]
+       traceEvHeapGoal          = 34 // gcController.heapGoal() (formerly next_gc) change [timestamp, heap goal in bytes]
        traceEvTimerGoroutine    = 35 // not currently used; previously denoted timer goroutine [timer goroutine id]
        traceEvFutileWakeup      = 36 // denotes that the previous wakeup of this goroutine was futile [timestamp]
        traceEvString            = 37 // string dictionary entry [ID, length, string]
@@ -1170,7 +1169,8 @@ func traceHeapAlloc() {
 }
 
 func traceHeapGoal() {
-       if heapGoal := atomic.Load64(&gcController.heapGoal); heapGoal == ^uint64(0) {
+       heapGoal := gcController.heapGoal()
+       if heapGoal == ^uint64(0) {
                // Heap-based triggering is disabled.
                traceEvent(traceEvHeapGoal, -1, 0)
        } else {