runtime: convert gcController.heapLive to atomic type

Atomic operations are used even during STW for consistency.

For #53821.

Change-Id: Ibe7afe5cf893b1288ce24fc96b7691b1f81754ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/417775
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/runtime/align_runtime_test.go

@@ -23,7 +23,6 @@ var AtomicFields = []uintptr{
 	unsafe.Offsetof(schedt{}.timeToRun),
 	unsafe.Offsetof(gcControllerState{}.bgScanCredit),
 	unsafe.Offsetof(gcControllerState{}.maxStackScan),
-	unsafe.Offsetof(gcControllerState{}.heapLive),
 	unsafe.Offsetof(gcControllerState{}.heapScan),
 	unsafe.Offsetof(gcControllerState{}.dedicatedMarkTime),
 	unsafe.Offsetof(gcControllerState{}.dedicatedMarkWorkersNeeded),

src/runtime/export_test.go

@@ -1324,7 +1324,7 @@ func (c *GCController) StartCycle(stackSize, globalsSize uint64, scannableFrac f
 	}
 	c.maxStackScan = stackSize
 	c.globalsScan = globalsSize
-	c.heapLive = trigger
+	c.heapLive.Store(trigger)
 	c.heapScan += uint64(float64(trigger-c.heapMarked) * scannableFrac)
 	c.startCycle(0, gomaxprocs, gcTrigger{kind: gcTriggerHeap})
 }
