From ce502b063cd810aa5897e6ce72d545591e9368a0 Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Wed, 22 Apr 2015 17:44:36 -0400
Subject: [PATCH] runtime: use park/ready to wake up GC at end of concurrent
 mark
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Currently, the main GC goroutine sleeps on a note during concurrent
mark and the first background mark worker or assist to finish marking
use wakes up that note to let the main goroutine proceed into mark
termination. Unfortunately, the latency of this wakeup can be quite
high, since the GC goroutine will typically have lost its P while in
the futex sleep, meaning it will be placed on the global run queue and
will wait there until some P is kind enough to pick it up. This delay
gives the mutator more time to allocate and create floating garbage,
growing the heap unnecessarily. Worse, it's likely that background
marking has stopped at this point (unless GOMAXPROCS>4), so anything
that's allocated and published to the heap during this window will
have to be scanned during mark termination while the world is stopped.

This change replaces the note sleep/wakeup with a gopark/ready
scheme. This keeps the wakeup inside the Go scheduler and lets the
garbage collector take advantage of the new scheduler semantics that
run the ready()d goroutine immediately when the ready()ing goroutine
sleeps.

For the json benchmark from x/benchmarks with GOMAXPROCS=4, this
reduces the delay in waking up the GC goroutine and entering mark
termination once concurrent marking is done from ~100ms to typically
<100µs.

Change-Id: Ib11f8b581b8914f2d68e0094f121e49bac3bb384
Reviewed-on: https://go-review.googlesource.com/9291
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
---
 src/runtime/mgc.go     | 48 ++++++++++++++++++++++++++++++++++++------
 src/runtime/mgcmark.go |  6 ++----
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 3bc56893b96..9bb1acad208 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -590,7 +590,13 @@ var work struct {
 
 	bgMarkReady note   // signal background mark worker has started
 	bgMarkDone  uint32 // cas to 1 when at a background mark completion point
-	bgMarkNote  note   // signal background mark completion
+
+	// Background mark completion signaling
+	bgMarkWake struct {
+		lock mutex
+		g    *g
+		wake bool
+	}
 
 	// Copy of mheap.allspans for marker or sweeper.
 	spans []*mspan
@@ -781,8 +787,18 @@ func gc(mode int) {
 		if debug.gctrace > 0 {
 			tMark = nanotime()
 		}
-		notetsleepg(&work.bgMarkNote, -1)
-		noteclear(&work.bgMarkNote)
+
+		// Wait for background mark completion.
+		lock(&work.bgMarkWake.lock)
+		if work.bgMarkWake.wake {
+			// Wakeup already happened
+			unlock(&work.bgMarkWake.lock)
+		} else {
+			work.bgMarkWake.g = getg()
+			goparkunlock(&work.bgMarkWake.lock, "mark wait (idle)", traceEvGoBlock, 1)
+		}
+		work.bgMarkWake.wake = false
+		work.bgMarkWake.g = nil
 
 		// Begin mark termination.
 		gctimer.cycle.markterm = nanotime()
@@ -1054,10 +1070,10 @@ func gcBgMarkWorker(p *p) {
 		}
 		gcw.dispose()
 
-		// If this is the first worker to reach a background
-		// completion point this cycle, signal the coordinator.
-		if done && cas(&work.bgMarkDone, 0, 1) {
-			notewakeup(&work.bgMarkNote)
+		// If this worker reached a background mark completion
+		// point, signal the main GC goroutine.
+		if done {
+			gcBgMarkDone()
 		}
 
 		duration := nanotime() - startTime
@@ -1073,6 +1089,24 @@ func gcBgMarkWorker(p *p) {
 	}
 }
 
+// gcBgMarkDone signals the completion of background marking. This can
+// be called multiple times during a cycle; only the first call has
+// any effect.
+func gcBgMarkDone() {
+	if cas(&work.bgMarkDone, 0, 1) {
+		// This is the first worker to reach completion.
+		// Signal the main GC goroutine.
+		lock(&work.bgMarkWake.lock)
+		if work.bgMarkWake.g == nil {
+			// It hasn't parked yet.
+			work.bgMarkWake.wake = true
+		} else {
+			ready(work.bgMarkWake.g, 0)
+		}
+		unlock(&work.bgMarkWake.lock)
+	}
+}
+
 // gcMark runs the mark (or, for concurrent GC, mark termination)
 // STW is in effect at this point.
 //TODO go:nowritebarrier
diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 4afdca432bd..5d5a0dab756 100644
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -246,10 +246,8 @@ func gcAssistAlloc(size uintptr, allowAssist bool) {
 		// signal a completion point.
 		if xadd(&work.nwait, +1) == work.nproc && work.full == 0 && work.partial == 0 {
 			// This has reached a background completion
-			// point. Is it the first this cycle?
-			if cas(&work.bgMarkDone, 0, 1) {
-				notewakeup(&work.bgMarkNote)
-			}
+			// point.
+			gcBgMarkDone()
 		}
 
 		duration := nanotime() - startTime