From: Austin Clements <austin@google.com>
Date: Fri, 21 Apr 2017 15:45:44 +0000 (-0400)
Subject: runtime: consistently use atomic loads for heap_live
X-Git-Tag: go1.9beta1~563
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=9d36163c0b35ddd384534f850fb04170e0d0c7c4;p=gostls13.git

runtime: consistently use atomic loads for heap_live

heap_live is updated atomically without locking, so we should also use
atomic loads to read it. Fix the reads of heap_live that happen
outside of STW to be atomic.

Change-Id: Idca9451c348168c2a792a9499af349833a3c333f
Reviewed-on: https://go-review.googlesource.com/41371
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
---

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 097b742a7b..70d5795441 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -537,7 +537,7 @@ func (c *gcControllerState) revise() {
 	}
 
 	// Compute the heap distance remaining.
-	heapDistance := int64(memstats.next_gc) - int64(memstats.heap_live)
+	heapDistance := int64(memstats.next_gc) - int64(atomic.Load64(&memstats.heap_live))
 	if heapDistance <= 0 {
 		// This shouldn't happen, but if it does, avoid
 		// dividing by zero or setting the assist negative.
@@ -1073,6 +1073,10 @@ func (t gcTrigger) test() bool {
 	}
 	switch t.kind {
 	case gcTriggerHeap:
+		// Non-atomic access to heap_live for performance. If
+		// we are going to trigger on this, this thread just
+		// atomically wrote heap_live anyway and we'll see our
+		// own write.
 		return memstats.heap_live >= memstats.gc_trigger
 	case gcTriggerTime:
 		lastgc := int64(atomic.Load64(&memstats.last_gc_nanotime))
@@ -1157,7 +1161,7 @@ func gcStart(mode gcMode, trigger gcTrigger) {
 	now := nanotime()
 	work.stwprocs, work.maxprocs = gcprocs(), gomaxprocs
 	work.tSweepTerm = now
-	work.heap0 = memstats.heap_live
+	work.heap0 = atomic.Load64(&memstats.heap_live)
 	work.pauseNS = 0
 	work.mode = mode
 
@@ -1985,7 +1989,7 @@ func gcResetMarkState() {
 	unlock(&allglock)
 
 	work.bytesMarked = 0
-	work.initialHeapLive = memstats.heap_live
+	work.initialHeapLive = atomic.Load64(&memstats.heap_live)
 	work.markrootDone = false
 }
 
diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go
index c2fa6ad9a9..ae8c1e39c1 100644
--- a/src/runtime/mstats.go
+++ b/src/runtime/mstats.go
@@ -121,6 +121,8 @@ type mstats struct {
 	// leads to a conservative GC rate rather than a GC rate that
 	// is potentially too low.
 	//
+	// Reads should likewise be atomic (or during STW).
+	//
 	// Whenever this is updated, call traceHeapAlloc() and
 	// gcController.revise().
 	heap_live uint64