]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: look for idle p to run current goroutine when switching to GC or traceReader
authorDavid Chase <drchase@google.com>
Thu, 1 Nov 2018 21:31:20 +0000 (17:31 -0400)
committerDavid Chase <drchase@google.com>
Wed, 1 May 2019 16:10:05 +0000 (16:10 +0000)
This repairs one of the several causes of pauses uncovered
by a GC microbenchmark.  A pause can occur when a goroutine's
quantum expires "at the same time" a GC is needed.  The
current M switches to running a GC worker, which means that
the amount of available work has expanded by one.  The GC
worker, however, does not call ready, and does not itself
conditionally wake a P (a "normal" thread would do this).

This is also true if M switches to a traceReader.

This is problem 4 in this list:
https://github.com/golang/go/issues/27732#issuecomment-423301252

Updates #27732.

Change-Id: I6905365cac8504cde6faab2420f4421536551f0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/146817
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
src/runtime/proc.go

index 3bf39e03bfc877110bfa16ad51bd4bd87d9c010a..f314f0121151c2da3a52f1d9b3bdbb42dffd2861 100644 (file)
@@ -2487,15 +2487,22 @@ top:
 
        var gp *g
        var inheritTime bool
+
+       // Normal goroutines will check for need to wakeP in ready,
+       // but GCworkers and tracereaders will not, so the check must
+       // be done here instead.
+       tryWakeP := false
        if trace.enabled || trace.shutdown {
                gp = traceReader()
                if gp != nil {
                        casgstatus(gp, _Gwaiting, _Grunnable)
                        traceGoUnpark(gp, 0)
+                       tryWakeP = true
                }
        }
        if gp == nil && gcBlackenEnabled != 0 {
                gp = gcController.findRunnableGCWorker(_g_.m.p.ptr())
+               tryWakeP = tryWakeP || gp != nil
        }
        if gp == nil {
                // Check the global runnable queue once in a while to ensure fairness.
@@ -2541,6 +2548,13 @@ top:
                }
        }
 
+       // If about to schedule a not-normal goroutine (a GCworker or tracereader),
+       // wake a P if there is one.
+       if tryWakeP {
+               if atomic.Load(&sched.npidle) != 0 && atomic.Load(&sched.nmspinning) == 0 {
+                       wakep()
+               }
+       }
        if gp.lockedm != 0 {
                // Hands off own p to the locked m,
                // then blocks waiting for a new p.