From: Michael Pratt Date: Fri, 15 Jul 2022 18:21:02 +0000 (-0400) Subject: runtime: convert gcController.heapLive to atomic type X-Git-Tag: go1.20rc1~1793 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=3a9281ff6181031adcc1d3991a1b1413db046430;p=gostls13.git 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 Reviewed-by: Michael Knyszek TryBot-Result: Gopher Robot --- diff --git a/src/runtime/align_runtime_test.go b/src/runtime/align_runtime_test.go index ec7956d1bb..6a9ffeffa4 100644 --- a/src/runtime/align_runtime_test.go +++ b/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), diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index ab0537d8b2..fd1e89609b 100644 --- a/src/runtime/export_test.go +++ b/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) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 84a7216b10..c35e9af05b 100644 --- a/src/runtime/mgc.go +++ b/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 diff --git a/src/runtime/mgcpacer.go b/src/runtime/mgcpacer.go index 77abee73da..29ee2d5909 100644 --- a/src/runtime/mgcpacer.go +++ b/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 - // contention, this is updated only when obtaining a span from - // an mcentral and at this point it counts all of the - // unallocated slots in that span (which will be allocated - // before that mcache obtains another span from that - // mcentral). Hence, it slightly overestimates the "true" live - // heap size. It's better to overestimate than to - // underestimate because 1) this triggers the GC earlier than - // necessary rather than potentially too late and 2) this - // leads to a conservative GC rate rather than a GC rate that - // is potentially too low. - // - // Reads should likewise be atomic (or during STW). + // To reduce contention, this is updated only when obtaining a span + // from an mcentral and at this point it counts all of the unallocated + // slots in that span (which will be allocated before that mcache + // obtains another span from that mcentral). Hence, it slightly + // overestimates the "true" live heap size. It's better to overestimate + // than to underestimate because 1) this triggers the GC earlier than + // necessary rather than potentially too late and 2) this leads to a + // conservative GC rate rather than a GC rate that is potentially too + // low. // // 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 diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 2ac5d507dd..0e2cfdc9c4 100644 --- a/src/runtime/mgcsweep.go +++ b/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 diff --git a/src/runtime/trace.go b/src/runtime/trace.go index 9b12b42f11..55e85be6e3 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -1335,8 +1335,8 @@ func traceGoSysBlock(pp *p) { releasem(mp) } -func traceHeapAlloc() { - traceEvent(traceEvHeapAlloc, -1, gcController.heapLive) +func traceHeapAlloc(live uint64) { + traceEvent(traceEvHeapAlloc, -1, live) } func traceHeapGoal() {