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/6990Reviewed-by: Austin Clements <austin@google.com>

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)

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)