From bb6320535d6bcef1a5d44cb7429877166c4d5298 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 27 Mar 2015 17:01:53 -0400 Subject: [PATCH] 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 Reviewed-by: Rick Hudson --- src/runtime/malloc.go | 2 +- src/runtime/mgc.go | 42 ++++++++++++++++++++++++----------------- src/runtime/mgcmark.go | 2 +- src/runtime/proc1.go | 9 +++++---- src/runtime/runtime2.go | 5 +++++ 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 5896e74e91..91d69b5a9b 100644 --- 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 943a7233ae..d173e68a38 100644 --- 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 2b6e9a37d3..5483c68c56 100644 --- 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 9590895af3..0859015b0a 100644 --- 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 e4ac804b71..04ed059e19 100644 --- 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 */ -- 2.48.1