]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: flush local_scan directly and more often
authorMichael Anthony Knyszek <mknyszek@google.com>
Thu, 23 Jul 2020 22:36:58 +0000 (22:36 +0000)
committerMichael Knyszek <mknyszek@google.com>
Mon, 26 Oct 2020 17:26:40 +0000 (17:26 +0000)
Now that local_scan is the last mcache-based statistic that is flushed
by purgecachedstats, and heap_scan and gcController.revise may be
interacted with concurrently, we don't need to flush heap_scan at
arbitrary locations where the heap is locked, and we don't need
purgecachedstats and cachestats anymore. Instead, we can flush
local_scan at the same time we update heap_live in refill, so the two
updates may share the same revise call.

Clean up unused functions, remove code that would cause the heap to get
locked in the allocSpan when it didn't need to (other than to flush
local_scan), and flush local_scan explicitly in a few important places.
Notably we need to flush local_scan whenever we flush the other stats,
but it doesn't need to be donated anywhere, so have releaseAll do the
flushing. Also, we need to flush local_scan before we set heap_scan at
the end of a GC, which was previously handled by cachestats. Just do so
explicitly -- it's not much code and it becomes a lot more clear why we
need to do so.

Change-Id: I35ac081784df7744d515479896a41d530653692d
Reviewed-on: https://go-review.googlesource.com/c/go/+/246968
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
src/runtime/mcache.go
src/runtime/mgc.go
src/runtime/mheap.go
src/runtime/mstats.go

