runtime: replace STW for enabling write barriers with ragged barrier

Currently, we use a full stop-the-world around enabling write
barriers. This is to ensure that all Gs have enabled write barriers
before any blackening occurs (either in gcBgMarkWorker() or in
gcAssistAlloc()).

However, there's no need to bring the whole world to a synchronous
stop to ensure this. This change replaces the STW with a ragged
barrier that ensures each P has individually observed that write
barriers should be enabled before GC performs any blackening.

Change-Id: If2f129a6a55bd8bdd4308067af2b739f3fb41955
Reviewed-on: https://go-review.googlesource.com/8207
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>

diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go
index 5896e74e919482e94c81af8bfce5c79e5db20c88..91d69b5a9bf75a858a3e885e56abe9ee4a795279 100644 (file)
--- a/src/runtime/malloc.go
+++ b/src/runtime/malloc.go
@@ -686,7 +686,7 @@ func mallocgc(size uintptr, typ *_type, flags uint32) unsafe.Pointer {
 
        if shouldtriggergc() {
                startGC(gcBackgroundMode)
-       } else if gcphase == _GCmark {
+       } else if gcBlackenEnabled != 0 {
                // Assist garbage collector. We delay this until the
                // epilogue so that it doesn't interfere with the
                // inner working of malloc such as mcache refills that

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 943a7233aef985d93dfc00a38708cceb8b523641..d173e68a38ccda687942dd8befa79128ede3e57f 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -465,10 +465,10 @@ func (c *gcControllerState) endCycle() {
 }
 
 // findRunnable returns the background mark worker for _p_ if it
-// should be run. This must only be called when gcphase == _GCmark.
+// should be run. This must only be called when gcBlackenEnabled != 0.
 func (c *gcControllerState) findRunnable(_p_ *p) *g {
-       if gcphase != _GCmark {
-               throw("gcControllerState.findRunnable: not in mark phase")
+       if gcBlackenEnabled == 0 {
+               throw("gcControllerState.findRunnable: blackening not enabled")
        }
        if _p_.gcBgMarkWorker == nil {
                throw("gcControllerState.findRunnable: no background mark worker")
@@ -764,23 +764,24 @@ func gc(mode int) {
                        gcscan_m()
                        gctimer.cycle.installmarkwb = nanotime()
 
-                       // Enter mark phase, enabling write barriers
-                       // and mutator assists.
-                       //
-                       // TODO: Elimate this STW. This requires
-                       // enabling write barriers in all mutators
-                       // before enabling any mutator assists or
-                       // background marking.
+                       // Enter mark phase. This enables write
+                       // barriers.
                        if debug.gctrace > 0 {
                                tInstallWB = nanotime()
                        }
-                       stoptheworld()
-                       gcBgMarkPrepare()
-                       gcphase = _GCmark
-
-                       // Concurrent mark.
-                       starttheworld()
+                       atomicstore(&gcphase, _GCmark)
+                       // Ensure all Ps have observed the phase
+                       // change and have write barriers enabled
+                       // before any blackening occurs.
+                       forEachP(func(*p) {})
                })
+               // Concurrent mark.
+               gcBgMarkPrepare() // Must happen before assist enable.
+               // At this point all Ps have enabled the mark phase
+               // write barrier, thus maintaining the no white to
+               // black invariant. Mutator assists and mark workers
+               // can now be enabled to safely blacken grey objects.
+               atomicstore(&gcBlackenEnabled, 1)
                gctimer.cycle.mark = nanotime()
                if debug.gctrace > 0 {
                        tMark = nanotime()
@@ -824,6 +825,7 @@ func gc(mode int) {
 
        // World is stopped.
        // Start marktermination which includes enabling the write barrier.
+       atomicstore(&gcBlackenEnabled, 0)
        gcphase = _GCmarktermination
 
        if debug.gctrace > 0 {
@@ -921,7 +923,7 @@ func gc(mode int) {
                // Update work.totaltime
                sweepTermCpu := int64(stwprocs) * (tScan - tSweepTerm)
                scanCpu := tInstallWB - tScan
-               installWBCpu := int64(stwprocs) * (tMark - tInstallWB)
+               installWBCpu := int64(0)
                // We report idle marking time below, but omit it from
                // the overall utilization here since it's "free".
                markCpu := gcController.assistTime + gcController.dedicatedMarkTime + gcController.fractionalMarkTime
@@ -1047,12 +1049,18 @@ func gcBgMarkWorker(p *p) {
                // dispose the gcw, and then preempt.
                mp = acquirem()
 
+               if gcBlackenEnabled == 0 {
+                       throw("gcBgMarkWorker: blackening not enabled")
+               }
+
                startTime := nanotime()
 
                xadd(&work.nwait, -1)
 
                done := false
                switch p.gcMarkWorkerMode {
+               default:
+                       throw("gcBgMarkWorker: unexpected gcMarkWorkerMode")
                case gcMarkWorkerDedicatedMode:
                        gcDrain(&p.gcw, gcBgCreditSlack)
                        // gcDrain did the xadd(&work.nwait +1) to

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 2b6e9a37d31eec300b5c5b58e77df150ce4415bf..5483c68c56dbead2fee967de38e36e99d131b9c1 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -172,7 +172,7 @@ func markroot(desc *parfor, i uint32) {
 // allowAssist is true, may assist GC scanning in proportion to the
 // allocations performed by this mutator since the last assist.
 //
-// It should only be called during gcphase == _GCmark.
+// It should only be called if gcAssistAlloc != 0.
 //
 // This must be called with preemption disabled.
 //go:nowritebarrier

diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index 9590895af32d6d878b1bbc2bdb4bb1c7b5c8c639..0859015b0ad86631e0db64cad4e504c59b3d0239 100644 (file)
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -1441,9 +1441,10 @@ top:
        }
 stop:
 
-       // We have nothing to do. If we're in the GC mark phase, run
-       // idle-time marking rather than give up the P.
-       if _p_ := _g_.m.p.ptr(); gcphase == _GCmark && _p_.gcBgMarkWorker != nil {
+       // We have nothing to do. If we're in the GC mark phaseand can
+       // safely scan and blacken objects, run idle-time marking
+       // rather than give up the P.
+       if _p_ := _g_.m.p.ptr(); gcBlackenEnabled != 0 && _p_.gcBgMarkWorker != nil {
                _p_.gcMarkWorkerMode = gcMarkWorkerIdleMode
                gp := _p_.gcBgMarkWorker
                casgstatus(gp, _Gwaiting, _Grunnable)
@@ -1596,7 +1597,7 @@ top:
                        resetspinning()
                }
        }
-       if gp == nil && gcphase == _GCmark {
+       if gp == nil && gcBlackenEnabled != 0 {
                gp = gcController.findRunnable(_g_.m.p.ptr())
                if gp != nil {
                        resetspinning()

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index e4ac804b710bb8dd451dedb73dab918e2f8276c1..04ed059e198bf8fbf08cf0c2e45a04e6172760e5 100644 (file)
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -533,6 +533,11 @@ type forcegcstate struct {
 
 var gcphase uint32
 
+// gcBlackenEnabled is 1 if mutator assists and background mark
+// workers are allowed to blacken objects. This must only be set when
+// gcphase == _GCmark.
+var gcBlackenEnabled uint32
+
 /*
  * known to compiler
  */