]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: split findRunnableGCWorker in two
authorMichael Pratt <mpratt@google.com>
Mon, 17 Nov 2025 20:09:50 +0000 (15:09 -0500)
committerGopher Robot <gobot@golang.org>
Thu, 20 Nov 2025 16:08:47 +0000 (08:08 -0800)
The first part, assignWaitingGCWorker selects a mark worker (if any) and
assigns it to the P. The second part, findRunnableGCWorker, is
responsible for actually marking the worker as runnable and updating the
CPU limiter.

The advantage of this split is that assignWaitingGCWorker is safe to do
during STW, which will allow the next CL to make selections during
procresize.

This change is a semantic no-op in preparation for the next CL.

For #65694.

Change-Id: I6a6a636c8beb212185829946cfa1e49f706ac31a
Reviewed-on: https://go-review.googlesource.com/c/go/+/721001
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>

src/runtime/mgc.go
src/runtime/mgcpacer.go
src/runtime/runtime2.go

index 43afbc330bb71cf88cb1b1cc2e1365afb09ce35c..febcd9558c09763ef0be646899c091508d85464d 100644 (file)
@@ -1727,7 +1727,13 @@ func gcBgMarkWorker(ready chan struct{}) {
        // the stack (see gopark). Prevent deadlock from recursively
        // starting GC by disabling preemption.
        gp.m.preemptoff = "GC worker init"
-       node := &new(gcBgMarkWorkerNodePadded).gcBgMarkWorkerNode // TODO: technically not allowed in the heap. See comment in tagptr.go.
+       // TODO: This is technically not allowed in the heap. See comment in tagptr.go.
+       //
+       // It is kept alive simply by virtue of being used in the infinite loop
+       // below. gcBgMarkWorkerPool keeps pointers to nodes that are not
+       // GC-visible, so this must be kept alive indefinitely (even if
+       // GOMAXPROCS decreases).
+       node := &new(gcBgMarkWorkerNodePadded).gcBgMarkWorkerNode
        gp.m.preemptoff = ""
 
        node.gp.set(gp)
index 32c1b941e538a3e3738bc72f2abe9b51b0ce26ed..f78fb6f636707bebbbe3743e43c8ff172162bd24 100644 (file)
@@ -10,7 +10,7 @@ import (
        "internal/runtime/atomic"
        "internal/runtime/math"
        "internal/strconv"
-       _ "unsafe" // for go:linkname
+       _ "unsafe"
 )
 
 const (
@@ -749,30 +749,33 @@ func (c *gcControllerState) enlistWorker() {
        }
 }
 
-// findRunnableGCWorker returns a background mark worker for pp if it
-// should be run. This must only be called when gcBlackenEnabled != 0.
-func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
+// assignWaitingGCWorker assigns a background mark worker to pp if one should
+// be run.
+//
+// If a worker is selected, it is assigned to pp.nextMarkGCWorker and the P is
+// wired as a GC mark worker. The G is still in _Gwaiting. If no worker is
+// selected, ok returns false.
+//
+// If assignedWaitingGCWorker returns true, this P must either:
+// - Mark the G as runnable and run it, clearing pp.nextMarkGCWorker.
+// - Or, call c.releaseNextGCMarkWorker.
+//
+// This must only be called when gcBlackenEnabled != 0.
+func (c *gcControllerState) assignWaitingGCWorker(pp *p, now int64) (bool, int64) {
        if gcBlackenEnabled == 0 {
                throw("gcControllerState.findRunnable: blackening not enabled")
        }
 
-       // Since we have the current time, check if the GC CPU limiter
-       // hasn't had an update in a while. This check is necessary in
-       // case the limiter is on but hasn't been checked in a while and
-       // so may have left sufficient headroom to turn off again.
        if now == 0 {
                now = nanotime()
        }
-       if gcCPULimiter.needUpdate(now) {
-               gcCPULimiter.update(now)
-       }
 
        if !gcShouldScheduleWorker(pp) {
                // No good reason to schedule a worker. This can happen at
                // the end of the mark phase when there are still
                // assists tapering off. Don't bother running a worker
                // now because it'll just return immediately.
-               return nil, now
+               return false, now
        }
 
        if c.dedicatedMarkWorkersNeeded.Load() <= 0 && c.fractionalUtilizationGoal == 0 {
@@ -783,7 +786,7 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
                // When a dedicated worker stops running, the gcBgMarkWorker loop notes
                // the need for the worker before returning it to the pool. If we don't
                // see the need now, we wouldn't have found it in the pool anyway.
-               return nil, now
+               return false, now
        }
 
        // Grab a worker before we commit to running below.
@@ -800,7 +803,7 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
                // it will always do so with queued global work. Thus, that P
                // will be immediately eligible to re-run the worker G it was
                // just using, ensuring work can complete.
-               return nil, now
+               return false, now
        }
 
        decIfPositive := func(val *atomic.Int64) bool {
@@ -823,7 +826,7 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
        } else if c.fractionalUtilizationGoal == 0 {
                // No need for fractional workers.
                gcBgMarkWorkerPool.push(&node.node)
-               return nil, now
+               return false, now
        } else {
                // Is this P behind on the fractional utilization
                // goal?
@@ -833,12 +836,48 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
                if delta > 0 && float64(pp.gcFractionalMarkTime.Load())/float64(delta) > c.fractionalUtilizationGoal {
                        // Nope. No need to run a fractional worker.
                        gcBgMarkWorkerPool.push(&node.node)
-                       return nil, now
+                       return false, now
                }
                // Run a fractional worker.
                pp.gcMarkWorkerMode = gcMarkWorkerFractionalMode
        }
 
+       pp.nextGCMarkWorker = node
+       return true, now
+}
+
+// findRunnableGCWorker returns a background mark worker for pp if it
+// should be run.
+//
+// If findRunnableGCWorker returns a G, this P is wired as a GC mark worker and
+// must run the G.
+//
+// This must only be called when gcBlackenEnabled != 0.
+//
+// This function is allowed to have write barriers because it is called from
+// the portion of findRunnable that always has a P.
+//
+//go:yeswritebarrierrec
+func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
+       // Since we have the current time, check if the GC CPU limiter
+       // hasn't had an update in a while. This check is necessary in
+       // case the limiter is on but hasn't been checked in a while and
+       // so may have left sufficient headroom to turn off again.
+       if now == 0 {
+               now = nanotime()
+       }
+       if gcCPULimiter.needUpdate(now) {
+               gcCPULimiter.update(now)
+       }
+
+       ok, now := c.assignWaitingGCWorker(pp, now)
+       if !ok {
+               return nil, now
+       }
+
+       node := pp.nextGCMarkWorker
+       pp.nextGCMarkWorker = nil
+
        // Run the background mark worker.
        gp := node.gp.ptr()
        trace := traceAcquire()