index fe603116a2e49cce59b9525c48f5b24b4074f681..b8e388cc4f671c66fd9eb392dd6de8e1d4678f71 100644 (file)
@@ -124,7 +124,6 @@ func freemcache(c *mcache, recipient *mcache) {
                // gcworkbuffree(c.gcworkbuf)
 
                lock(&mheap_.lock)
-               purgecachedstats(c)
                // Donate anything else that's left.
                c.donate(recipient)
                mheap_.cachealloc.free(unsafe.Pointer(c))
@@ -135,6 +134,8 @@ func freemcache(c *mcache, recipient *mcache) {
 // donate flushes data and resources which have no global
 // pool to another mcache.
 func (c *mcache) donate(d *mcache) {
+       // local_scan is handled separately because it's not
+       // like these stats -- it's used for GC pacing.
        d.local_largealloc += c.local_largealloc
        c.local_largealloc = 0
        d.local_nlargealloc += c.local_nlargealloc
@@ -192,14 +193,22 @@ func (c *mcache) refill(spc spanClass) {
        // Assume all objects from this span will be allocated in the
        // mcache. If it gets uncached, we'll adjust this.
        c.local_nsmallalloc[spc.sizeclass()] += uintptr(s.nelems) - uintptr(s.allocCount)
+
+       // Update heap_live with the same assumption.
        usedBytes := uintptr(s.allocCount) * s.elemsize
        atomic.Xadd64(&memstats.heap_live, int64(s.npages*pageSize)-int64(usedBytes))
+
+       // While we're here, flush local_scan, since we have to call
+       // revise anyway.
+       atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan))
+       c.local_scan = 0
+
        if trace.enabled {
                // heap_live changed.
                traceHeapAlloc()
        }
        if gcBlackenEnabled != 0 {
-               // heap_live changed.
+               // heap_live and heap_scan changed.
                gcController.revise()
        }
 
@@ -248,6 +257,10 @@ func (c *mcache) largeAlloc(size uintptr, needzero bool, noscan bool) *mspan {
 }
 
 func (c *mcache) releaseAll() {
+       // Take this opportunity to flush local_scan.
+       atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan))
+       c.local_scan = 0
+
        sg := mheap_.sweepgen
        for i := range c.alloc {
                s := c.alloc[i]
@@ -273,6 +286,11 @@ func (c *mcache) releaseAll() {
        // Clear tinyalloc pool.
        c.tiny = 0
        c.tinyoffset = 0
+
+       // Updated heap_scan and possible heap_live.
+       if gcBlackenEnabled != 0 {
+               gcController.revise()
+       }
 }
 
 // prepareForSweep flushes c if the system has entered a new sweep phase
index c54f893689804f3ee9cfcd679fed0977a3f96b1c..55554c117c83b3d4d2370d4d2578ed2069105bfb 100644 (file)
@@ -2083,11 +2083,21 @@ func gcMark(start_time int64) {
                gcw.dispose()
        }
 
-       cachestats()
-
        // Update the marked heap stat.
        memstats.heap_marked = work.bytesMarked
 
+       // Flush local_scan from each mcache since we're about to modify
+       // heap_scan directly. If we were to flush this later, then local_scan
+       // might have incorrect information.
+       for _, p := range allp {
+               c := p.mcache
+               if c == nil {
+                       continue
+               }
+               memstats.heap_scan += uint64(c.local_scan)
+               c.local_scan = 0
+       }
+
        // Update other GC heap size stats. This must happen after
        // cachestats (which flushes local statistics to these) and
        // flushallmcaches (which modifies heap_live).
index 47f86ee38c1c6b1ae5815d7949a565f332980613..40fd58b0ef4ce4e01b2ce3acb7edb03539fea6fd 100644 (file)
@@ -1102,23 +1102,11 @@ func (h *mheap) allocSpan(npages uintptr, manual bool, spanclass spanClass, sysS
                base, scav = c.alloc(npages)
                if base != 0 {
                        s = h.tryAllocMSpan()
-
-                       if s != nil && gcBlackenEnabled == 0 && (manual || spanclass.sizeclass() != 0) {
+                       if s != nil {
                                goto HaveSpan
                        }
-                       // We're either running duing GC, failed to acquire a mspan,
-                       // or the allocation is for a large object. This means we
-                       // have to lock the heap and do a bunch of extra work,
-                       // so go down the HaveBaseLocked path.
-                       //
-                       // We must do this during GC to avoid skew with heap_scan
-                       // since we flush mcache stats whenever we lock.
-                       //
-                       // TODO(mknyszek): It would be nice to not have to
-                       // lock the heap if it's a large allocation, but
-                       // it's fine for now. The critical section here is
-                       // short and large object allocations are relatively
-                       // infrequent.
+                       // We have a base but no mspan, so we need
+                       // to lock the heap.
                }
        }
 
@@ -1145,30 +1133,6 @@ func (h *mheap) allocSpan(npages uintptr, manual bool, spanclass spanClass, sysS
                // one now that we have the heap lock.
                s = h.allocMSpanLocked()
        }
-       if !manual {
-               // This is a heap span, so we should do some additional accounting
-               // which may only be done with the heap locked.
-
-               // Transfer stats from mcache to global.
-               var c *mcache
-               if gp.m.p != 0 {
-                       c = gp.m.p.ptr().mcache
-               } else {
-                       // This case occurs while bootstrapping.
-                       // See the similar code in mallocgc.
-                       c = mcache0
-                       if c == nil {
-                               throw("mheap.allocSpan called with no P")
-                       }
-               }
-               atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan))
-               c.local_scan = 0
-
-               // heap_scan was been updated.
-               if gcBlackenEnabled != 0 {
-                       gcController.revise()
-               }
-       }
        unlock(&h.lock)
 
 HaveSpan:
@@ -1352,20 +1316,13 @@ func (h *mheap) grow(npage uintptr) bool {
 // Free the span back into the heap.
 func (h *mheap) freeSpan(s *mspan) {
        systemstack(func() {
-               c := getg().m.p.ptr().mcache
                lock(&h.lock)
-               atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan))
-               c.local_scan = 0
                if msanenabled {
                        // Tell msan that this entire span is no longer in use.
                        base := unsafe.Pointer(s.base())
                        bytes := s.npages << _PageShift
                        msanfree(base, bytes)
                }
-               if gcBlackenEnabled != 0 {
-                       // heap_scan changed.
-                       gcController.revise()
-               }
                h.freeSpanLocked(s, true, true)
                unlock(&h.lock)
        })
index 341906fcedae00212b0de1cd4be44563a3f9a36a..5eeb17364036ebfc98a7af086e423cb9866d975c 100644 (file)
@@ -556,9 +556,6 @@ func updatememstats() {
                memstats.by_size[i].nfree = 0
        }
 
-       // Aggregate local stats.
-       cachestats()
-
        // Collect allocation stats. This is safe and consistent
        // because the world is stopped.
        var smallFree, totalAlloc, totalFree uint64
@@ -602,21 +599,6 @@ func updatememstats() {
        memstats.heap_objects = memstats.nmalloc - memstats.nfree
 }
 
-// cachestats flushes all mcache stats.
-//
-// The world must be stopped.
-//
-//go:nowritebarrier
-func cachestats() {
-       for _, p := range allp {
-               c := p.mcache
-               if c == nil {
-                       continue
-               }
-               purgecachedstats(c)
-       }
-}
-
 // flushmcache flushes the mcache of allp[i].
 //
 // The world must be stopped.
@@ -643,13 +625,6 @@ func flushallmcaches() {
        }
 }
 
-//go:nosplit
-func purgecachedstats(c *mcache) {
-       // Protected by heap lock.
-       atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan))
-       c.local_scan = 0
-}
-
 // Atomically increases a given *system* memory stat. We are counting on this
 // stat never overflowing a uintptr, so this function must only be used for
 // system memory stats.