From 9429aab9999e00958abd8b21d06fa4a2253437c2 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Sun, 11 Sep 2016 16:55:34 -0400 Subject: [PATCH] runtime: remove gcWork flushes in mark termination MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The only reason these flushes are still necessary at all is that gcmarknewobject doesn't flush its gcWork stats like it's supposed to. By changing gcmarknewobject to follow the standard protocol, the flushes become completely unnecessary because mark 2 ensures caches are flushed (and stay flushed) before we ever enter mark termination. In the garbage benchmark, this takes roughly 50 µs, which is surprisingly long for doing nothing. We still double-check after draining that they are in fact empty. Change-Id: Ia1c7cf98a53f72baa513792eb33eca6a0b4a7128 Reviewed-on: https://go-review.googlesource.com/31134 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot Reviewed-by: Rick Hudson --- src/runtime/mgc.go | 26 +++++--------------------- src/runtime/mgcmark.go | 5 +++++ 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index ee86c5a4fe..682790587b 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1146,11 +1146,6 @@ top: // this before waking blocked assists. atomic.Store(&gcBlackenEnabled, 0) - // Flush the gcWork caches. This must be done before - // endCycle since endCycle depends on statistics kept - // in these caches. - gcFlushGCWork() - // Wake all blocked assists. These will run when we // start the world again. gcWakeAllAssists() @@ -1160,6 +1155,8 @@ top: // world again. semrelease(&work.markDoneSema) + // endCycle depends on all gcWork cache stats being + // flushed. This is ensured by mark 2. gcController.endCycle() // Perform mark termination. This will restart the world. @@ -1540,18 +1537,8 @@ func gcMarkWorkAvailable(p *p) bool { return false } -// gcFlushGCWork disposes the gcWork caches of all Ps. The world must -// be stopped. -//go:nowritebarrier -func gcFlushGCWork() { - // Gather all cached GC work. All other Ps are stopped, so - // it's safe to manipulate their GC work caches. - for i := 0; i < int(gomaxprocs); i++ { - allp[i].gcw.dispose() - } -} - // gcMark runs the mark (or, for concurrent GC, mark termination) +// All gcWork caches must be empty. // STW is in effect at this point. //TODO go:nowritebarrier func gcMark(start_time int64) { @@ -1566,11 +1553,6 @@ func gcMark(start_time int64) { gcCopySpans() // TODO(rlh): should this be hoisted and done only once? Right now it is done for normal marking and also for checkmarking. - // Make sure the per-P gcWork caches are empty. During mark - // termination, these caches can still be used temporarily, - // but must be disposed to the global lists immediately. - gcFlushGCWork() - // Queue root marking jobs. gcMarkRootPrepare() @@ -1609,6 +1591,8 @@ func gcMark(start_time int64) { // Record that at least one root marking pass has completed. work.markrootDone = true + // Double-check that all gcWork caches are empty. This should + // be ensured by mark 2 before we enter mark termination. for i := 0; i < int(gomaxprocs); i++ { gcw := &allp[i].gcw if !gcw.empty() { diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index 5dd7c52b21..0cc87f8509 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -1386,6 +1386,11 @@ func gcmarknewobject(obj, size, scanSize uintptr) { gcw := &getg().m.p.ptr().gcw gcw.bytesMarked += uint64(size) gcw.scanWork += int64(scanSize) + if gcBlackenPromptly { + // There shouldn't be anything in the work queue, but + // we still need to flush stats. + gcw.dispose() + } } // Checkmarking -- 2.48.1