@@ -850,6 +889,23 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
        return gp, now
 }
 
+// Release an unused pp.nextGCMarkWorker, if any.
+//
+// This function is allowed to have write barriers because it is called from
+// the portion of schedule.
+//
+//go:yeswritebarrierrec
+func (c *gcControllerState) releaseNextGCMarkWorker(pp *p) {
+       node := pp.nextGCMarkWorker
+       if node == nil {
+               return
+       }
+
+       c.markWorkerStop(pp.gcMarkWorkerMode, 0)
+       gcBgMarkWorkerPool.push(&node.node)
+       pp.nextGCMarkWorker = nil
+}
+
 // resetLive sets up the controller state for the next mark phase after the end
 // of the previous one. Must be called after endCycle and before commit, before
 // the world is started.
index e415df6ec1a54f0c99a1ab38f31200109e50c494..56082bf7f507cc28c2bfde2b0bb2319ae4576e63 100644 (file)
@@ -854,6 +854,18 @@ type p struct {
        // mark worker started.
        gcMarkWorkerStartTime int64
 
+       // nextGCMarkWorker is the next mark worker to run. This may be set
+       // during start-the-world to assign a worker to this P. The P runs this
+       // worker on the next call to gcController.findRunnableGCWorker. If the
+       // P runs something else or stops, it must release this worker via
+       // gcController.releaseNextGCMarkWorker.
+       //
+       // See comment in gcBgMarkWorker about the lifetime of
+       // gcBgMarkWorkerNode.
+       //
+       // Only accessed by this P or during STW.
+       nextGCMarkWorker *gcBgMarkWorkerNode
+
        // gcw is this P's GC work buffer cache. The work buffer is
        // filled by write barriers, drained by mutator assists, and
        // disposed on certain GC state transitions.