runtime: Adjust when write barriers are active

Even though the world is stopped the GC may do pointer
writes that need to be protected by write barriers.
This means that the write barrier must be on
continuously from the time the mark phase starts and
the mark termination phase ends. Checks were added to
ensure that no allocation happens during a GC.

Hoist the logic that clears pools the start of the GC
so that the memory can be reclaimed during this GC cycle.

Change-Id: I9d1551ac5db9bac7bac0cb5370d5b2b19a9e6a52
Reviewed-on: https://go-review.googlesource.com/6990
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go
index 6a2c85aa9f6d15f5d82cf8f79321466c794d1e00..87ccc13df9b1e3a2e5c272680fbce6eb37c2e1d4 100644 (file)
--- a/src/runtime/malloc.go
+++ b/src/runtime/malloc.go
@@ -472,6 +472,9 @@ const (
 // Small objects are allocated from the per-P cache's free lists.
 // Large objects (> 32 kB) are allocated straight from the heap.
 func mallocgc(size uintptr, typ *_type, flags uint32) unsafe.Pointer {
+       if gcphase == _GCmarktermination {
+               throw("mallocgc called with gcphase == _GCmarktermination")
+       }
        shouldhelpgc := false
        if size == 0 {
                return unsafe.Pointer(&zerobase)

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 16f2e13200d807b09f1eab279b2607f4c87ddfe2..541dbc615dbbd16956d06b7a525afc7408210d0c 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -319,6 +319,9 @@ func gc(mode int) {
 
        systemstack(stoptheworld)
        systemstack(finishsweep_m) // finish sweep before we start concurrent scan.
+       // clearpools before we start the GC. If we wait they memory will not be
+       // reclaimed until the next GC cycle.
+       clearpools()
 
        if mode == gcBackgroundMode { // Do as much work concurrently as possible
                systemstack(func() {
@@ -345,7 +348,9 @@ func gc(mode int) {
                        // Begin mark termination.
                        gctimer.cycle.markterm = nanotime()
                        stoptheworld()
-                       gcphase = _GCoff
+                       // The gcphase is _GCmark, it will transition to _GCmarktermination
+                       // below. The important thing is that the wb remains active until
+                       // all marking is complete. This includes writes made by the GC.
                })
        } else {
                // For non-concurrent GC (mode != gcBackgroundMode)
@@ -354,14 +359,15 @@ func gc(mode int) {
                gcResetGState()
        }
 
+       // World is stopped.
+       // Start marktermination which includes enabling the write barrier.
+       gcphase = _GCmarktermination
+
        startTime := nanotime()
        if mp != acquirem() {
                throw("gcwork: rescheduled")
        }
 
-       // TODO(rsc): Should the concurrent GC clear pools earlier?
-       clearpools()
-
        _g_ := getg()
        _g_.m.traceback = 2
        gp := _g_.m.curg
@@ -383,6 +389,9 @@ func gc(mode int) {
                        gcMark(startTime)
                        clearCheckmarks()
                }
+
+               // marking is complete so we can turn the write barrier off
+               gcphase = _GCoff
                gcSweep(mode)
 
                if debug.gctrace > 1 {
@@ -392,7 +401,13 @@ func gc(mode int) {
                        // Reset these so that all stacks will be rescanned.
                        gcResetGState()
                        finishsweep_m()
+
+                       // Still in STW but gcphase is _GCoff, reset to _GCmarktermination
+                       // At this point all objects will be found during the gcMark which
+                       // does a complete STW mark and object scan.
+                       gcphase = _GCmarktermination
                        gcMark(startTime)
+                       gcphase = _GCoff // marking is done, turn off wb.
                        gcSweep(mode)
                }
        })
@@ -422,6 +437,10 @@ func gc(mode int) {
                }
        }
 
+       if gcphase != _GCoff {
+               throw("gc done but gcphase != _GCoff")
+       }
+
        systemstack(starttheworld)
 
        releasem(mp)
@@ -442,16 +461,17 @@ func gcMark(start_time int64) {
                tracegc()
        }
 
+       if gcphase != _GCmarktermination {
+               throw("in gcMark expecting to see gcphase as _GCmarktermination")
+       }
        t0 := start_time
        work.tstart = start_time
-       gcphase = _GCmarktermination
-
        var t1 int64
        if debug.gctrace > 0 {
                t1 = nanotime()
        }
 
-       gcCopySpans()
+       gcCopySpans() // TODO(rlh): should this be hoisted and done only once? Right now it is done for normal marking and also for checkmarking.
 
        work.nwait = 0
        work.ndone = 0
@@ -486,7 +506,6 @@ func gcMark(start_time int64) {
                throw("work.partial != 0")
        }
 
-       gcphase = _GCoff
        var t3 int64
        if debug.gctrace > 0 {
                t3 = nanotime()
@@ -556,6 +575,9 @@ func gcMark(start_time int64) {
 }
 
 func gcSweep(mode int) {
+       if gcphase != _GCoff {
+               throw("gcSweep being done but phase is not GCoff")
+       }
        gcCopySpans()
 
        lock(&mheap_.lock)