runtime: don't recheck heap trigger for periodic GC

88e945f introduced a non-speculative double check of the heap trigger
before actually starting a concurrent GC. This was necessary to fix a
race for heap-triggered GC, but broke sysmon-triggered periodic GC,
since the heap check will of course fail for periodically triggered
GC.

Fix this by telling startGC whether or not this GC was triggered by
heap size or a timer and only doing the heap size double check for GCs
triggered by heap size.

Fixes #12026.

Change-Id: I7c3f6ec364545c36d619f2b4b3bf3b758e3bcbd6
Reviewed-on: https://go-review.googlesource.com/13168
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go
index 40f672abb08845c7bda17f12addec2c00fb4780b..353f84083fcbf60b2ebf509959d5b887b3202941 100644 (file)
--- a/src/runtime/malloc.go
+++ b/src/runtime/malloc.go
@@ -702,7 +702,7 @@ func mallocgc(size uintptr, typ *_type, flags uint32) unsafe.Pointer {
        }
 
        if shouldhelpgc && shouldtriggergc() {
-               startGC(gcBackgroundMode)
+               startGC(gcBackgroundMode, false)
        } else if gcBlackenEnabled != 0 {
                // Assist garbage collector. We delay this until the
                // epilogue so that it doesn't interfere with the

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index f7e9908a8af8542c7e2449ff4fd39357fe9652f5..f7fd4e51d5fec859f6ab27100b8065665cfc03bc 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -811,7 +811,7 @@ var work struct {
 // garbage collection is complete. It may also block the entire
 // program.
 func GC() {
-       startGC(gcForceBlockMode)
+       startGC(gcForceBlockMode, false)
 }
 
 const (
@@ -820,7 +820,12 @@ const (
        gcForceBlockMode        // stop-the-world GC now and wait for sweep
 )
 
-func startGC(mode int) {
+// startGC starts a GC cycle. 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 int, 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
@@ -853,7 +858,7 @@ func startGC(mode int) {
        // recheck that this really should trigger GC. (For example,
        // we may have gone through a whole GC cycle since the
        // speculative check.)
-       if !shouldtriggergc() {
+       if !(forceTrigger || shouldtriggergc()) {
                unlock(&bggc.lock)
                return
        }

diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go
index be4d61215632ecaa849ee6a31a6a4d57d5e93cfa..bc4e7c127248ab267e51c95695e060c791b5b694 100644 (file)
--- a/src/runtime/mheap.go
+++ b/src/runtime/mheap.go
@@ -836,7 +836,7 @@ func mHeap_Scavenge(k int32, now, limit uint64) {
 
 //go:linkname runtime_debug_freeOSMemory runtime/debug.freeOSMemory
 func runtime_debug_freeOSMemory() {
-       startGC(gcForceBlockMode)
+       startGC(gcForceBlockMode, false)
        systemstack(func() { mHeap_Scavenge(-1, ^uint64(0), 0) })
 }
 

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 1a4c6c109f4315ae7906ab4d7481c50543130076..c5b4a8c9af9ff0e7f786ceaef40e23096fd58032 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -154,7 +154,7 @@ func forcegchelper() {
                if debug.gctrace > 0 {
                        println("GC forced")
                }
-               startGC(gcBackgroundMode)
+               startGC(gcBackgroundMode, true)
        }
 }