runtime: yield to GC coordinator after assist completion

Currently it's possible for the GC assist to signal completion of the
mark phase, which puts the GC coordinator goroutine on the current P's
run queue, and then return to mutator code that delays until the next
forced preemption before actually yielding control to the GC
coordinator, dragging out completion of the mark phase. This delay can
be further exacerbated if the mutator makes other goroutines runnable
before yielding control, since this will push the GC coordinator on
the back of the P's run queue.

To fix this, this adds a Gosched to the assist if it completed the
mark phase. This immediately and directly yields control to the GC
coordinator. This already happens implicitly in the background mark
workers because they park immediately after completing the mark.

This is one of the reasons completion of the mark phase is being
dragged out and allowing the mutator to allocate without assisting,
leading to the large heap goal overshoot in issue #11677. This is also
a prerequisite to making the assist block when it can't pay off its
debt.

Change-Id: I586adfbecb3ca042a37966752c1dc757f5c7fc78
Reviewed-on: https://go-review.googlesource.com/12670
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index eab13a99fc1e2eb54fdf6fb6dc305456fe04f1d8..d669b3bc461ec388dd5ecde5e0d4f99eaf3dd49e 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -706,6 +706,10 @@ func (s *bgMarkSignal) wait() {
 // complete signals the completion of this phase of marking. This can
 // be called multiple times during a cycle; only the first call has
 // any effect.
+//
+// The caller should arrange to deschedule itself as soon as possible
+// after calling complete in order to let the coordinator goroutine
+// run.
 func (s *bgMarkSignal) complete() {
        if cas(&s.done, 0, 1) {
                // This is the first worker to reach this completion point.

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 62f08814390ff992c9b3ad53154956a059f311aa..a26f93062e9d3c1640f0e49d843b0977a68c66f2 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -202,6 +202,7 @@ func gcAssistAlloc(size uintptr, allowAssist bool) {
        }
 
        // Perform assist work
+       completed := false
        systemstack(func() {
                if atomicload(&gcBlackenEnabled) == 0 {
                        // The gcBlackenEnabled check in malloc races with the
@@ -255,6 +256,7 @@ func gcAssistAlloc(size uintptr, allowAssist bool) {
                        } else {
                                work.bgMark1.complete()
                        }
+                       completed = true
                }
                duration := nanotime() - startTime
                _p_ := gp.m.p.ptr()
@@ -264,6 +266,12 @@ func gcAssistAlloc(size uintptr, allowAssist bool) {
                        _p_.gcAssistTime = 0
                }
        })
+
+       if completed {
+               // We called complete() above, so we should yield to
+               // the now-runnable GC coordinator.
+               Gosched()
+       }
 }
 
 //go:nowritebarrier