runtime: start dedicated mark workers even if there's no work

Currently, findRunnable only considers running a mark worker if
there's work in the work queue. In principle, this can delay the start
of the desired number of dedicated mark workers if there's no work
pending. This is unlikely to occur in practice, since there should be
work queued from the scan phase, but if it were to come up, a CPU hog
mutator could slow down or delay garbage collection.

This check makes sense for fractional mark workers, since they'll just
return to the scheduler immediately if there's no work, but we want
the scheduler to start all of the dedicated mark workers promptly,
even if there's currently no queued work. Hence, this change moves the
pending work check after the check for starting a dedicated worker.

Change-Id: I52b851cc9e41f508a0955b3f905ca80f109ea101
Reviewed-on: https://go-review.googlesource.com/9298
Reviewed-by: Rick Hudson <rlh@golang.org>

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index db7f859ddfe90fc3a5b7140e468b463d5925b92f..497c20f0ea5a581314ac50dc51e3084f418cde1c 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -481,13 +481,6 @@ func (c *gcControllerState) findRunnable(_p_ *p) *g {
                // immediately with no work to do.)
                return nil
        }
-       if work.full == 0 && work.partial == 0 {
-               // No work to be done right now. This can happen at
-               // the end of the mark phase when there are still
-               // assists tapering off. Don't bother running
-               // background mark because it'll just return immediately.
-               return nil
-       }
 
        decIfPositive := func(ptr *int64) bool {
                if *ptr > 0 {
@@ -507,7 +500,20 @@ func (c *gcControllerState) findRunnable(_p_ *p) *g {
                // TODO(austin): This P isn't going to run anything
                // else for a while, so kick everything out of its run
                // queue.
-       } else if decIfPositive(&c.fractionalMarkWorkersNeeded) {
+       } else {
+               if work.full == 0 && work.partial == 0 {
+                       // No work to be done right now. This can
+                       // happen at the end of the mark phase when
+                       // there are still assists tapering off. Don't
+                       // bother running background mark because
+                       // it'll just return immediately.
+                       return nil
+               }
+               if !decIfPositive(&c.fractionalMarkWorkersNeeded) {
+                       // No more workers are need right now.
+                       return nil
+               }
+
                // This P has picked the token for the fractional
                // worker. If this P were to run the worker for the
                // next time slice, then at the end of that time
@@ -533,9 +539,6 @@ func (c *gcControllerState) findRunnable(_p_ *p) *g {
                        return nil
                }
                _p_.gcMarkWorkerMode = gcMarkWorkerFractionalMode
-       } else {
-               // All workers that need to be running are running
-               return nil
        }
 
        // Run the background mark worker