]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: convert gcController.heapLive to atomic type
authorMichael Pratt <mpratt@google.com>
Fri, 15 Jul 2022 18:21:02 +0000 (14:21 -0400)
committerMichael Pratt <mpratt@google.com>
Mon, 8 Aug 2022 14:11:09 +0000 (14:11 +0000)
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
src/runtime/export_test.go
src/runtime/mgc.go
src/runtime/mgcpacer.go
src/runtime/mgcsweep.go
src/runtime/trace.go

index ec7956d1bba55b0bb2b7ba677c8c813cd98cdfd1..6a9ffeffa448857127d1d78eceebf8db6eefb0e9 100644 (file)
@@ -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),
index ab0537d8b20f2b3b0bdc2dbefb5db579e65d342f..fd1e89609b79f69d3a8b820f5250fcc0548ce9cc 100644 (file)
@@ -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)
index 84a7216b108e5b4a50af3bd793628ddcb140a6e7..c35e9af05bf27ff8be824d36c4fecc665f7093bc 100644 (file)
@@ -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
index 77abee73dacda51ad96cb8709cefd57fd146e84f..29ee2d5909b902b27154e9ce9e4f53b59f22fc1a 100644 (file)
@@ -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
index 2ac5d507dde5c820f80a3459d9b5fa2de9281619..0e2cfdc9c4ee856dd83957e1b830543d7c64c4be 100644 (file)
@@ -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
index 9b12b42f1170243471b3c19f83383d58c94dcdbf..55e85be6e3c2673137a35e63089a46e40065c363 100644 (file)
@@ -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() {