diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 23c15da413..230849609f 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -734,8 +734,8 @@ func mallocgc(size uintptr, typ *_type, flags uint32) unsafe.Pointer { assistG.gcAssistBytes -= int64(size - dataSize) } - if shouldhelpgc && shouldtriggergc() { - startGC(gcBackgroundMode, false) + if shouldhelpgc && gcShouldStart(false) { + gcStart(gcBackgroundMode, false) } else if shouldhelpgc && bggc.working != 0 && gcBlackenEnabled == 0 { // The GC is starting up or shutting down, so we can't // assist, but we also can't allocate unabated. Slow diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index b70d914125..2df3d45865 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -165,6 +165,7 @@ func gcinit() { datap.gcbssmask = progToPointerMask((*byte)(unsafe.Pointer(datap.gcbss)), datap.ebss-datap.bss) } memstats.next_gc = heapminimum + work.startSema = 1 } func readgogc() int32 { @@ -752,19 +753,6 @@ const gcAssistTimeSlack = 5000 // of future allocations. const gcOverAssistBytes = 1 << 20 -// Determine whether to initiate a GC. -// If the GC is already working no need to trigger another one. -// This should establish a feedback loop where if the GC does not -// have sufficient time to complete then more memory will be -// requested from the OS increasing heap size thus allow future -// GCs more time to complete. -// memstat.heap_live read has a benign race. -// A false negative simple does not start a GC, a false positive -// will start a GC needlessly. Neither have correctness issues. -func shouldtriggergc() bool { - return memstats.heap_live >= memstats.next_gc && atomicloaduint(&bggc.working) == 0 -} - // bgMarkSignal synchronizes the GC coordinator and background mark workers. type bgMarkSignal struct { // Workers race to cas to 1. Winner signals coordinator. @@ -840,6 +828,22 @@ var work struct { // STW GC, this happens during mark termination. finalizersDone bool + // Each type of GC state transition is protected by a lock. + // Since multiple threads can simultaneously detect the state + // transition condition, any thread that detects a transition + // condition must acquire the appropriate transition lock, + // re-check the transition condition and return if it no + // longer holds or perform the transition if it does. + // Likewise, any transition must invalidate the transition + // condition before releasing the lock. This ensures that each + // transition is performed by exactly one thread and threads + // that need the transition to happen block until it has + // happened. + // + // startSema protects the transition from "off" to mark or + // mark termination. + startSema uint32 + bgMarkReady note // signal background mark worker has started bgMarkDone uint32 // cas to 1 when at a background mark completion point // Background mark completion signaling @@ -898,7 +902,7 @@ var work struct { // garbage collection is complete. It may also block the entire // program. func GC() { - startGC(gcForceBlockMode, false) + gcStart(gcForceBlockMode, false) } // gcMode indicates how concurrent a GC cycle should be. @@ -910,34 +914,14 @@ const ( gcForceBlockMode // stop-the-world GC now and STW sweep ) -// startGC starts a GC cycle. If mode is gcBackgroundMode, this will +// startGCCoordinator starts and readies the GC coordinator goroutine. +// If mode is gcBackgroundMode, this will // start GC in the background and return. Otherwise, this will block -// until the new GC cycle is started and finishes. If forceTrigger is -// true, it indicates that GC should be started regardless of the -// current heap size. -func startGC(mode gcMode, forceTrigger bool) { - // The gc is turned off (via enablegc) until the bootstrap has completed. - // Also, malloc gets called in the guts of a number of libraries that might be - // holding locks. To avoid deadlocks during stop-the-world, don't bother - // trying to run gc while holding a lock. The next mallocgc without a lock - // will do the gc instead. - mp := acquirem() - if gp := getg(); gp == mp.g0 || mp.locks > 1 || mp.preemptoff != "" || !memstats.enablegc || panicking != 0 || gcpercent < 0 { - releasem(mp) - return - } - releasem(mp) - mp = nil - - // TODO: In gcstoptheworld debug mode, multiple goroutines may - // detect the heap trigger simultaneously and then start - // multiple STW GCs, which will run sequentially. - if debug.gcstoptheworld == 1 { - mode = gcForceMode - } else if debug.gcstoptheworld == 2 { - mode = gcForceBlockMode - } - +// until the new GC cycle is started and finishes. +// +// TODO(austin): This function is temporary and will go away when we +// finish the transition to the decentralized state machine. +func startGCCoordinator(mode gcMode) { if mode != gcBackgroundMode { // special synchronous cases gc(mode) @@ -947,14 +931,6 @@ func startGC(mode gcMode, forceTrigger bool) { // trigger concurrent GC readied := false lock(&bggc.lock) - // The trigger was originally checked speculatively, so - // recheck that this really should trigger GC. (For example, - // we may have gone through a whole GC cycle since the - // speculative check.) - if !(forceTrigger || shouldtriggergc()) { - unlock(&bggc.lock) - return - } if !bggc.started { bggc.working = 1 bggc.started = true @@ -993,6 +969,74 @@ func backgroundgc() { } } +// gcShouldStart returns true if the exit condition for the _GCoff +// phase has been met. The exit condition should be tested when +// allocating. +// +// If forceTrigger is true, it ignores the current heap size, but +// checks all other conditions. In general this should be false. +func gcShouldStart(forceTrigger bool) bool { + return gcphase == _GCoff && (forceTrigger || memstats.heap_live >= memstats.next_gc) && memstats.enablegc && panicking == 0 && gcpercent >= 0 +} + +// gcStart transitions the GC from _GCoff to _GCmark (if mode == +// gcBackgroundMode) or _GCmarktermination (if mode != +// gcBackgroundMode) by performing sweep termination and GC +// initialization. +// +// This may return without performing this transition in some cases, +// such as when called on a system stack or with locks held. +func gcStart(mode gcMode, forceTrigger bool) { + // Since this is called from malloc and malloc is called in + // the guts of a number of libraries that might be holding + // locks, don't attempt to start GC in non-preemptible or + // potentially unstable situations. + mp := acquirem() + if gp := getg(); gp == mp.g0 || mp.locks > 1 || mp.preemptoff != "" { + releasem(mp) + return + } + releasem(mp) + mp = nil + + // Perform GC initialization and the sweep termination + // transition. + // + // If this is a forced GC, don't acquire the transition lock + // or re-check the transition condition because we + // specifically *don't* want to share the transition with + // another thread. + useStartSema := mode == gcBackgroundMode + if useStartSema { + semacquire(&work.startSema, false) + // Re-check transition condition under transition lock. + if !gcShouldStart(forceTrigger) { + semrelease(&work.startSema) + return + } + } + + // In gcstoptheworld debug mode, upgrade the mode accordingly. + // We do this after re-checking the transition condition so + // that multiple goroutines that detect the heap trigger don't + // start multiple STW GCs. + if mode == gcBackgroundMode { + if debug.gcstoptheworld == 1 { + mode = gcForceMode + } else if debug.gcstoptheworld == 2 { + mode = gcForceBlockMode + } + } + + // TODO: Move sweep termination and initialization from the + // coordinator to here. + startGCCoordinator(mode) + + if useStartSema { + semrelease(&work.startSema) + } +} + func gc(mode gcMode) { // Ok, we're doing it! Stop everybody else semacquire(&worldsema, false) diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 08b10ee925..907c27b3a6 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -874,7 +874,7 @@ func mHeap_Scavenge(k int32, now, limit uint64) { //go:linkname runtime_debug_freeOSMemory runtime/debug.freeOSMemory func runtime_debug_freeOSMemory() { - startGC(gcForceBlockMode, false) + gcStart(gcForceBlockMode, false) systemstack(func() { mHeap_Scavenge(-1, ^uint64(0), 0) }) } diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 41e5ea9751..eb0eac837f 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -171,7 +171,7 @@ func forcegchelper() { if debug.gctrace > 0 { println("GC forced") } - startGC(gcBackgroundMode, true) + gcStart(gcBackgroundMode, true) } } diff --git a/src/runtime/trace.go b/src/runtime/trace.go index 6631bc29d1..06bdf970ec 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -773,7 +773,7 @@ func traceProcStop(pp *p) { } func traceGCStart() { - traceEvent(traceEvGCStart, 4) + traceEvent(traceEvGCStart, 5) } func traceGCDone() {