@@ -1338,7 +1338,7 @@ func (c *GCController) HeapGoal() uint64 {
 }
 
 func (c *GCController) HeapLive() uint64 {
-	return c.heapLive
+	return c.heapLive.Load()
 }
 
 func (c *GCController) HeapMarked() uint64 {
@@ -1358,7 +1358,7 @@ type GCControllerReviseDelta struct {
 }
 
 func (c *GCController) Revise(d GCControllerReviseDelta) {
-	c.heapLive += uint64(d.HeapLive)
+	c.heapLive.Add(d.HeapLive)
 	c.heapScan += uint64(d.HeapScan)
 	c.heapScanWork.Add(d.HeapScanWork)
 	c.stackScanWork.Add(d.StackScanWork)

src/runtime/mgc.go

@@ -556,7 +556,7 @@ func (t gcTrigger) test() bool {
 		// atomically wrote gcController.heapLive anyway and we'll see our
 		// own write.
 		trigger, _ := gcController.trigger()
-		return atomic.Load64(&gcController.heapLive) >= trigger
+		return gcController.heapLive.Load() >= trigger
 	case gcTriggerTime:
 		if gcController.gcPercent.Load() < 0 {
 			return false
@@ -652,7 +652,7 @@ func gcStart(trigger gcTrigger) {
 		// so it can't be more than ncpu, even if GOMAXPROCS is.
 		work.stwprocs = ncpu
 	}
-	work.heap0 = atomic.Load64(&gcController.heapLive)
+	work.heap0 = gcController.heapLive.Load()
 	work.pauseNS = 0
 	work.mode = mode
 
@@ -924,7 +924,7 @@ func gcMarkTermination() {
 	// Start marktermination (write barrier remains enabled for now).
 	setGCPhase(_GCmarktermination)
 
-	work.heap1 = gcController.heapLive
+	work.heap1 = gcController.heapLive.Load()
 	startTime := nanotime()
 
 	mp := acquirem()
@@ -1565,7 +1565,7 @@ func gcResetMarkState() {
 	}
 
 	work.bytesMarked = 0
-	work.initialHeapLive = atomic.Load64(&gcController.heapLive)
+	work.initialHeapLive = gcController.heapLive.Load()
 }
 
 // Hooks for other packages

src/runtime/mgcpacer.go

@@ -8,7 +8,7 @@ import (
 	"internal/cpu"
 	"internal/goexperiment"
 	"runtime/internal/atomic"
-	"unsafe"
+	_ "unsafe" // for go:linkname
 )
 
 // go119MemoryLimitSupport is a feature flag for a number of changes
@@ -74,13 +74,6 @@ const (
 	memoryLimitHeapGoalHeadroom = 1 << 20
 )
 
-func init() {
-	if offset := unsafe.Offsetof(gcController.heapLive); offset%8 != 0 {
-		println(offset)
-		throw("gcController.heapLive not aligned to 8 bytes")
-	}
-}
-
 // gcController implements the GC pacing controller that determines
 // when to trigger concurrent garbage collection and how much marking
 // work to do in mutator assists and background marking.
@@ -193,23 +186,19 @@ type gcControllerState struct {
 	// hence goes up as we allocate and down as we sweep) while heapLive
 	// excludes these objects (and hence only goes up between GCs).
 	//
-	// This is updated atomically without locking. To reduce
+	// To reduce contention, this is updated only when obtaining a span
-	// contention, this is updated only when obtaining a span from
+	// from an mcentral and at this point it counts all of the unallocated
-	// an mcentral and at this point it counts all of the
+	// slots in that span (which will be allocated before that mcache
-	// unallocated slots in that span (which will be allocated
+	// obtains another span from that mcentral). Hence, it slightly
-	// before that mcache obtains another span from that
+	// overestimates the "true" live heap size. It's better to overestimate
-	// mcentral). Hence, it slightly overestimates the "true" live
+	// than to underestimate because 1) this triggers the GC earlier than
-	// heap size. It's better to overestimate than to
+	// necessary rather than potentially too late and 2) this leads to a
-	// underestimate because 1) this triggers the GC earlier than
+	// conservative GC rate rather than a GC rate that is potentially too
-	// necessary rather than potentially too late and 2) this
+	// low.
-	// 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
 	// this gcControllerState's revise() method.
-	heapLive uint64
+	heapLive atomic.Uint64
 
 	// heapScan is the number of bytes of "scannable" heap. This
 	// is the live heap (as counted by heapLive), but omitting
@@ -559,7 +548,7 @@ func (c *gcControllerState) revise() {
 		// act like GOGC is huge for the below calculations.
 		gcPercent = 100000
 	}
-	live := atomic.Load64(&c.heapLive)
+	live := c.heapLive.Load()
 	scan := atomic.Load64(&c.heapScan)
 	work := c.heapScanWork.Load() + c.stackScanWork.Load() + c.globalsScanWork.Load()
 
@@ -675,7 +664,7 @@ func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
 		utilization += float64(c.assistTime.Load()) / float64(assistDuration*int64(procs))
 	}
 
-	if c.heapLive <= c.triggered {
+	if c.heapLive.Load() <= c.triggered {
 		// Shouldn't happen, but let's be very safe about this in case the
 		// GC is somehow extremely short.
 		//
@@ -719,7 +708,7 @@ func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
 	//
 	// Note that because we only care about the ratio, assistDuration and procs cancel out.
 	scanWork := c.heapScanWork.Load() + c.stackScanWork.Load() + c.globalsScanWork.Load()
-	currentConsMark := (float64(c.heapLive-c.triggered) * (utilization + idleUtilization)) /
+	currentConsMark := (float64(c.heapLive.Load()-c.triggered) * (utilization + idleUtilization)) /
 		(float64(scanWork) * (1 - utilization))
 
 	// Update cons/mark controller. The time period for this is 1 GC cycle.
@@ -753,7 +742,8 @@ func (c *gcControllerState) endCycle(now int64, procs int, userForced bool) {
 		goal := gcGoalUtilization * 100
 		print("pacer: ", int(utilization*100), "% CPU (", int(goal), " exp.) for ")
 		print(c.heapScanWork.Load(), "+", c.stackScanWork.Load(), "+", c.globalsScanWork.Load(), " B work (", c.lastHeapScan+c.lastStackScan+c.globalsScan, " B exp.) ")
-		print("in ", c.triggered, " B -> ", c.heapLive, " B (∆goal ", int64(c.heapLive)-int64(c.lastHeapGoal), ", cons/mark ", oldConsMark, ")")
+		live := c.heapLive.Load()
+		print("in ", c.triggered, " B -> ", live, " B (∆goal ", int64(live)-int64(c.lastHeapGoal), ", cons/mark ", oldConsMark, ")")
 		if !ok {
 			print("[controller reset]")
 		}
@@ -900,7 +890,7 @@ func (c *gcControllerState) findRunnableGCWorker(pp *p, now int64) (*g, int64) {
 // The world must be stopped.
 func (c *gcControllerState) resetLive(bytesMarked uint64) {
 	c.heapMarked = bytesMarked
-	c.heapLive = bytesMarked
+	c.heapLive.Store(bytesMarked)
 	c.heapScan = uint64(c.heapScanWork.Load())
 	c.lastHeapScan = uint64(c.heapScanWork.Load())
 	c.lastStackScan = uint64(c.stackScanWork.Load())
@@ -908,7 +898,7 @@ func (c *gcControllerState) resetLive(bytesMarked uint64) {
 
 	// heapLive was updated, so emit a trace event.
 	if trace.enabled {
-		traceHeapAlloc()
+		traceHeapAlloc(bytesMarked)
 	}
 }
 
@@ -935,10 +925,10 @@ func (c *gcControllerState) markWorkerStop(mode gcMarkWorkerMode, duration int64
 
 func (c *gcControllerState) update(dHeapLive, dHeapScan int64) {
 	if dHeapLive != 0 {
-		atomic.Xadd64(&gcController.heapLive, dHeapLive)
+		live := gcController.heapLive.Add(dHeapLive)
 		if trace.enabled {
 			// gcController.heapLive changed.
-			traceHeapAlloc()
+			traceHeapAlloc(live)
 		}
 	}
 	if gcBlackenEnabled == 0 {
@@ -1260,7 +1250,7 @@ func (c *gcControllerState) commit(isSweepDone bool) {
 		// Concurrent sweep happens in the heap growth
 		// from gcController.heapLive to trigger. Make sure we
 		// give the sweeper some runway if it doesn't have enough.
-		c.sweepDistMinTrigger.Store(atomic.Load64(&c.heapLive) + sweepMinHeapDistance)
+		c.sweepDistMinTrigger.Store(c.heapLive.Load() + sweepMinHeapDistance)
 	}
 
 	// Compute the next GC goal, which is when the allocated heap

src/runtime/mgcsweep.go

@@ -177,7 +177,8 @@ func (a *activeSweep) end(sl sweepLocker) {
 				return
 			}
 			if debug.gcpacertrace > 0 {
-				print("pacer: sweep done at heap size ", gcController.heapLive>>20, "MB; allocated ", (gcController.heapLive-mheap_.sweepHeapLiveBasis)>>20, "MB during sweep; swept ", mheap_.pagesSwept.Load(), " pages at ", mheap_.sweepPagesPerByte, " pages/byte\n")
+				live := gcController.heapLive.Load()
+				print("pacer: sweep done at heap size ", live>>20, "MB; allocated ", (live-mheap_.sweepHeapLiveBasis)>>20, "MB during sweep; swept ", mheap_.pagesSwept.Load(), " pages at ", mheap_.sweepPagesPerByte, " pages/byte\n")
 			}
 			return
 		}
@@ -818,7 +819,7 @@ retry:
 	sweptBasis := mheap_.pagesSweptBasis.Load()
 
 	// Fix debt if necessary.
-	newHeapLive := uintptr(atomic.Load64(&gcController.heapLive)-mheap_.sweepHeapLiveBasis) + spanBytes
+	newHeapLive := uintptr(gcController.heapLive.Load()-mheap_.sweepHeapLiveBasis) + spanBytes
 	pagesTarget := int64(mheap_.sweepPagesPerByte*float64(newHeapLive)) - int64(callerSweepPages)
 	for pagesTarget > int64(mheap_.pagesSwept.Load()-sweptBasis) {
 		if sweepone() == ^uintptr(0) {
@@ -862,7 +863,7 @@ func gcPaceSweeper(trigger uint64) {
 		// trigger. Compute the ratio of in-use pages to sweep
 		// per byte allocated, accounting for the fact that
 		// some might already be swept.
-		heapLiveBasis := atomic.Load64(&gcController.heapLive)
+		heapLiveBasis := gcController.heapLive.Load()
 		heapDistance := int64(trigger) - int64(heapLiveBasis)
 		// Add a little margin so rounding errors and
 		// concurrent sweep are less likely to leave pages

src/runtime/trace.go

@@ -1335,8 +1335,8 @@ func traceGoSysBlock(pp *p) {
 	releasem(mp)
 }
 
-func traceHeapAlloc() {
+func traceHeapAlloc(live uint64) {
-	traceEvent(traceEvHeapAlloc, -1, gcController.heapLive)
+	traceEvent(traceEvHeapAlloc, -1, live)
 }
 
 func traceHeapGoal() {