From 8cc280aa727bc7159adfdd083861472aa3066a35 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 20:13:49 +0000 Subject: runtime: define and enforce synchronization on heap_scan Currently heap_scan is mostly protected by the heap lock, but gcControllerState.revise sometimes accesses it without a lock. In an effort to make gcControllerState.revise callable from more contexts (and have its synchronization guarantees actually respected), make heap_scan atomically read from and written to, unless the world is stopped. Note that we don't update gcControllerState.revise's erroneous doc comment here because this change isn't about revise's guarantees, just about heap_scan. The comment is updated in a later change. Change-Id: Iddbbeb954767c704c2bd1d221f36e6c4fc9948a6 Reviewed-on: https://go-review.googlesource.com/c/go/+/246960 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Emmanuel Odeke Reviewed-by: Michael Pratt --- src/runtime/mheap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 1a57bcd66e..124bbacd1d 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1168,7 +1168,7 @@ func (h *mheap) allocSpan(npages uintptr, manual bool, spanclass spanClass, sysS throw("mheap.allocSpan called with no P") } } - memstats.heap_scan += uint64(c.local_scan) + atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) c.local_scan = 0 memstats.tinyallocs += uint64(c.local_tinyallocs) c.local_tinyallocs = 0 @@ -1375,7 +1375,7 @@ func (h *mheap) freeSpan(s *mspan) { systemstack(func() { c := getg().m.p.ptr().mcache lock(&h.lock) - memstats.heap_scan += uint64(c.local_scan) + atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) c.local_scan = 0 memstats.tinyallocs += uint64(c.local_tinyallocs) c.local_tinyallocs = 0 -- cgit v1.3 From 42019613df2d9b6ad39e8ccf80861e75666025a0 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 21:02:05 +0000 Subject: runtime: make distributed/local malloc stats the source-of-truth This change makes it so that various local malloc stats (excluding heap_scan and local_tinyallocs) are no longer written first to mheap fields but are instead accessed directly from each mcache. This change is part of a move toward having stats be distributed, and cleaning up some old code related to the stats. Note that because there's no central source-of-truth, when an mcache dies, it must donate its stats to another mcache. It's always safe to donate to the mcache for the 0th P, so do that. Change-Id: I2556093dbc27357cb9621c9b97671f3c00aa1173 Reviewed-on: https://go-review.googlesource.com/c/go/+/246964 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/export_test.go | 26 ++++++++++++++++++-------- src/runtime/mcache.go | 31 +++++++++++++++++++++++++++++-- src/runtime/mheap.go | 7 ++----- src/runtime/mstats.go | 41 +++++++++++++++++++---------------------- src/runtime/proc.go | 2 +- 5 files changed, 69 insertions(+), 38 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index e65b7b8ea7..d5a90ca65b 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -339,18 +339,28 @@ func ReadMemStatsSlow() (base, slow MemStats) { // Add in frees. readmemstats_m flushed the cached stats, so // these are up-to-date. - var smallFree uint64 - slow.Frees = mheap_.nlargefree - for i := range mheap_.nsmallfree { - slow.Frees += mheap_.nsmallfree[i] - bySize[i].Frees = mheap_.nsmallfree[i] - bySize[i].Mallocs += mheap_.nsmallfree[i] - smallFree += mheap_.nsmallfree[i] * uint64(class_to_size[i]) + var largeFree, smallFree uint64 + for _, p := range allp { + c := p.mcache + if c == nil { + continue + } + // Collect large allocation stats. + largeFree += uint64(c.local_largefree) + slow.Frees += uint64(c.local_nlargefree) + + // Collect per-sizeclass stats. + for i := 0; i < _NumSizeClasses; i++ { + slow.Frees += uint64(c.local_nsmallfree[i]) + bySize[i].Frees += uint64(c.local_nsmallfree[i]) + bySize[i].Mallocs += uint64(c.local_nsmallfree[i]) + smallFree += uint64(c.local_nsmallfree[i]) * uint64(class_to_size[i]) + } } slow.Frees += memstats.tinyallocs slow.Mallocs += slow.Frees - slow.TotalAlloc = slow.Alloc + mheap_.largefree + smallFree + slow.TotalAlloc = slow.Alloc + largeFree + smallFree for i := range slow.BySize { slow.BySize[i].Mallocs = bySize[i].Mallocs diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 7a7d33ccae..5baa7b3da8 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -41,7 +41,13 @@ type mcache struct { stackcache [_NumStackOrders]stackfreelist - // Local allocator stats, flushed during GC. + // Allocator stats (source-of-truth). + // Only the P that owns this mcache may write to these + // variables, so it's safe for that P to read non-atomically. + // + // When read with stats from other mcaches and with the world + // stopped, the result will accurately reflect the state of the + // application. local_largefree uintptr // bytes freed for large objects (>maxsmallsize) local_nlargefree uintptr // number of frees for large objects (>maxsmallsize) local_nsmallfree [_NumSizeClasses]uintptr // number of frees for small objects (<=maxsmallsize) @@ -97,7 +103,13 @@ func allocmcache() *mcache { return c } -func freemcache(c *mcache) { +// freemcache releases resources associated with this +// mcache and puts the object onto a free list. +// +// In some cases there is no way to simply release +// resources, such as statistics, so donate them to +// a different mcache (the recipient). +func freemcache(c *mcache, recipient *mcache) { systemstack(func() { c.releaseAll() stackcache_clear(c) @@ -109,11 +121,26 @@ func freemcache(c *mcache) { lock(&mheap_.lock) purgecachedstats(c) + // Donate anything else that's left. + c.donate(recipient) mheap_.cachealloc.free(unsafe.Pointer(c)) unlock(&mheap_.lock) }) } +// donate flushes data and resources which have no global +// pool to another mcache. +func (c *mcache) donate(d *mcache) { + d.local_largefree += c.local_largefree + c.local_largefree = 0 + d.local_nlargefree += c.local_nlargefree + c.local_nlargefree = 0 + for i := range c.local_nsmallfree { + d.local_nsmallfree[i] += c.local_nsmallfree[i] + c.local_nsmallfree[i] = 0 + } +} + // refill acquires a new span of span class spc for c. This span will // have at least one free object. The current span in c must be full. // diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 124bbacd1d..1b41b204ab 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -129,11 +129,8 @@ type mheap struct { reclaimCredit uintptr // Malloc stats. - largealloc uint64 // bytes allocated for large objects - nlargealloc uint64 // number of large object allocations - largefree uint64 // bytes freed for large objects (>maxsmallsize) - nlargefree uint64 // number of frees for large objects (>maxsmallsize) - nsmallfree [_NumSizeClasses]uint64 // number of frees for small objects (<=maxsmallsize) + largealloc uint64 // bytes allocated for large objects + nlargealloc uint64 // number of large object allocations // arenas is the heap arena map. It points to the metadata for // the heap for every arena frame of the entire usable virtual diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 8cc20552fb..d81d2ebe81 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -571,21 +571,27 @@ func updatememstats() { memstats.by_size[i].nmalloc += c.nmalloc totalAlloc += c.nmalloc * uint64(class_to_size[i]) } - // Collect per-sizeclass stats. - for i := 0; i < _NumSizeClasses; i++ { - if i == 0 { - memstats.nmalloc += mheap_.nlargealloc - totalAlloc += mheap_.largealloc - totalFree += mheap_.largefree - memstats.nfree += mheap_.nlargefree + + for _, p := range allp { + c := p.mcache + if c == nil { continue } - - // The mcache stats have been flushed to mheap_. - memstats.nfree += mheap_.nsmallfree[i] - memstats.by_size[i].nfree = mheap_.nsmallfree[i] - smallFree += mheap_.nsmallfree[i] * uint64(class_to_size[i]) + // Collect large allocation stats. + totalFree += uint64(c.local_largefree) + memstats.nfree += uint64(c.local_nlargefree) + + // Collect per-sizeclass stats. + for i := 0; i < _NumSizeClasses; i++ { + memstats.nfree += uint64(c.local_nsmallfree[i]) + memstats.by_size[i].nfree += uint64(c.local_nsmallfree[i]) + smallFree += uint64(c.local_nsmallfree[i]) * uint64(class_to_size[i]) + } } + // Collect remaining large allocation stats. + memstats.nmalloc += mheap_.nlargealloc + totalAlloc += mheap_.largealloc + totalFree += smallFree memstats.nfree += memstats.tinyallocs @@ -641,20 +647,11 @@ func flushallmcaches() { //go:nosplit func purgecachedstats(c *mcache) { - // Protected by either heap or GC lock. - h := &mheap_ + // Protected by heap lock. atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) c.local_scan = 0 memstats.tinyallocs += uint64(c.local_tinyallocs) c.local_tinyallocs = 0 - h.largefree += uint64(c.local_largefree) - c.local_largefree = 0 - h.nlargefree += uint64(c.local_nlargefree) - c.local_nlargefree = 0 - for i := 0; i < len(c.local_nsmallfree); i++ { - h.nsmallfree[i] += uint64(c.local_nsmallfree[i]) - c.local_nsmallfree[i] = 0 - } } // Atomically increases a given *system* memory stat. We are counting on this diff --git a/src/runtime/proc.go b/src/runtime/proc.go index ebecc92745..4f4cff38aa 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -4550,7 +4550,7 @@ func (pp *p) destroy() { pp.mspancache.len = 0 pp.pcache.flush(&mheap_.pages) }) - freemcache(pp.mcache) + freemcache(pp.mcache, allp[0].mcache) pp.mcache = nil gfpurge(pp) traceProcFree(pp) -- cgit v1.3 From e63716bc76d3264f669843434bc365a78f2141d2 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 21:10:29 +0000 Subject: runtime: make nlargealloc and largealloc mcache fields This change makes nlargealloc and largealloc into mcache fields just like nlargefree and largefree. These local fields become the new source-of-truth. This change also moves the accounting for these fields out of allocSpan (which is an inappropriate place for it -- this accounting generally happens much closer to the point of allocation) and into largeAlloc. This move is partially possible now that we can call gcController.revise at that point. Furthermore, this change moves largeAlloc into mcache.go and makes it a method of mcache. While there's a little bit of a mismatch here because largeAlloc barely interacts with the mcache, it helps solidify the mcache as the first allocation layer and provides a clear place to aggregate and manage statistics. Change-Id: I37b5e648710733bb4c04430b71e96700e438587a Reviewed-on: https://go-review.googlesource.com/c/go/+/246965 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/malloc.go | 33 +------------------------------ src/runtime/mcache.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++--- src/runtime/mheap.go | 18 +---------------- src/runtime/mstats.go | 4 ++-- 4 files changed, 55 insertions(+), 54 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index b19d1f2671..ec601ccb39 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -1082,9 +1082,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { } } else { shouldhelpgc = true - systemstack(func() { - span = largeAlloc(size, needzero, noscan) - }) + span = c.largeAlloc(size, needzero, noscan) span.freeindex = 1 span.allocCount = 1 x = unsafe.Pointer(span.base()) @@ -1179,35 +1177,6 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { return x } -func largeAlloc(size uintptr, needzero bool, noscan bool) *mspan { - // print("largeAlloc size=", size, "\n") - - if size+_PageSize < size { - throw("out of memory") - } - npages := size >> _PageShift - if size&_PageMask != 0 { - npages++ - } - - // Deduct credit for this span allocation and sweep if - // necessary. mHeap_Alloc will also sweep npages, so this only - // pays the debt down to npage pages. - deductSweepCredit(npages*_PageSize, npages) - - spc := makeSpanClass(0, noscan) - s := mheap_.alloc(npages, spc, needzero) - if s == nil { - throw("out of memory") - } - // Put the large span in the mcentral swept list so that it's - // visible to the background sweeper. - mheap_.central[spc].mcentral.fullSwept(mheap_.sweepgen).push(s) - s.limit = s.base() + size - heapBitsForAddr(s.base()).initSpan(s) - return s -} - // implementation of new builtin // compiler (both frontend and SSA backend) knows the signature // of this function diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 5baa7b3da8..3657c0b86a 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -10,6 +10,7 @@ import ( ) // Per-thread (in Go, per-P) cache for small objects. +// This includes a small object cache and local allocation stats. // No locking needed because it is per-thread (per-P). // // mcaches are allocated from non-GC'd memory, so any heap pointers @@ -48,9 +49,11 @@ type mcache struct { // When read with stats from other mcaches and with the world // stopped, the result will accurately reflect the state of the // application. - local_largefree uintptr // bytes freed for large objects (>maxsmallsize) - local_nlargefree uintptr // number of frees for large objects (>maxsmallsize) - local_nsmallfree [_NumSizeClasses]uintptr // number of frees for small objects (<=maxsmallsize) + local_largealloc uintptr // bytes allocated for large objects + local_nlargealloc uintptr // number of large object allocations + local_largefree uintptr // bytes freed for large objects (>maxsmallsize) + local_nlargefree uintptr // number of frees for large objects (>maxsmallsize) + local_nsmallfree [_NumSizeClasses]uintptr // number of frees for small objects (<=maxsmallsize) // flushGen indicates the sweepgen during which this mcache // was last flushed. If flushGen != mheap_.sweepgen, the spans @@ -131,6 +134,10 @@ 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) { + d.local_largealloc += c.local_largealloc + c.local_largealloc = 0 + d.local_nlargealloc += c.local_nlargealloc + c.local_nlargealloc = 0 d.local_largefree += c.local_largefree c.local_largefree = 0 d.local_nlargefree += c.local_nlargefree @@ -178,6 +185,47 @@ func (c *mcache) refill(spc spanClass) { c.alloc[spc] = s } +// largeAlloc allocates a span for a large object. +func (c *mcache) largeAlloc(size uintptr, needzero bool, noscan bool) *mspan { + if size+_PageSize < size { + throw("out of memory") + } + npages := size >> _PageShift + if size&_PageMask != 0 { + npages++ + } + + // Deduct credit for this span allocation and sweep if + // necessary. mHeap_Alloc will also sweep npages, so this only + // pays the debt down to npage pages. + deductSweepCredit(npages*_PageSize, npages) + + spc := makeSpanClass(0, noscan) + s := mheap_.alloc(npages, spc, needzero) + if s == nil { + throw("out of memory") + } + c.local_largealloc += npages * pageSize + c.local_nlargealloc++ + + // Update heap_live and revise pacing if needed. + atomic.Xadd64(&memstats.heap_live, int64(npages*pageSize)) + if trace.enabled { + // Trace that a heap alloc occurred because heap_live changed. + traceHeapAlloc() + } + if gcBlackenEnabled != 0 { + gcController.revise() + } + + // Put the large span in the mcentral swept list so that it's + // visible to the background sweeper. + mheap_.central[spc].mcentral.fullSwept(mheap_.sweepgen).push(s) + s.limit = s.base() + size + heapBitsForAddr(s.base()).initSpan(s) + return s +} + func (c *mcache) releaseAll() { for i := range c.alloc { s := c.alloc[i] diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 1b41b204ab..5635dc6784 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -128,10 +128,6 @@ type mheap struct { // This is accessed atomically. reclaimCredit uintptr - // Malloc stats. - largealloc uint64 // bytes allocated for large objects - nlargealloc uint64 // number of large object allocations - // arenas is the heap arena map. It points to the metadata for // the heap for every arena frame of the entire usable virtual // address space. @@ -1170,14 +1166,7 @@ func (h *mheap) allocSpan(npages uintptr, manual bool, spanclass spanClass, sysS memstats.tinyallocs += uint64(c.local_tinyallocs) c.local_tinyallocs = 0 - // Do some additional accounting if it's a large allocation. - if spanclass.sizeclass() == 0 { - mheap_.largealloc += uint64(npages * pageSize) - mheap_.nlargealloc++ - atomic.Xadd64(&memstats.heap_live, int64(npages*pageSize)) - } - - // Either heap_live or heap_scan could have been updated. + // heap_scan was been updated. if gcBlackenEnabled != 0 { gcController.revise() } @@ -1277,11 +1266,6 @@ HaveSpan: // Update related page sweeper stats. atomic.Xadd64(&h.pagesInUse, int64(npages)) - - if trace.enabled { - // Trace that a heap alloc occurred. - traceHeapAlloc() - } } // Make sure the newly allocated span will be observed diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index d81d2ebe81..d9acb361d5 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -578,6 +578,8 @@ func updatememstats() { continue } // Collect large allocation stats. + memstats.nmalloc += uint64(c.local_nlargealloc) + totalAlloc += uint64(c.local_largealloc) totalFree += uint64(c.local_largefree) memstats.nfree += uint64(c.local_nlargefree) @@ -589,8 +591,6 @@ func updatememstats() { } } // Collect remaining large allocation stats. - memstats.nmalloc += mheap_.nlargealloc - totalAlloc += mheap_.largealloc totalFree += smallFree -- cgit v1.3 From cca3d1e5533cb40beb9ef55bbc332b733adcc6ba Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 22:16:46 +0000 Subject: runtime: don't flush local_tinyallocs This change makes local_tinyallocs work like the rest of the malloc stats and doesn't flush local_tinyallocs, instead making that the source-of-truth. Change-Id: I3e6cb5f1b3d086e432ce7d456895511a48e3617a Reviewed-on: https://go-review.googlesource.com/c/go/+/246967 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/export_test.go | 7 +++++-- src/runtime/mcache.go | 8 +++++--- src/runtime/mheap.go | 4 ---- src/runtime/mstats.go | 6 ++++-- 4 files changed, 14 insertions(+), 11 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index d5a90ca65b..d71b180f76 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -339,7 +339,7 @@ func ReadMemStatsSlow() (base, slow MemStats) { // Add in frees. readmemstats_m flushed the cached stats, so // these are up-to-date. - var largeFree, smallFree uint64 + var tinyAllocs, largeFree, smallFree uint64 for _, p := range allp { c := p.mcache if c == nil { @@ -349,6 +349,9 @@ func ReadMemStatsSlow() (base, slow MemStats) { largeFree += uint64(c.local_largefree) slow.Frees += uint64(c.local_nlargefree) + // Collect tiny allocation stats. + tinyAllocs += uint64(c.local_tinyallocs) + // Collect per-sizeclass stats. for i := 0; i < _NumSizeClasses; i++ { slow.Frees += uint64(c.local_nsmallfree[i]) @@ -357,7 +360,7 @@ func ReadMemStatsSlow() (base, slow MemStats) { smallFree += uint64(c.local_nsmallfree[i]) * uint64(class_to_size[i]) } } - slow.Frees += memstats.tinyallocs + slow.Frees += tinyAllocs slow.Mallocs += slow.Frees slow.TotalAlloc = slow.Alloc + largeFree + smallFree diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 4d2ba6dff0..fe603116a2 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -32,9 +32,8 @@ type mcache struct { // tiny is a heap pointer. Since mcache is in non-GC'd memory, // we handle it by clearing it in releaseAll during mark // termination. - tiny uintptr - tinyoffset uintptr - local_tinyallocs uintptr // number of tiny allocs not counted in other stats + tiny uintptr + tinyoffset uintptr // The rest is not accessed on every malloc. @@ -49,6 +48,7 @@ type mcache struct { // When read with stats from other mcaches and with the world // stopped, the result will accurately reflect the state of the // application. + local_tinyallocs uintptr // number of tiny allocs not counted in other stats local_largealloc uintptr // bytes allocated for large objects local_nlargealloc uintptr // number of large object allocations local_nsmallalloc [_NumSizeClasses]uintptr // number of allocs for small objects @@ -151,6 +151,8 @@ func (c *mcache) donate(d *mcache) { d.local_nsmallfree[i] += c.local_nsmallfree[i] c.local_nsmallfree[i] = 0 } + d.local_tinyallocs += c.local_tinyallocs + c.local_tinyallocs = 0 } // refill acquires a new span of span class spc for c. This span will diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 5635dc6784..47f86ee38c 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1163,8 +1163,6 @@ func (h *mheap) allocSpan(npages uintptr, manual bool, spanclass spanClass, sysS } atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) c.local_scan = 0 - memstats.tinyallocs += uint64(c.local_tinyallocs) - c.local_tinyallocs = 0 // heap_scan was been updated. if gcBlackenEnabled != 0 { @@ -1358,8 +1356,6 @@ func (h *mheap) freeSpan(s *mspan) { lock(&h.lock) atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) c.local_scan = 0 - memstats.tinyallocs += uint64(c.local_tinyallocs) - c.local_tinyallocs = 0 if msanenabled { // Tell msan that this entire span is no longer in use. base := unsafe.Pointer(s.base()) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 44cf17c85b..341906fced 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -550,6 +550,7 @@ func updatememstats() { memstats.total_alloc = 0 memstats.nmalloc = 0 memstats.nfree = 0 + memstats.tinyallocs = 0 for i := 0; i < len(memstats.by_size); i++ { memstats.by_size[i].nmalloc = 0 memstats.by_size[i].nfree = 0 @@ -572,6 +573,9 @@ func updatememstats() { totalFree += uint64(c.local_largefree) memstats.nfree += uint64(c.local_nlargefree) + // Collect tiny allocation stats. + memstats.tinyallocs += uint64(c.local_tinyallocs) + // Collect per-sizeclass stats. for i := 0; i < _NumSizeClasses; i++ { // Malloc stats. @@ -644,8 +648,6 @@ func purgecachedstats(c *mcache) { // Protected by heap lock. atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) c.local_scan = 0 - memstats.tinyallocs += uint64(c.local_tinyallocs) - c.local_tinyallocs = 0 } // Atomically increases a given *system* memory stat. We are counting on this -- cgit v1.3 From d677899e903c4741920846f1af2c14c56f6e710e Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 22:36:58 +0000 Subject: runtime: flush local_scan directly and more often 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 Trust: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/mcache.go | 22 ++++++++++++++++++++-- src/runtime/mgc.go | 14 ++++++++++++-- src/runtime/mheap.go | 49 +++---------------------------------------------- src/runtime/mstats.go | 25 ------------------------- 4 files changed, 35 insertions(+), 75 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index fe603116a2..b8e388cc4f 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -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 diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index c54f893689..55554c117c 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -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). diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 47f86ee38c..40fd58b0ef 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -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) }) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 341906fced..5eeb173640 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -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. -- cgit v1.3 From dc02578ac8bb27359c7d0451ca249e47bdef2a9e Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Wed, 29 Jul 2020 19:00:37 +0000 Subject: runtime: make the span allocation purpose more explicit This change modifies mheap's span allocation API to have each caller declare a purpose, defined as a new enum called spanAllocType. The purpose behind this change is two-fold: 1. Tight control over who gets to allocate heap memory is, generally speaking, a good thing. Every codepath that allocates heap memory places additional implicit restrictions on the allocator. A notable example of a restriction is work bufs coming from heap memory: write barriers are not allowed in allocation paths because then we could have a situation where the allocator calls into the allocator. 2. Memory statistic updating is explicit. Instead of passing an opaque pointer for statistic updating, which places restrictions on how that statistic may be updated, we use the spanAllocType to determine which statistic to update and how. We also take this opportunity to group all the statistic updating code together, which should make the accounting code a little easier to follow. Change-Id: Ic0b0898959ba2a776f67122f0e36c9d7d60e3085 Reviewed-on: https://go-review.googlesource.com/c/go/+/246970 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/mbitmap.go | 4 +-- src/runtime/mgcwork.go | 4 +-- src/runtime/mheap.go | 78 +++++++++++++++++++++++++++++++++++++------------- src/runtime/stack.go | 12 ++++---- 4 files changed, 68 insertions(+), 30 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go index 51c3625c3d..fbfaae0f93 100644 --- a/src/runtime/mbitmap.go +++ b/src/runtime/mbitmap.go @@ -1868,12 +1868,12 @@ func materializeGCProg(ptrdata uintptr, prog *byte) *mspan { bitmapBytes := divRoundUp(ptrdata, 8*sys.PtrSize) // Compute the number of pages needed for bitmapBytes. pages := divRoundUp(bitmapBytes, pageSize) - s := mheap_.allocManual(pages, &memstats.gc_sys) + s := mheap_.allocManual(pages, spanAllocPtrScalarBits) runGCProg(addb(prog, 4), nil, (*byte)(unsafe.Pointer(s.startAddr)), 1) return s } func dematerializeGCProg(s *mspan) { - mheap_.freeManual(s, &memstats.gc_sys) + mheap_.freeManual(s, spanAllocPtrScalarBits) } func dumpGCProg(p *byte) { diff --git a/src/runtime/mgcwork.go b/src/runtime/mgcwork.go index 51e0fe9219..b3a068661e 100644 --- a/src/runtime/mgcwork.go +++ b/src/runtime/mgcwork.go @@ -371,7 +371,7 @@ func getempty() *workbuf { } if s == nil { systemstack(func() { - s = mheap_.allocManual(workbufAlloc/pageSize, &memstats.gc_sys) + s = mheap_.allocManual(workbufAlloc/pageSize, spanAllocWorkBuf) }) if s == nil { throw("out of memory") @@ -473,7 +473,7 @@ func freeSomeWbufs(preemptible bool) bool { break } work.wbufSpans.free.remove(span) - mheap_.freeManual(span, &memstats.gc_sys) + mheap_.freeManual(span, spanAllocWorkBuf) } }) more := !work.wbufSpans.free.isEmpty() diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 40fd58b0ef..df659e222b 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -861,6 +861,22 @@ func (h *mheap) reclaimChunk(arenas []arenaIdx, pageIdx, n uintptr) uintptr { return nFreed } +// spanAllocType represents the type of allocation to make, or +// the type of allocation to be freed. +type spanAllocType uint8 + +const ( + spanAllocHeap spanAllocType = iota // heap span + spanAllocStack // stack span + spanAllocPtrScalarBits // unrolled GC prog bitmap span + spanAllocWorkBuf // work buf span +) + +// manual returns true if the span allocation is manually managed. +func (s spanAllocType) manual() bool { + return s != spanAllocHeap +} + // alloc allocates a new span of npage pages from the GC'd heap. // // spanclass indicates the span's size class and scannability. @@ -877,7 +893,7 @@ func (h *mheap) alloc(npages uintptr, spanclass spanClass, needzero bool) *mspan if h.sweepdone == 0 { h.reclaim(npages) } - s = h.allocSpan(npages, false, spanclass, &memstats.heap_inuse) + s = h.allocSpan(npages, spanAllocHeap, spanclass) }) if s != nil { @@ -902,9 +918,15 @@ func (h *mheap) alloc(npages uintptr, spanclass spanClass, needzero bool) *mspan // allocManual must be called on the system stack because it may // acquire the heap lock via allocSpan. See mheap for details. // +// If new code is written to call allocManual, do NOT use an +// existing spanAllocType value and instead declare a new one. +// //go:systemstack -func (h *mheap) allocManual(npages uintptr, stat *uint64) *mspan { - return h.allocSpan(npages, true, 0, stat) +func (h *mheap) allocManual(npages uintptr, typ spanAllocType) *mspan { + if !typ.manual() { + throw("manual span allocation called with non-manually-managed type") + } + return h.allocSpan(npages, typ, 0) } // setSpans modifies the span map so [spanOf(base), spanOf(base+npage*pageSize)) @@ -1066,7 +1088,7 @@ func (h *mheap) freeMSpanLocked(s *mspan) { // allocSpan allocates an mspan which owns npages worth of memory. // -// If manual == false, allocSpan allocates a heap span of class spanclass +// If typ.manual() == false, allocSpan allocates a heap span of class spanclass // and updates heap accounting. If manual == true, allocSpan allocates a // manually-managed span (spanclass is ignored), and the caller is // responsible for any accounting related to its use of the span. Either @@ -1081,7 +1103,7 @@ func (h *mheap) freeMSpanLocked(s *mspan) { // the heap lock and because it must block GC transitions. // //go:systemstack -func (h *mheap) allocSpan(npages uintptr, manual bool, spanclass spanClass, sysStat *uint64) (s *mspan) { +func (h *mheap) allocSpan(npages uintptr, typ spanAllocType, spanclass spanClass) (s *mspan) { // Function-global state. gp := getg() base, scav := uintptr(0), uintptr(0) @@ -1143,12 +1165,10 @@ HaveSpan: s.needzero = 1 } nbytes := npages * pageSize - if manual { + if typ.manual() { s.manualFreeList = 0 s.nelems = 0 s.limit = s.base() + s.npages*pageSize - // Manually managed memory doesn't count toward heap_sys. - mSysStatDec(&memstats.heap_sys, s.npages*pageSize) s.state.set(mSpanManual) } else { // We must set span properties before the span is published anywhere @@ -1205,7 +1225,18 @@ HaveSpan: mSysStatDec(&memstats.heap_released, scav) } // Update stats. - mSysStatInc(sysStat, nbytes) + switch typ { + case spanAllocHeap: + mSysStatInc(&memstats.heap_inuse, nbytes) + case spanAllocStack: + mSysStatInc(&memstats.stacks_inuse, nbytes) + case spanAllocPtrScalarBits, spanAllocWorkBuf: + mSysStatInc(&memstats.gc_sys, nbytes) + } + if typ.manual() { + // Manually managed memory doesn't count toward heap_sys. + mSysStatDec(&memstats.heap_sys, nbytes) + } mSysStatDec(&memstats.heap_idle, nbytes) // Publish the span in various locations. @@ -1217,7 +1248,7 @@ HaveSpan: // before that happens) or pageInUse is updated. h.setSpans(s.base(), npages, s) - if !manual { + if !typ.manual() { // Mark in-use span in arena page bitmap. // // This publishes the span to the page sweeper, so @@ -1323,13 +1354,13 @@ func (h *mheap) freeSpan(s *mspan) { bytes := s.npages << _PageShift msanfree(base, bytes) } - h.freeSpanLocked(s, true, true) + h.freeSpanLocked(s, spanAllocHeap) unlock(&h.lock) }) } // freeManual frees a manually-managed span returned by allocManual. -// stat must be the same as the stat passed to the allocManual that +// typ must be the same as the spanAllocType passed to the allocManual that // allocated s. // // This must only be called when gcphase == _GCoff. See mSpanState for @@ -1339,16 +1370,14 @@ func (h *mheap) freeSpan(s *mspan) { // the heap lock. See mheap for details. // //go:systemstack -func (h *mheap) freeManual(s *mspan, stat *uint64) { +func (h *mheap) freeManual(s *mspan, typ spanAllocType) { s.needzero = 1 lock(&h.lock) - mSysStatDec(stat, s.npages*pageSize) - mSysStatInc(&memstats.heap_sys, s.npages*pageSize) - h.freeSpanLocked(s, false, true) + h.freeSpanLocked(s, typ) unlock(&h.lock) } -func (h *mheap) freeSpanLocked(s *mspan, acctinuse, acctidle bool) { +func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { switch s.state.get() { case mSpanManual: if s.allocCount != 0 { @@ -1368,12 +1397,21 @@ func (h *mheap) freeSpanLocked(s *mspan, acctinuse, acctidle bool) { throw("mheap.freeSpanLocked - invalid span state") } - if acctinuse { + // Update stats. + // + // Mirrors the code in allocSpan. + switch typ { + case spanAllocHeap: mSysStatDec(&memstats.heap_inuse, s.npages*pageSize) + case spanAllocStack: + mSysStatDec(&memstats.stacks_inuse, s.npages*pageSize) + case spanAllocPtrScalarBits, spanAllocWorkBuf: + mSysStatDec(&memstats.gc_sys, s.npages*pageSize) } - if acctidle { - mSysStatInc(&memstats.heap_idle, s.npages*pageSize) + if typ.manual() { + mSysStatInc(&memstats.heap_sys, s.npages*pageSize) } + mSysStatInc(&memstats.heap_idle, s.npages*pageSize) // Mark the space as free. h.pages.free(s.base(), s.npages) diff --git a/src/runtime/stack.go b/src/runtime/stack.go index 2afc2635aa..7b9dce5393 100644 --- a/src/runtime/stack.go +++ b/src/runtime/stack.go @@ -187,7 +187,7 @@ func stackpoolalloc(order uint8) gclinkptr { lockWithRankMayAcquire(&mheap_.lock, lockRankMheap) if s == nil { // no free stacks. Allocate another span worth. - s = mheap_.allocManual(_StackCacheSize>>_PageShift, &memstats.stacks_inuse) + s = mheap_.allocManual(_StackCacheSize>>_PageShift, spanAllocStack) if s == nil { throw("out of memory") } @@ -251,7 +251,7 @@ func stackpoolfree(x gclinkptr, order uint8) { stackpool[order].item.span.remove(s) s.manualFreeList = 0 osStackFree(s) - mheap_.freeManual(s, &memstats.stacks_inuse) + mheap_.freeManual(s, spanAllocStack) } } @@ -396,7 +396,7 @@ func stackalloc(n uint32) stack { if s == nil { // Allocate a new stack from the heap. - s = mheap_.allocManual(npage, &memstats.stacks_inuse) + s = mheap_.allocManual(npage, spanAllocStack) if s == nil { throw("out of memory") } @@ -480,7 +480,7 @@ func stackfree(stk stack) { // Free the stack immediately if we're // sweeping. osStackFree(s) - mheap_.freeManual(s, &memstats.stacks_inuse) + mheap_.freeManual(s, spanAllocStack) } else { // If the GC is running, we can't return a // stack span to the heap because it could be @@ -1193,7 +1193,7 @@ func freeStackSpans() { list.remove(s) s.manualFreeList = 0 osStackFree(s) - mheap_.freeManual(s, &memstats.stacks_inuse) + mheap_.freeManual(s, spanAllocStack) } s = next } @@ -1207,7 +1207,7 @@ func freeStackSpans() { next := s.next stackLarge.free[i].remove(s) osStackFree(s) - mheap_.freeManual(s, &memstats.stacks_inuse) + mheap_.freeManual(s, spanAllocStack) s = next } } -- cgit v1.3 From 8ebc58452af3a586a3da1f68725bc83c78d4b073 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Wed, 29 Jul 2020 20:25:05 +0000 Subject: runtime: delineate which memstats are system stats with a type This change modifies the type of several mstats fields to be a new type: sysMemStat. This type has the same structure as the fields used to have. The purpose of this change is to make it very clear which stats may be used in various functions for accounting (usually the platform-specific sys* functions, but there are others). Currently there's an implicit understanding that the *uint64 value passed to these functions is some kind of statistic whose value is atomically managed. This understanding isn't inherently problematic, but we're about to change how some stats (which currently use mSysStatInc and mSysStatDec) work, so we want to make it very clear what the various requirements are around "sysStat". This change also removes mSysStatInc and mSysStatDec in favor of a method on sysMemStat. Note that those two functions were originally written the way they were because atomic 64-bit adds required a valid G on ARM, but this hasn't been the case for a very long time (since golang.org/cl/14204, but even before then it wasn't clear if mutexes required a valid G anymore). Today we implement 64-bit adds on ARM with a spinlock table. Change-Id: I4e9b37cf14afc2ae20cf736e874eb0064af086d7 Reviewed-on: https://go-review.googlesource.com/c/go/+/246971 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/export_test.go | 4 +-- src/runtime/heapdump.go | 14 ++++---- src/runtime/malloc.go | 10 +++--- src/runtime/mem_aix.go | 12 +++---- src/runtime/mem_bsd.go | 12 +++---- src/runtime/mem_darwin.go | 12 +++---- src/runtime/mem_js.go | 10 +++--- src/runtime/mem_linux.go | 12 +++---- src/runtime/mem_plan9.go | 12 +++---- src/runtime/mem_windows.go | 12 +++---- src/runtime/mfixalloc.go | 4 +-- src/runtime/mgcscavenge.go | 4 +-- src/runtime/mheap.go | 28 ++++++++-------- src/runtime/mpagealloc.go | 4 +-- src/runtime/mranges.go | 4 +-- src/runtime/mstats.go | 82 +++++++++++++++++----------------------------- src/runtime/os_darwin.go | 3 +- 17 files changed, 109 insertions(+), 130 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index 47cbc286f6..cb753ee819 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -820,7 +820,7 @@ type AddrRanges struct { // Add. func NewAddrRanges() AddrRanges { r := addrRanges{} - r.init(new(uint64)) + r.init(new(sysMemStat)) return AddrRanges{r, true} } @@ -844,7 +844,7 @@ func MakeAddrRanges(a ...AddrRange) AddrRanges { return AddrRanges{addrRanges{ ranges: ranges, totalBytes: total, - sysStat: new(uint64), + sysStat: new(sysMemStat), }, false} } diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index 4c35309211..495ecc5164 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -548,20 +548,20 @@ func dumpmemstats() { dumpint(memstats.nmalloc) dumpint(memstats.nfree) dumpint(memstats.heap_alloc) - dumpint(memstats.heap_sys) + dumpint(memstats.heap_sys.load()) dumpint(memstats.heap_idle) dumpint(memstats.heap_inuse) dumpint(memstats.heap_released) dumpint(memstats.heap_objects) dumpint(memstats.stacks_inuse) - dumpint(memstats.stacks_sys) + dumpint(memstats.stacks_sys.load()) dumpint(memstats.mspan_inuse) - dumpint(memstats.mspan_sys) + dumpint(memstats.mspan_sys.load()) dumpint(memstats.mcache_inuse) - dumpint(memstats.mcache_sys) - dumpint(memstats.buckhash_sys) - dumpint(memstats.gc_sys) - dumpint(memstats.other_sys) + dumpint(memstats.mcache_sys.load()) + dumpint(memstats.buckhash_sys.load()) + dumpint(memstats.gc_sys.load()) + dumpint(memstats.other_sys.load()) dumpint(memstats.next_gc) dumpint(memstats.last_gc_unix) dumpint(memstats.pause_total_ns) diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 0f48d7f68e..27d678d917 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -1313,7 +1313,7 @@ var persistentChunks *notInHeap // The returned memory will be zeroed. // // Consider marking persistentalloc'd types go:notinheap. -func persistentalloc(size, align uintptr, sysStat *uint64) unsafe.Pointer { +func persistentalloc(size, align uintptr, sysStat *sysMemStat) unsafe.Pointer { var p *notInHeap systemstack(func() { p = persistentalloc1(size, align, sysStat) @@ -1324,7 +1324,7 @@ func persistentalloc(size, align uintptr, sysStat *uint64) unsafe.Pointer { // Must run on system stack because stack growth can (re)invoke it. // See issue 9174. //go:systemstack -func persistentalloc1(size, align uintptr, sysStat *uint64) *notInHeap { +func persistentalloc1(size, align uintptr, sysStat *sysMemStat) *notInHeap { const ( maxBlock = 64 << 10 // VM reservation granularity is 64K on windows ) @@ -1383,8 +1383,8 @@ func persistentalloc1(size, align uintptr, sysStat *uint64) *notInHeap { } if sysStat != &memstats.other_sys { - mSysStatInc(sysStat, size) - mSysStatDec(&memstats.other_sys, size) + sysStat.add(int64(size)) + memstats.other_sys.add(-int64(size)) } return p } @@ -1425,7 +1425,7 @@ func (l *linearAlloc) init(base, size uintptr) { l.end = base + size } -func (l *linearAlloc) alloc(size, align uintptr, sysStat *uint64) unsafe.Pointer { +func (l *linearAlloc) alloc(size, align uintptr, sysStat *sysMemStat) unsafe.Pointer { p := alignUp(l.next, align) if p+size > l.end { return nil diff --git a/src/runtime/mem_aix.go b/src/runtime/mem_aix.go index 7e145b072a..957aa4dcc2 100644 --- a/src/runtime/mem_aix.go +++ b/src/runtime/mem_aix.go @@ -11,7 +11,7 @@ import ( // Don't split the stack as this method may be invoked without a valid G, which // prevents us from allocating more stack. //go:nosplit -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { p, err := mmap(nil, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_PRIVATE, -1, 0) if err != 0 { if err == _EACCES { @@ -24,7 +24,7 @@ func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { } return nil } - mSysStatInc(sysStat, n) + sysStat.add(int64(n)) return p } @@ -41,8 +41,8 @@ func sysHugePage(v unsafe.Pointer, n uintptr) { // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) munmap(v, n) } @@ -59,8 +59,8 @@ func sysReserve(v unsafe.Pointer, n uintptr) unsafe.Pointer { return p } -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatInc(sysStat, n) +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(int64(n)) // AIX does not allow mapping a range that is already mapped. // So, call mprotect to change permissions. diff --git a/src/runtime/mem_bsd.go b/src/runtime/mem_bsd.go index 4d860e7bd3..bc672019fb 100644 --- a/src/runtime/mem_bsd.go +++ b/src/runtime/mem_bsd.go @@ -13,12 +13,12 @@ import ( // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { v, err := mmap(nil, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_PRIVATE, -1, 0) if err != 0 { return nil } - mSysStatInc(sysStat, n) + sysStat.add(int64(n)) return v } @@ -35,8 +35,8 @@ func sysHugePage(v unsafe.Pointer, n uintptr) { // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) munmap(v, n) } @@ -65,8 +65,8 @@ func sysReserve(v unsafe.Pointer, n uintptr) unsafe.Pointer { const _sunosEAGAIN = 11 const _ENOMEM = 12 -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatInc(sysStat, n) +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(int64(n)) p, err := mmap(v, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_FIXED|_MAP_PRIVATE, -1, 0) if err == _ENOMEM || ((GOOS == "solaris" || GOOS == "illumos") && err == _sunosEAGAIN) { diff --git a/src/runtime/mem_darwin.go b/src/runtime/mem_darwin.go index 3b5d565b0f..7fccd2bb8e 100644 --- a/src/runtime/mem_darwin.go +++ b/src/runtime/mem_darwin.go @@ -11,12 +11,12 @@ import ( // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { v, err := mmap(nil, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_PRIVATE, -1, 0) if err != 0 { return nil } - mSysStatInc(sysStat, n) + sysStat.add(int64(n)) return v } @@ -39,8 +39,8 @@ func sysHugePage(v unsafe.Pointer, n uintptr) { // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) munmap(v, n) } @@ -58,8 +58,8 @@ func sysReserve(v unsafe.Pointer, n uintptr) unsafe.Pointer { const _ENOMEM = 12 -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatInc(sysStat, n) +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(int64(n)) p, err := mmap(v, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_FIXED|_MAP_PRIVATE, -1, 0) if err == _ENOMEM { diff --git a/src/runtime/mem_js.go b/src/runtime/mem_js.go index 092b3d4fa2..957ed36ffa 100644 --- a/src/runtime/mem_js.go +++ b/src/runtime/mem_js.go @@ -13,7 +13,7 @@ import ( // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { p := sysReserve(nil, n) sysMap(p, n, sysStat) return p @@ -31,8 +31,8 @@ func sysHugePage(v unsafe.Pointer, n uintptr) { // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) } func sysFault(v unsafe.Pointer, n uintptr) { @@ -80,6 +80,6 @@ func growMemory(pages int32) int32 // This allows the front-end to replace the old DataView object with a new one. func resetMemoryDataView() -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatInc(sysStat, n) +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(int64(n)) } diff --git a/src/runtime/mem_linux.go b/src/runtime/mem_linux.go index 59b0bca970..3436851091 100644 --- a/src/runtime/mem_linux.go +++ b/src/runtime/mem_linux.go @@ -17,7 +17,7 @@ const ( // Don't split the stack as this method may be invoked without a valid G, which // prevents us from allocating more stack. //go:nosplit -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { p, err := mmap(nil, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_PRIVATE, -1, 0) if err != 0 { if err == _EACCES { @@ -30,7 +30,7 @@ func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { } return nil } - mSysStatInc(sysStat, n) + sysStat.add(int64(n)) return p } @@ -144,8 +144,8 @@ func sysHugePage(v unsafe.Pointer, n uintptr) { // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) munmap(v, n) } @@ -161,8 +161,8 @@ func sysReserve(v unsafe.Pointer, n uintptr) unsafe.Pointer { return p } -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatInc(sysStat, n) +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(int64(n)) p, err := mmap(v, n, _PROT_READ|_PROT_WRITE, _MAP_ANON|_MAP_FIXED|_MAP_PRIVATE, -1, 0) if err == _ENOMEM { diff --git a/src/runtime/mem_plan9.go b/src/runtime/mem_plan9.go index 4fea851cdd..53d8e6dffa 100644 --- a/src/runtime/mem_plan9.go +++ b/src/runtime/mem_plan9.go @@ -140,19 +140,19 @@ func sbrk(n uintptr) unsafe.Pointer { return unsafe.Pointer(bl) } -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { lock(&memlock) p := memAlloc(n) memCheck() unlock(&memlock) if p != nil { - mSysStatInc(sysStat, n) + sysStat.add(int64(n)) } return p } -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) lock(&memlock) if uintptr(v)+n == bloc { // Address range being freed is at the end of memory, @@ -176,10 +176,10 @@ func sysUsed(v unsafe.Pointer, n uintptr) { func sysHugePage(v unsafe.Pointer, n uintptr) { } -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { // sysReserve has already allocated all heap memory, // but has not adjusted stats. - mSysStatInc(sysStat, n) + sysStat.add(int64(n)) } func sysFault(v unsafe.Pointer, n uintptr) { diff --git a/src/runtime/mem_windows.go b/src/runtime/mem_windows.go index 165062ec27..3a805b9767 100644 --- a/src/runtime/mem_windows.go +++ b/src/runtime/mem_windows.go @@ -24,8 +24,8 @@ const ( // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer { - mSysStatInc(sysStat, n) +func sysAlloc(n uintptr, sysStat *sysMemStat) unsafe.Pointer { + sysStat.add(int64(n)) return unsafe.Pointer(stdcall4(_VirtualAlloc, 0, n, _MEM_COMMIT|_MEM_RESERVE, _PAGE_READWRITE)) } @@ -97,8 +97,8 @@ func sysHugePage(v unsafe.Pointer, n uintptr) { // Don't split the stack as this function may be invoked without a valid G, // which prevents us from allocating more stack. //go:nosplit -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatDec(sysStat, n) +func sysFree(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(-int64(n)) r := stdcall3(_VirtualFree, uintptr(v), 0, _MEM_RELEASE) if r == 0 { print("runtime: VirtualFree of ", n, " bytes failed with errno=", getlasterror(), "\n") @@ -124,6 +124,6 @@ func sysReserve(v unsafe.Pointer, n uintptr) unsafe.Pointer { return unsafe.Pointer(stdcall4(_VirtualAlloc, 0, n, _MEM_RESERVE, _PAGE_READWRITE)) } -func sysMap(v unsafe.Pointer, n uintptr, sysStat *uint64) { - mSysStatInc(sysStat, n) +func sysMap(v unsafe.Pointer, n uintptr, sysStat *sysMemStat) { + sysStat.add(int64(n)) } diff --git a/src/runtime/mfixalloc.go b/src/runtime/mfixalloc.go index f9dd6ca474..293c16b38b 100644 --- a/src/runtime/mfixalloc.go +++ b/src/runtime/mfixalloc.go @@ -32,7 +32,7 @@ type fixalloc struct { chunk uintptr // use uintptr instead of unsafe.Pointer to avoid write barriers nchunk uint32 inuse uintptr // in-use bytes now - stat *uint64 + stat *sysMemStat zero bool // zero allocations } @@ -49,7 +49,7 @@ type mlink struct { // Initialize f to allocate objects of the given size, // using the allocator to obtain chunks of memory. -func (f *fixalloc) init(size uintptr, first func(arg, p unsafe.Pointer), arg unsafe.Pointer, stat *uint64) { +func (f *fixalloc) init(size uintptr, first func(arg, p unsafe.Pointer), arg unsafe.Pointer, stat *sysMemStat) { f.size = size f.first = first f.arg = arg diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 6328b295ca..8b1a0be353 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -100,7 +100,7 @@ const ( // heapRetained returns an estimate of the current heap RSS. func heapRetained() uint64 { - return atomic.Load64(&memstats.heap_sys) - atomic.Load64(&memstats.heap_released) + return memstats.heap_sys.load() - atomic.Load64(&memstats.heap_released) } // gcPaceScavenger updates the scavenger's pacing, particularly @@ -711,7 +711,7 @@ func (p *pageAlloc) scavengeRangeLocked(ci chunkIdx, base, npages uint) uintptr // Update global accounting only when not in test, otherwise // the runtime's accounting will be wrong. - mSysStatInc(&memstats.heap_released, uintptr(npages)*pageSize) + atomic.Xadd64(&memstats.heap_released, int64(npages)*pageSize) return addr } diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index df659e222b..27c1bfbcf1 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1222,22 +1222,22 @@ HaveSpan: // sysUsed all the pages that are actually available // in the span since some of them might be scavenged. sysUsed(unsafe.Pointer(base), nbytes) - mSysStatDec(&memstats.heap_released, scav) + atomic.Xadd64(&memstats.heap_released, -int64(scav)) } // Update stats. switch typ { case spanAllocHeap: - mSysStatInc(&memstats.heap_inuse, nbytes) + atomic.Xadd64(&memstats.heap_inuse, int64(nbytes)) case spanAllocStack: - mSysStatInc(&memstats.stacks_inuse, nbytes) + atomic.Xadd64(&memstats.stacks_inuse, int64(nbytes)) case spanAllocPtrScalarBits, spanAllocWorkBuf: - mSysStatInc(&memstats.gc_sys, nbytes) + memstats.gc_sys.add(int64(nbytes)) } if typ.manual() { // Manually managed memory doesn't count toward heap_sys. - mSysStatDec(&memstats.heap_sys, nbytes) + memstats.heap_sys.add(-int64(nbytes)) } - mSysStatDec(&memstats.heap_idle, nbytes) + atomic.Xadd64(&memstats.heap_idle, -int64(nbytes)) // Publish the span in various locations. @@ -1314,8 +1314,8 @@ func (h *mheap) grow(npage uintptr) bool { // The allocation is always aligned to the heap arena // size which is always > physPageSize, so its safe to // just add directly to heap_released. - mSysStatInc(&memstats.heap_released, asize) - mSysStatInc(&memstats.heap_idle, asize) + atomic.Xadd64(&memstats.heap_released, int64(asize)) + atomic.Xadd64(&memstats.heap_idle, int64(asize)) // Recalculate nBase. // We know this won't overflow, because sysAlloc returned @@ -1400,18 +1400,20 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { // Update stats. // // Mirrors the code in allocSpan. + nbytes := s.npages * pageSize switch typ { case spanAllocHeap: - mSysStatDec(&memstats.heap_inuse, s.npages*pageSize) + atomic.Xadd64(&memstats.heap_inuse, -int64(nbytes)) case spanAllocStack: - mSysStatDec(&memstats.stacks_inuse, s.npages*pageSize) + atomic.Xadd64(&memstats.stacks_inuse, -int64(nbytes)) case spanAllocPtrScalarBits, spanAllocWorkBuf: - mSysStatDec(&memstats.gc_sys, s.npages*pageSize) + memstats.gc_sys.add(-int64(nbytes)) } if typ.manual() { - mSysStatInc(&memstats.heap_sys, s.npages*pageSize) + // Manually managed memory doesn't count toward heap_sys, so add it back. + memstats.heap_sys.add(int64(nbytes)) } - mSysStatInc(&memstats.heap_idle, s.npages*pageSize) + atomic.Xadd64(&memstats.heap_idle, int64(nbytes)) // Mark the space as free. h.pages.free(s.base(), s.npages) diff --git a/src/runtime/mpagealloc.go b/src/runtime/mpagealloc.go index 560babed03..2af1c97e0b 100644 --- a/src/runtime/mpagealloc.go +++ b/src/runtime/mpagealloc.go @@ -293,13 +293,13 @@ type pageAlloc struct { // sysStat is the runtime memstat to update when new system // memory is committed by the pageAlloc for allocation metadata. - sysStat *uint64 + sysStat *sysMemStat // Whether or not this struct is being used in tests. test bool } -func (p *pageAlloc) init(mheapLock *mutex, sysStat *uint64) { +func (p *pageAlloc) init(mheapLock *mutex, sysStat *sysMemStat) { if levelLogPages[0] > logMaxPackedValue { // We can't represent 1< 0 && int64(val) < n) || (n < 0 && int64(val)+n < n) { + print("runtime: val=", val, " n=", n, "\n") + throw("sysMemStat overflow") } } diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go index 394bd6fb0f..3f5bb7cf96 100644 --- a/src/runtime/os_darwin.go +++ b/src/runtime/os_darwin.go @@ -198,7 +198,6 @@ func newosproc(mp *m) { exit(1) } mp.g0.stack.hi = stacksize // for mstart - //mSysStatInc(&memstats.stacks_sys, stacksize) //TODO: do this? // Tell the pthread library we won't join with this thread. if pthread_attr_setdetachstate(&attr, _PTHREAD_CREATE_DETACHED) != 0 { @@ -247,7 +246,7 @@ func newosproc0(stacksize uintptr, fn uintptr) { exit(1) } g0.stack.hi = stacksize // for mstart - mSysStatInc(&memstats.stacks_sys, stacksize) + memstats.stacks_sys.add(int64(stacksize)) // Tell the pthread library we won't join with this thread. if pthread_attr_setdetachstate(&attr, _PTHREAD_CREATE_DETACHED) != 0 { -- cgit v1.3 From ad863ba32a2ede207d708fa15897e9de1d14dd87 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 3 Aug 2020 19:23:30 +0000 Subject: runtime: break down memstats.gc_sys This change breaks apart gc_sys into three distinct pieces. Two of those pieces are pieces which come from heap_sys since they're allocated from the page heap. The rest comes from memory mapped from e.g. persistentalloc which better fits the purpose of a sysMemStat. Also, rename gc_sys to gcMiscSys. Change-Id: I098789170052511e7b31edbcdc9a53e5c24573f7 Reviewed-on: https://go-review.googlesource.com/c/go/+/246973 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/heapdump.go | 5 ++++- src/runtime/malloc.go | 6 +++--- src/runtime/mcheckmark.go | 2 +- src/runtime/mfinal.go | 2 +- src/runtime/mheap.go | 16 ++++++++++------ src/runtime/mspanset.go | 4 ++-- src/runtime/mstats.go | 31 ++++++++++++++++++------------- 7 files changed, 39 insertions(+), 27 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index 495ecc5164..eed47930f0 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -540,6 +540,9 @@ func dumpms() { } func dumpmemstats() { + // These ints should be identical to the exported + // MemStats structure and should be ordered the same + // way too. dumpint(tagMemStats) dumpint(memstats.alloc) dumpint(memstats.total_alloc) @@ -560,7 +563,7 @@ func dumpmemstats() { dumpint(memstats.mcache_inuse) dumpint(memstats.mcache_sys.load()) dumpint(memstats.buckhash_sys.load()) - dumpint(memstats.gc_sys.load()) + dumpint(memstats.gcMiscSys.load() + memstats.gcWorkBufInUse + memstats.gcProgPtrScalarBitsInUse) dumpint(memstats.other_sys.load()) dumpint(memstats.next_gc) dumpint(memstats.last_gc_unix) diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 27d678d917..ee22bad58c 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -743,9 +743,9 @@ mapped: throw("arena already initialized") } var r *heapArena - r = (*heapArena)(h.heapArenaAlloc.alloc(unsafe.Sizeof(*r), sys.PtrSize, &memstats.gc_sys)) + r = (*heapArena)(h.heapArenaAlloc.alloc(unsafe.Sizeof(*r), sys.PtrSize, &memstats.gcMiscSys)) if r == nil { - r = (*heapArena)(persistentalloc(unsafe.Sizeof(*r), sys.PtrSize, &memstats.gc_sys)) + r = (*heapArena)(persistentalloc(unsafe.Sizeof(*r), sys.PtrSize, &memstats.gcMiscSys)) if r == nil { throw("out of memory allocating heap arena metadata") } @@ -757,7 +757,7 @@ mapped: if size == 0 { size = physPageSize } - newArray := (*notInHeap)(persistentalloc(size, sys.PtrSize, &memstats.gc_sys)) + newArray := (*notInHeap)(persistentalloc(size, sys.PtrSize, &memstats.gcMiscSys)) if newArray == nil { throw("out of memory allocating allArenas") } diff --git a/src/runtime/mcheckmark.go b/src/runtime/mcheckmark.go index 1fd8e4e78f..c0b028d715 100644 --- a/src/runtime/mcheckmark.go +++ b/src/runtime/mcheckmark.go @@ -41,7 +41,7 @@ func startCheckmarks() { if bitmap == nil { // Allocate bitmap on first use. - bitmap = (*checkmarksMap)(persistentalloc(unsafe.Sizeof(*bitmap), 0, &memstats.gc_sys)) + bitmap = (*checkmarksMap)(persistentalloc(unsafe.Sizeof(*bitmap), 0, &memstats.gcMiscSys)) if bitmap == nil { throw("out of memory allocating checkmarks bitmap") } diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go index 6676ae6736..6ec5133be0 100644 --- a/src/runtime/mfinal.go +++ b/src/runtime/mfinal.go @@ -88,7 +88,7 @@ func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot lock(&finlock) if finq == nil || finq.cnt == uint32(len(finq.fin)) { if finc == nil { - finc = (*finblock)(persistentalloc(_FinBlockSize, 0, &memstats.gc_sys)) + finc = (*finblock)(persistentalloc(_FinBlockSize, 0, &memstats.gcMiscSys)) finc.alllink = allfin allfin = finc if finptrmask[0] == 0 { diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 27c1bfbcf1..1624a04b9d 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -713,7 +713,7 @@ func (h *mheap) init() { h.central[i].mcentral.init(spanClass(i)) } - h.pages.init(&h.lock, &memstats.gc_sys) + h.pages.init(&h.lock, &memstats.gcMiscSys) } // reclaim sweeps and reclaims at least npage pages into the heap. @@ -1230,8 +1230,10 @@ HaveSpan: atomic.Xadd64(&memstats.heap_inuse, int64(nbytes)) case spanAllocStack: atomic.Xadd64(&memstats.stacks_inuse, int64(nbytes)) - case spanAllocPtrScalarBits, spanAllocWorkBuf: - memstats.gc_sys.add(int64(nbytes)) + case spanAllocWorkBuf: + atomic.Xadd64(&memstats.gcWorkBufInUse, int64(nbytes)) + case spanAllocPtrScalarBits: + atomic.Xadd64(&memstats.gcProgPtrScalarBitsInUse, int64(nbytes)) } if typ.manual() { // Manually managed memory doesn't count toward heap_sys. @@ -1406,8 +1408,10 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { atomic.Xadd64(&memstats.heap_inuse, -int64(nbytes)) case spanAllocStack: atomic.Xadd64(&memstats.stacks_inuse, -int64(nbytes)) - case spanAllocPtrScalarBits, spanAllocWorkBuf: - memstats.gc_sys.add(-int64(nbytes)) + case spanAllocWorkBuf: + atomic.Xadd64(&memstats.gcWorkBufInUse, -int64(nbytes)) + case spanAllocPtrScalarBits: + atomic.Xadd64(&memstats.gcProgPtrScalarBitsInUse, -int64(nbytes)) } if typ.manual() { // Manually managed memory doesn't count toward heap_sys, so add it back. @@ -1956,7 +1960,7 @@ func newArenaMayUnlock() *gcBitsArena { var result *gcBitsArena if gcBitsArenas.free == nil { unlock(&gcBitsArenas.lock) - result = (*gcBitsArena)(sysAlloc(gcBitsChunkBytes, &memstats.gc_sys)) + result = (*gcBitsArena)(sysAlloc(gcBitsChunkBytes, &memstats.gcMiscSys)) if result == nil { throw("runtime: cannot allocate memory") } diff --git a/src/runtime/mspanset.go b/src/runtime/mspanset.go index 490eed4549..10d2596c38 100644 --- a/src/runtime/mspanset.go +++ b/src/runtime/mspanset.go @@ -102,7 +102,7 @@ retry: if newCap == 0 { newCap = spanSetInitSpineCap } - newSpine := persistentalloc(newCap*sys.PtrSize, cpu.CacheLineSize, &memstats.gc_sys) + newSpine := persistentalloc(newCap*sys.PtrSize, cpu.CacheLineSize, &memstats.gcMiscSys) if b.spineCap != 0 { // Blocks are allocated off-heap, so // no write barriers. @@ -283,7 +283,7 @@ func (p *spanSetBlockAlloc) alloc() *spanSetBlock { if s := (*spanSetBlock)(p.stack.pop()); s != nil { return s } - return (*spanSetBlock)(persistentalloc(unsafe.Sizeof(spanSetBlock{}), cpu.CacheLineSize, &memstats.gc_sys)) + return (*spanSetBlock)(persistentalloc(unsafe.Sizeof(spanSetBlock{}), cpu.CacheLineSize, &memstats.gcMiscSys)) } // free returns a spanSetBlock back to the pool. diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 466f33836c..967fe6e2be 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -44,15 +44,17 @@ type mstats struct { // Statistics about allocation of low-level fixed-size structures. // Protected by FixAlloc locks. - stacks_inuse uint64 // bytes in manually-managed stack spans; updated atomically or during STW - stacks_sys sysMemStat // only counts newosproc0 stack in mstats; differs from MemStats.StackSys - mspan_inuse uint64 // mspan structures - mspan_sys sysMemStat - mcache_inuse uint64 // mcache structures - mcache_sys sysMemStat - buckhash_sys sysMemStat // profiling bucket hash table - gc_sys sysMemStat // updated atomically or during STW - other_sys sysMemStat // updated atomically or during STW + stacks_inuse uint64 // bytes in manually-managed stack spans; updated atomically or during STW + stacks_sys sysMemStat // only counts newosproc0 stack in mstats; differs from MemStats.StackSys + mspan_inuse uint64 // mspan structures + mspan_sys sysMemStat + mcache_inuse uint64 // mcache structures + mcache_sys sysMemStat + buckhash_sys sysMemStat // profiling bucket hash table + gcWorkBufInUse uint64 // updated atomically or during STW + gcProgPtrScalarBitsInUse uint64 // updated atomically or during STW + gcMiscSys sysMemStat // updated atomically or during STW + other_sys sysMemStat // updated atomically or during STW // Statistics about the garbage collector. @@ -472,7 +474,10 @@ func readmemstats_m(stats *MemStats) { stats.MCacheInuse = memstats.mcache_inuse stats.MCacheSys = memstats.mcache_sys.load() stats.BuckHashSys = memstats.buckhash_sys.load() - stats.GCSys = memstats.gc_sys.load() + // MemStats defines GCSys as an aggregate of all memory related + // to the memory management system, but we track this memory + // at a more granular level in the runtime. + stats.GCSys = memstats.gcMiscSys.load() + memstats.gcWorkBufInUse + memstats.gcProgPtrScalarBitsInUse stats.OtherSys = memstats.other_sys.load() stats.NextGC = memstats.next_gc stats.LastGC = memstats.last_gc_unix @@ -557,11 +562,11 @@ func updatememstats() { memstats.mcache_inuse = uint64(mheap_.cachealloc.inuse) memstats.mspan_inuse = uint64(mheap_.spanalloc.inuse) memstats.sys = memstats.heap_sys.load() + memstats.stacks_sys.load() + memstats.mspan_sys.load() + - memstats.mcache_sys.load() + memstats.buckhash_sys.load() + memstats.gc_sys.load() + + memstats.mcache_sys.load() + memstats.buckhash_sys.load() + memstats.gcMiscSys.load() + memstats.other_sys.load() - // We also count stacks_inuse as sys memory. - memstats.sys += memstats.stacks_inuse + // We also count stacks_inuse, gcWorkBufInUse, and gcProgPtrScalarBitsInUse as sys memory. + memstats.sys += memstats.stacks_inuse + memstats.gcWorkBufInUse + memstats.gcProgPtrScalarBitsInUse // Calculate memory allocator stats. // During program execution we only count number of frees and amount of freed memory. -- cgit v1.3 From c5dea8f38726572ddc161e5d169a453639edb7b1 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 3 Aug 2020 19:27:59 +0000 Subject: runtime: remove memstats.heap_idle This statistic is updated in many places but for MemStats may be computed from existing statistics. Specifically by definition heap_idle = heap_sys - heap_inuse since heap_sys is all memory allocated from the OS for use in the heap minus memory used for non-heap purposes. heap_idle is almost the same (since it explicitly includes memory that *could* be used for non-heap purposes) but also doesn't include memory that's actually used to hold heap objects. Although it has some utility as a sanity check, it complicates accounting and we want fewer, orthogonal statistics for upcoming metrics changes, so just drop it. Change-Id: I40af54a38e335f43249f6e218f35088bfd4380d1 Reviewed-on: https://go-review.googlesource.com/c/go/+/246974 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/heapdump.go | 2 +- src/runtime/mheap.go | 3 --- src/runtime/mstats.go | 19 +++++++++++++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index eed47930f0..f96475e848 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -552,7 +552,7 @@ func dumpmemstats() { dumpint(memstats.nfree) dumpint(memstats.heap_alloc) dumpint(memstats.heap_sys.load()) - dumpint(memstats.heap_idle) + dumpint(memstats.heap_sys.load() - memstats.heap_inuse) dumpint(memstats.heap_inuse) dumpint(memstats.heap_released) dumpint(memstats.heap_objects) diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 1624a04b9d..87d2fd495b 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1239,7 +1239,6 @@ HaveSpan: // Manually managed memory doesn't count toward heap_sys. memstats.heap_sys.add(-int64(nbytes)) } - atomic.Xadd64(&memstats.heap_idle, -int64(nbytes)) // Publish the span in various locations. @@ -1317,7 +1316,6 @@ func (h *mheap) grow(npage uintptr) bool { // size which is always > physPageSize, so its safe to // just add directly to heap_released. atomic.Xadd64(&memstats.heap_released, int64(asize)) - atomic.Xadd64(&memstats.heap_idle, int64(asize)) // Recalculate nBase. // We know this won't overflow, because sysAlloc returned @@ -1417,7 +1415,6 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { // Manually managed memory doesn't count toward heap_sys, so add it back. memstats.heap_sys.add(int64(nbytes)) } - atomic.Xadd64(&memstats.heap_idle, int64(nbytes)) // Mark the space as free. h.pages.free(s.base(), s.npages) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 967fe6e2be..43f74273f7 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -34,7 +34,6 @@ type mstats struct { // in manually-managed spans. heap_alloc uint64 // bytes allocated and not yet freed (same as alloc above) heap_sys sysMemStat // virtual address space obtained from system for GC'd heap - heap_idle uint64 // bytes in idle spans heap_inuse uint64 // bytes in mSpanInUse spans heap_released uint64 // bytes released to the os @@ -461,7 +460,23 @@ func readmemstats_m(stats *MemStats) { stats.Frees = memstats.nfree stats.HeapAlloc = memstats.heap_alloc stats.HeapSys = memstats.heap_sys.load() - stats.HeapIdle = memstats.heap_idle + // By definition, HeapIdle is memory that was mapped + // for the heap but is not currently used to hold heap + // objects. It also specifically is memory that can be + // used for other purposes, like stacks, but this memory + // is subtracted out of HeapSys before it makes that + // transition. Put another way: + // + // heap_sys = bytes allocated from the OS for the heap - bytes ultimately used for non-heap purposes + // heap_idle = bytes allocated from the OS for the heap - bytes ultimately used for any purpose + // + // or + // + // heap_sys = sys - stacks_inuse - gcWorkBufInUse - gcProgPtrScalarBitsInUse + // heap_idle = sys - stacks_inuse - gcWorkBufInUse - gcProgPtrScalarBitsInUse - heap_inuse + // + // => heap_idle = heap_sys - heap_inuse + stats.HeapIdle = memstats.heap_sys.load() - memstats.heap_inuse stats.HeapInuse = memstats.heap_inuse stats.HeapReleased = memstats.heap_released stats.HeapObjects = memstats.heap_objects -- cgit v1.3 From fe7ff71185cf30f9bdee3e8d8897e8b6069ad02e Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 3 Aug 2020 20:11:04 +0000 Subject: runtime: add consistent heap statistics This change adds a global set of heap statistics which are similar to existing memory statistics. The purpose of these new statistics is to be able to read them and get a consistent result without stopping the world. The goal is to eventually replace as many of the existing memstats statistics with the sharded ones as possible. The consistent memory statistics use a tailor-made synchronization mechanism to allow writers (allocators) to proceed with minimal synchronization by using a sequence counter and a global generation counter to determine which set of statistics to update. Readers increment the global generation counter to effectively grab a snapshot of the statistics, and then iterate over all Ps using the sequence counter to ensure that they may safely read the snapshotted statistics. To keep statistics fresh, the reader also has a responsibility to merge sets of statistics. These consistent statistics are computed, but otherwise unused for now. Upcoming changes will integrate them with the rest of the codebase and will begin to phase out existing statistics. Change-Id: I637a11f2439e2049d7dccb8650c5d82500733ca5 Reviewed-on: https://go-review.googlesource.com/c/go/+/247037 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/mcache.go | 4 + src/runtime/mgcscavenge.go | 11 ++- src/runtime/mheap.go | 34 +++++++++ src/runtime/mstats.go | 184 ++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 230 insertions(+), 3 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 5564e4a47d..e27a1c9ec0 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -61,6 +61,10 @@ type mcache struct { // in this mcache are stale and need to the flushed so they // can be swept. This is done in acquirep. flushGen uint32 + + // statsSeq is a counter indicating whether this P is currently + // writing any stats. Its value is even when not, odd when it is. + statsSeq uint32 } // A gclink is a node in a linked list of blocks, like mlink, diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 8b1a0be353..5843ada981 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -711,7 +711,16 @@ func (p *pageAlloc) scavengeRangeLocked(ci chunkIdx, base, npages uint) uintptr // Update global accounting only when not in test, otherwise // the runtime's accounting will be wrong. - atomic.Xadd64(&memstats.heap_released, int64(npages)*pageSize) + nbytes := int64(npages) * pageSize + atomic.Xadd64(&memstats.heap_released, nbytes) + + // Update consistent accounting too. + c := getMCache() + stats := memstats.heapStats.acquire(c) + atomic.Xaddint64(&stats.committed, -nbytes) + atomic.Xaddint64(&stats.released, nbytes) + memstats.heapStats.release(c) + return addr } diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 87d2fd495b..d17b6fa284 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1239,6 +1239,22 @@ HaveSpan: // Manually managed memory doesn't count toward heap_sys. memstats.heap_sys.add(-int64(nbytes)) } + // Update consistent stats. + c := getMCache() + stats := memstats.heapStats.acquire(c) + atomic.Xaddint64(&stats.committed, int64(scav)) + atomic.Xaddint64(&stats.released, -int64(scav)) + switch typ { + case spanAllocHeap: + atomic.Xaddint64(&stats.inHeap, int64(nbytes)) + case spanAllocStack: + atomic.Xaddint64(&stats.inStacks, int64(nbytes)) + case spanAllocPtrScalarBits: + atomic.Xaddint64(&stats.inPtrScalarBits, int64(nbytes)) + case spanAllocWorkBuf: + atomic.Xaddint64(&stats.inWorkBufs, int64(nbytes)) + } + memstats.heapStats.release(c) // Publish the span in various locations. @@ -1316,6 +1332,10 @@ func (h *mheap) grow(npage uintptr) bool { // size which is always > physPageSize, so its safe to // just add directly to heap_released. atomic.Xadd64(&memstats.heap_released, int64(asize)) + c := getMCache() + stats := memstats.heapStats.acquire(c) + atomic.Xaddint64(&stats.released, int64(asize)) + memstats.heapStats.release(c) // Recalculate nBase. // We know this won't overflow, because sysAlloc returned @@ -1415,6 +1435,20 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { // Manually managed memory doesn't count toward heap_sys, so add it back. memstats.heap_sys.add(int64(nbytes)) } + // Update consistent stats. + c := getMCache() + stats := memstats.heapStats.acquire(c) + switch typ { + case spanAllocHeap: + atomic.Xaddint64(&stats.inHeap, -int64(nbytes)) + case spanAllocStack: + atomic.Xaddint64(&stats.inStacks, -int64(nbytes)) + case spanAllocPtrScalarBits: + atomic.Xaddint64(&stats.inPtrScalarBits, -int64(nbytes)) + case spanAllocWorkBuf: + atomic.Xaddint64(&stats.inWorkBufs, -int64(nbytes)) + } + memstats.heapStats.release(c) // Mark the space as free. h.pages.free(s.base(), s.npages) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index a6e38d1c1b..76546c0f0c 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -148,6 +148,9 @@ type mstats struct { // unlike heap_live, heap_marked does not change until the // next mark termination. heap_marked uint64 + + // heapStats is a set of statistics + heapStats consistentHeapStats } var memstats mstats @@ -426,10 +429,20 @@ type MemStats struct { } func init() { - if unsafe.Offsetof(memstats.heap_live)%8 != 0 { - println(unsafe.Offsetof(memstats.heap_live)) + if offset := unsafe.Offsetof(memstats.heap_live); offset%8 != 0 { + println(offset) throw("memstats.heap_live not aligned to 8 bytes") } + if offset := unsafe.Offsetof(memstats.heapStats); offset%8 != 0 { + println(offset) + throw("memstats.heapStats not aligned to 8 bytes") + } + // Ensure the size of heapStatsDelta causes adjacent fields/slots (e.g. + // [3]heapStatsDelta) to be 8-byte aligned. + if size := unsafe.Sizeof(heapStatsDelta{}); size%8 != 0 { + println(size) + throw("heapStatsDelta not a multiple of 8 bytes in size") + } } // ReadMemStats populates m with memory allocator statistics. @@ -687,3 +700,170 @@ func (s *sysMemStat) add(n int64) { throw("sysMemStat overflow") } } + +// heapStatsDelta contains deltas of various runtime memory statistics +// that need to be updated together in order for them to be kept +// consistent with one another. +type heapStatsDelta struct { + committed int64 // byte delta of memory committed + released int64 // byte delta of released memory generated + inHeap int64 // byte delta of memory placed in the heap + inStacks int64 // byte delta of memory reserved for stacks + inWorkBufs int64 // byte delta of memory reserved for work bufs + inPtrScalarBits int64 // byte delta of memory reserved for unrolled GC prog bits +} + +// merge adds in the deltas from b into a. +func (a *heapStatsDelta) merge(b *heapStatsDelta) { + a.committed += b.committed + a.released += b.released + a.inHeap += b.inHeap + a.inStacks += b.inStacks + a.inWorkBufs += b.inWorkBufs + a.inPtrScalarBits += b.inPtrScalarBits +} + +// consistentHeapStats represents a set of various memory statistics +// whose updates must be viewed completely to get a consistent +// state of the world. +// +// To write updates to memory stats use the acquire and release +// methods. To obtain a consistent global snapshot of these statistics, +// use read. +type consistentHeapStats struct { + // stats is a ring buffer of heapStatsDelta values. + // Writers always atomically update the delta at index gen. + // + // Readers operate by rotating gen (0 -> 1 -> 2 -> 0 -> ...) + // and synchronizing with writers by observing each mcache's + // statsSeq field. If the reader observes a P (to which the + // mcache is bound) not writing, it can be sure that it will + // pick up the new gen value the next time it writes. + // The reader then takes responsibility by clearing space + // in the ring buffer for the next reader to rotate gen to + // that space (i.e. it merges in values from index (gen-2) mod 3 + // to index (gen-1) mod 3, then clears the former). + // + // Note that this means only one reader can be reading at a time. + // There is no way for readers to synchronize. + // + // This process is why we need ring buffer of size 3 instead + // of 2: one is for the writers, one contains the most recent + // data, and the last one is clear so writers can begin writing + // to it the moment gen is updated. + stats [3]heapStatsDelta + + // gen represents the current index into which writers + // are writing, and can take on the value of 0, 1, or 2. + // This value is updated atomically. + gen uint32 +} + +// acquire returns a heapStatsDelta to be updated. In effect, +// it acquires the shard for writing. release must be called +// as soon as the relevant deltas are updated. c must be +// a valid mcache not being used by any other thread. +// +// The returned heapStatsDelta must be updated atomically. +// +// Note however, that this is unsafe to call concurrently +// with other writers and there must be only one writer +// at a time. +func (m *consistentHeapStats) acquire(c *mcache) *heapStatsDelta { + seq := atomic.Xadd(&c.statsSeq, 1) + if seq%2 == 0 { + // Should have been incremented to odd. + print("runtime: seq=", seq, "\n") + throw("bad sequence number") + } + gen := atomic.Load(&m.gen) % 3 + return &m.stats[gen] +} + +// release indicates that the writer is done modifying +// the delta. The value returned by the corresponding +// acquire must no longer be accessed or modified after +// release is called. +// +// The mcache passed here must be the same as the one +// passed to acquire. +func (m *consistentHeapStats) release(c *mcache) { + seq := atomic.Xadd(&c.statsSeq, 1) + if seq%2 != 0 { + // Should have been incremented to even. + print("runtime: seq=", seq, "\n") + throw("bad sequence number") + } +} + +// unsafeRead aggregates the delta for this shard into out. +// +// Unsafe because it does so without any synchronization. The +// only safe time to call this is if the world is stopped or +// we're freezing the world or going down anyway (and we just +// want _some_ estimate). +func (m *consistentHeapStats) unsafeRead(out *heapStatsDelta) { + for i := range m.stats { + out.merge(&m.stats[i]) + } +} + +// unsafeClear clears the shard. +// +// Unsafe because the world must be stopped and values should +// be donated elsewhere before clearing. +func (m *consistentHeapStats) unsafeClear() { + for i := range m.stats { + m.stats[i] = heapStatsDelta{} + } +} + +// read takes a globally consistent snapshot of m +// and puts the aggregated value in out. Even though out is a +// heapStatsDelta, the resulting values should be complete and +// valid statistic values. +// +// Not safe to call concurrently. +func (m *consistentHeapStats) read(out *heapStatsDelta) { + // Getting preempted after this point is not safe because + // we read allp. We need to make sure a STW can't happen + // so it doesn't change out from under us. + mp := acquirem() + + // Rotate gen, effectively taking a snapshot of the state of + // these statistics at the point of the exchange by moving + // writers to the next set of deltas. + // + // This exchange is safe to do because we won't race + // with anyone else trying to update this value. + currGen := atomic.Load(&m.gen) + atomic.Xchg(&m.gen, (currGen+1)%3) + prevGen := currGen - 1 + if currGen == 0 { + prevGen = 2 + } + for _, p := range allp { + c := p.mcache + if c == nil { + continue + } + // Spin until there are no more writers. + for atomic.Load(&c.statsSeq)%2 != 0 { + } + } + + // At this point we've observed that each sequence + // number is even, so any future writers will observe + // the new gen value. That means it's safe to read from + // the other deltas in the stats buffer. + + // Perform our responsibilities and free up + // stats[prevGen] for the next time we want to take + // a snapshot. + m.stats[currGen].merge(&m.stats[prevGen]) + m.stats[prevGen] = heapStatsDelta{} + + // Finally, copy out the complete delta. + *out = m.stats[currGen] + releasem(mp) +} -- cgit v1.3 From f77a9025f1e4bf4bb3e2b582d13cce5f19c1ca51 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 3 Aug 2020 20:35:40 +0000 Subject: runtime: replace some memstats with consistent stats This change replaces stacks_inuse, gcWorkBufInUse and gcProgPtrScalarBitsInUse with their corresponding consistent stats. It also adds checks to make sure the rest of the sharded stats line up with existing stats in updatememstats. Change-Id: I17d0bd181aedb5c55e09c8dff18cef5b2a3a14e3 Reviewed-on: https://go-review.googlesource.com/c/go/+/247038 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/mheap.go | 18 ++----------- src/runtime/mstats.go | 73 ++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 62 insertions(+), 29 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index d17b6fa284..14a73c0491 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1225,15 +1225,8 @@ HaveSpan: atomic.Xadd64(&memstats.heap_released, -int64(scav)) } // Update stats. - switch typ { - case spanAllocHeap: + if typ == spanAllocHeap { atomic.Xadd64(&memstats.heap_inuse, int64(nbytes)) - case spanAllocStack: - atomic.Xadd64(&memstats.stacks_inuse, int64(nbytes)) - case spanAllocWorkBuf: - atomic.Xadd64(&memstats.gcWorkBufInUse, int64(nbytes)) - case spanAllocPtrScalarBits: - atomic.Xadd64(&memstats.gcProgPtrScalarBitsInUse, int64(nbytes)) } if typ.manual() { // Manually managed memory doesn't count toward heap_sys. @@ -1421,15 +1414,8 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { // // Mirrors the code in allocSpan. nbytes := s.npages * pageSize - switch typ { - case spanAllocHeap: + if typ == spanAllocHeap { atomic.Xadd64(&memstats.heap_inuse, -int64(nbytes)) - case spanAllocStack: - atomic.Xadd64(&memstats.stacks_inuse, -int64(nbytes)) - case spanAllocWorkBuf: - atomic.Xadd64(&memstats.gcWorkBufInUse, -int64(nbytes)) - case spanAllocPtrScalarBits: - atomic.Xadd64(&memstats.gcProgPtrScalarBitsInUse, -int64(nbytes)) } if typ.manual() { // Manually managed memory doesn't count toward heap_sys, so add it back. diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 76546c0f0c..4363eff1e0 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -40,19 +40,25 @@ type mstats struct { // computed on the fly by updatememstats. heap_objects uint64 // total number of allocated objects + // Statistics about stacks. + stacks_inuse uint64 // bytes in manually-managed stack spans; computed by updatememstats + stacks_sys sysMemStat // only counts newosproc0 stack in mstats; differs from MemStats.StackSys + // Statistics about allocation of low-level fixed-size structures. // Protected by FixAlloc locks. - stacks_inuse uint64 // bytes in manually-managed stack spans; updated atomically or during STW - stacks_sys sysMemStat // only counts newosproc0 stack in mstats; differs from MemStats.StackSys - mspan_inuse uint64 // mspan structures - mspan_sys sysMemStat - mcache_inuse uint64 // mcache structures - mcache_sys sysMemStat - buckhash_sys sysMemStat // profiling bucket hash table - gcWorkBufInUse uint64 // updated atomically or during STW - gcProgPtrScalarBitsInUse uint64 // updated atomically or during STW + mspan_inuse uint64 // mspan structures + mspan_sys sysMemStat + mcache_inuse uint64 // mcache structures + mcache_sys sysMemStat + buckhash_sys sysMemStat // profiling bucket hash table + + // Statistics about GC overhead. + gcWorkBufInUse uint64 // computed by updatememstats + gcProgPtrScalarBitsInUse uint64 // computed by updatememstats gcMiscSys sysMemStat // updated atomically or during STW - other_sys sysMemStat // updated atomically or during STW + + // Miscellaneous statistics. + other_sys sysMemStat // updated atomically or during STW // Statistics about the garbage collector. @@ -577,6 +583,10 @@ func readGCStats_m(pauses *[]uint64) { *pauses = p[:n+n+3] } +// Updates the memstats structure. +// +// The world must be stopped. +// //go:nowritebarrier func updatememstats() { // Flush mcaches to mcentral before doing anything else. @@ -591,9 +601,6 @@ func updatememstats() { memstats.mcache_sys.load() + memstats.buckhash_sys.load() + memstats.gcMiscSys.load() + memstats.other_sys.load() - // We also count stacks_inuse, gcWorkBufInUse, and gcProgPtrScalarBitsInUse as sys memory. - memstats.sys += memstats.stacks_inuse + memstats.gcWorkBufInUse + memstats.gcProgPtrScalarBitsInUse - // Calculate memory allocator stats. // During program execution we only count number of frees and amount of freed memory. // Current number of alive objects in the heap and amount of alive heap memory @@ -641,6 +648,9 @@ func updatememstats() { smallFree += uint64(c.smallFreeCount[i]) * uint64(class_to_size[i]) } } + // Collect consistent stats, which are the source-of-truth in the some cases. + var consStats heapStatsDelta + memstats.heapStats.unsafeRead(&consStats) totalFree += smallFree @@ -651,6 +661,43 @@ func updatememstats() { memstats.total_alloc = totalAlloc memstats.alloc = totalAlloc - totalFree memstats.heap_objects = memstats.nmalloc - memstats.nfree + + memstats.stacks_inuse = uint64(consStats.inStacks) + memstats.gcWorkBufInUse = uint64(consStats.inWorkBufs) + memstats.gcProgPtrScalarBitsInUse = uint64(consStats.inPtrScalarBits) + + // We also count stacks_inuse, gcWorkBufInUse, and gcProgPtrScalarBitsInUse as sys memory. + memstats.sys += memstats.stacks_inuse + memstats.gcWorkBufInUse + memstats.gcProgPtrScalarBitsInUse + + // The world is stopped, so the consistent stats (after aggregation) + // should be identical to some combination of memstats. In particular: + // + // * heap_inuse == inHeap + // * heap_released == released + // * heap_sys - heap_released == committed - inStacks - inWorkBufs - inPtrScalarBits + // + // Check if that's actually true. + // + // TODO(mknyszek): Maybe don't throw here. It would be bad if a + // bug in otherwise benign accounting caused the whole application + // to crash. + if memstats.heap_inuse != uint64(consStats.inHeap) { + print("runtime: heap_inuse=", memstats.heap_inuse, "\n") + print("runtime: consistent value=", consStats.inHeap, "\n") + throw("heap_inuse and consistent stats are not equal") + } + if memstats.heap_released != uint64(consStats.released) { + print("runtime: heap_released=", memstats.heap_released, "\n") + print("runtime: consistent value=", consStats.released, "\n") + throw("heap_released and consistent stats are not equal") + } + globalRetained := memstats.heap_sys.load() - memstats.heap_released + consRetained := uint64(consStats.committed - consStats.inStacks - consStats.inWorkBufs - consStats.inPtrScalarBits) + if globalRetained != consRetained { + print("runtime: global value=", globalRetained, "\n") + print("runtime: consistent value=", consRetained, "\n") + throw("measures of the retained heap are not equal") + } } // flushmcache flushes the mcache of allp[i]. -- cgit v1.3 From 9393b5bae5944acebed3ab6f995926b7de3ce429 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Fri, 21 Aug 2020 11:59:55 -0400 Subject: runtime: add heap lock assertions Some functions that required holding the heap lock _or_ world stop have been simplified to simply requiring the heap lock. This is conceptually simpler and taking the heap lock during world stop is guaranteed to not contend. This was only done on functions already called on the systemstack to avoid too many extra systemstack calls in GC. Updates #40677 Change-Id: I15aa1dadcdd1a81aac3d2a9ecad6e7d0377befdc Reviewed-on: https://go-review.googlesource.com/c/go/+/250262 Run-TryBot: Michael Pratt TryBot-Result: Go Bot Reviewed-by: Austin Clements Trust: Michael Pratt --- src/runtime/export_test.go | 61 +++++++++++++++++++++++++++++++++++++++++----- src/runtime/malloc.go | 2 ++ src/runtime/mgc.go | 4 +++ src/runtime/mgcscavenge.go | 18 ++++++++++++++ src/runtime/mheap.go | 29 +++++++++++++++++----- src/runtime/mpagealloc.go | 22 +++++++++++++++++ src/runtime/mpagecache.go | 14 ++++++++++- src/runtime/proc.go | 2 ++ 8 files changed, 139 insertions(+), 13 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index 4ca0420d2a..44551dcaf1 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -743,7 +743,16 @@ func (c *PageCache) Alloc(npages uintptr) (uintptr, uintptr) { return (*pageCache)(c).alloc(npages) } func (c *PageCache) Flush(s *PageAlloc) { - (*pageCache)(c).flush((*pageAlloc)(s)) + cp := (*pageCache)(c) + sp := (*pageAlloc)(s) + + systemstack(func() { + // None of the tests need any higher-level locking, so we just + // take the lock internally. + lock(sp.mheapLock) + cp.flush(sp) + unlock(sp.mheapLock) + }) } // Expose chunk index type. @@ -754,13 +763,41 @@ type ChunkIdx chunkIdx type PageAlloc pageAlloc func (p *PageAlloc) Alloc(npages uintptr) (uintptr, uintptr) { - return (*pageAlloc)(p).alloc(npages) + pp := (*pageAlloc)(p) + + var addr, scav uintptr + systemstack(func() { + // None of the tests need any higher-level locking, so we just + // take the lock internally. + lock(pp.mheapLock) + addr, scav = pp.alloc(npages) + unlock(pp.mheapLock) + }) + return addr, scav } func (p *PageAlloc) AllocToCache() PageCache { - return PageCache((*pageAlloc)(p).allocToCache()) + pp := (*pageAlloc)(p) + + var c PageCache + systemstack(func() { + // None of the tests need any higher-level locking, so we just + // take the lock internally. + lock(pp.mheapLock) + c = PageCache(pp.allocToCache()) + unlock(pp.mheapLock) + }) + return c } func (p *PageAlloc) Free(base, npages uintptr) { - (*pageAlloc)(p).free(base, npages) + pp := (*pageAlloc)(p) + + systemstack(func() { + // None of the tests need any higher-level locking, so we just + // take the lock internally. + lock(pp.mheapLock) + pp.free(base, npages) + unlock(pp.mheapLock) + }) } func (p *PageAlloc) Bounds() (ChunkIdx, ChunkIdx) { return ChunkIdx((*pageAlloc)(p).start), ChunkIdx((*pageAlloc)(p).end) @@ -768,6 +805,8 @@ func (p *PageAlloc) Bounds() (ChunkIdx, ChunkIdx) { func (p *PageAlloc) Scavenge(nbytes uintptr, mayUnlock bool) (r uintptr) { pp := (*pageAlloc)(p) systemstack(func() { + // None of the tests need any higher-level locking, so we just + // take the lock internally. lock(pp.mheapLock) r = pp.scavenge(nbytes, mayUnlock) unlock(pp.mheapLock) @@ -926,7 +965,11 @@ func NewPageAlloc(chunks, scav map[ChunkIdx][]BitRange) *PageAlloc { addr := chunkBase(chunkIdx(i)) // Mark the chunk's existence in the pageAlloc. - p.grow(addr, pallocChunkBytes) + systemstack(func() { + lock(p.mheapLock) + p.grow(addr, pallocChunkBytes) + unlock(p.mheapLock) + }) // Initialize the bitmap and update pageAlloc metadata. chunk := p.chunkOf(chunkIndex(addr)) @@ -957,13 +1000,19 @@ func NewPageAlloc(chunks, scav map[ChunkIdx][]BitRange) *PageAlloc { } // Update heap metadata for the allocRange calls above. - p.update(addr, pallocChunkPages, false, false) + systemstack(func() { + lock(p.mheapLock) + p.update(addr, pallocChunkPages, false, false) + unlock(p.mheapLock) + }) } + systemstack(func() { lock(p.mheapLock) p.scavengeStartGen() unlock(p.mheapLock) }) + return (*PageAlloc)(p) } diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 0563f49d17..4b798d129c 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -627,6 +627,8 @@ func mallocinit() { // // h must be locked. func (h *mheap) sysAlloc(n uintptr) (v unsafe.Pointer, size uintptr) { + assertLockHeld(&h.lock) + n = alignUp(n, heapArenaBytes) // First, try the arena pre-reservation. diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index fb3c149942..185d3201ca 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -821,6 +821,8 @@ func pollFractionalWorkerExit() bool { // // mheap_.lock must be held or the world must be stopped. func gcSetTriggerRatio(triggerRatio float64) { + assertWorldStoppedOrLockHeld(&mheap_.lock) + // Compute the next GC goal, which is when the allocated heap // has grown by GOGC/100 over the heap marked by the last // cycle. @@ -960,6 +962,8 @@ func gcSetTriggerRatio(triggerRatio float64) { // // mheap_.lock must be held or the world must be stopped. func gcEffectiveGrowthRatio() float64 { + assertWorldStoppedOrLockHeld(&mheap_.lock) + egogc := float64(atomic.Load64(&memstats.next_gc)-memstats.heap_marked) / float64(memstats.heap_marked) if egogc < 0 { // Shouldn't happen, but just in case. diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 5843ada981..a242577bd9 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -397,6 +397,8 @@ func bgscavenge(c chan int) { // //go:systemstack func (p *pageAlloc) scavenge(nbytes uintptr, mayUnlock bool) uintptr { + assertLockHeld(p.mheapLock) + var ( addrs addrRange gen uint32 @@ -446,6 +448,8 @@ func printScavTrace(gen uint32, released uintptr, forced bool) { // //go:systemstack func (p *pageAlloc) scavengeStartGen() { + assertLockHeld(p.mheapLock) + if debug.scavtrace > 0 { printScavTrace(p.scav.gen, p.scav.released, false) } @@ -495,6 +499,8 @@ func (p *pageAlloc) scavengeStartGen() { // //go:systemstack func (p *pageAlloc) scavengeReserve() (addrRange, uint32) { + assertLockHeld(p.mheapLock) + // Start by reserving the minimum. r := p.scav.inUse.removeLast(p.scav.reservationBytes) @@ -525,6 +531,8 @@ func (p *pageAlloc) scavengeReserve() (addrRange, uint32) { // //go:systemstack func (p *pageAlloc) scavengeUnreserve(r addrRange, gen uint32) { + assertLockHeld(p.mheapLock) + if r.size() == 0 || gen != p.scav.gen { return } @@ -552,6 +560,8 @@ func (p *pageAlloc) scavengeUnreserve(r addrRange, gen uint32) { // //go:systemstack func (p *pageAlloc) scavengeOne(work addrRange, max uintptr, mayUnlock bool) (uintptr, addrRange) { + assertLockHeld(p.mheapLock) + // Defensively check if we've recieved an empty address range. // If so, just return. if work.size() == 0 { @@ -610,6 +620,8 @@ func (p *pageAlloc) scavengeOne(work addrRange, max uintptr, mayUnlock bool) (ui // If we found something, scavenge it and return! if npages != 0 { work.limit = offAddr{p.scavengeRangeLocked(maxChunk, base, npages)} + + assertLockHeld(p.mheapLock) // Must be locked on return. return uintptr(npages) * pageSize, work } } @@ -674,12 +686,16 @@ func (p *pageAlloc) scavengeOne(work addrRange, max uintptr, mayUnlock bool) (ui base, npages := chunk.findScavengeCandidate(pallocChunkPages-1, minPages, maxPages) if npages > 0 { work.limit = offAddr{p.scavengeRangeLocked(candidateChunkIdx, base, npages)} + + assertLockHeld(p.mheapLock) // Must be locked on return. return uintptr(npages) * pageSize, work } // We were fooled, so let's continue from where we left off. work.limit = offAddr{chunkBase(candidateChunkIdx)} } + + assertLockHeld(p.mheapLock) // Must be locked on return. return 0, work } @@ -692,6 +708,8 @@ func (p *pageAlloc) scavengeOne(work addrRange, max uintptr, mayUnlock bool) (ui // // p.mheapLock must be held. func (p *pageAlloc) scavengeRangeLocked(ci chunkIdx, base, npages uint) uintptr { + assertLockHeld(p.mheapLock) + p.chunkOf(ci).scavenged.setRange(base, npages) // Compute the full address for the start of the range. diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 14a73c0491..66a59cb999 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -483,10 +483,15 @@ func (s *mspan) layout() (size, n, total uintptr) { // indirect call from the fixalloc initializer, the compiler can't see // this. // +// The heap lock must be held. +// //go:nowritebarrierrec func recordspan(vh unsafe.Pointer, p unsafe.Pointer) { h := (*mheap)(vh) s := (*mspan)(p) + + assertLockHeld(&h.lock) + if len(h.allspans) >= cap(h.allspans) { n := 64 * 1024 / sys.PtrSize if n < cap(h.allspans)*3/2 { @@ -721,7 +726,7 @@ func (h *mheap) init() { // // reclaim implements the page-reclaimer half of the sweeper. // -// h must NOT be locked. +// h.lock must NOT be held. func (h *mheap) reclaim(npage uintptr) { // TODO(austin): Half of the time spent freeing spans is in // locking/unlocking the heap (even with low contention). We @@ -804,6 +809,8 @@ func (h *mheap) reclaimChunk(arenas []arenaIdx, pageIdx, n uintptr) uintptr { // In particular, if a span were freed and merged concurrently // with this probing heapArena.spans, it would be possible to // observe arbitrary, stale span pointers. + assertLockHeld(&h.lock) + n0 := n var nFreed uintptr sg := h.sweepgen @@ -858,6 +865,8 @@ func (h *mheap) reclaimChunk(arenas []arenaIdx, pageIdx, n uintptr) uintptr { traceGCSweepSpan((n0 - nFreed) * pageSize) lock(&h.lock) } + + assertLockHeld(&h.lock) // Must be locked on return. return nFreed } @@ -1011,7 +1020,7 @@ func (h *mheap) allocNeedsZero(base, npage uintptr) (needZero bool) { // tryAllocMSpan attempts to allocate an mspan object from // the P-local cache, but may fail. // -// h need not be locked. +// h.lock need not be held. // // This caller must ensure that its P won't change underneath // it during this function. Currently to ensure that we enforce @@ -1035,7 +1044,7 @@ func (h *mheap) tryAllocMSpan() *mspan { // allocMSpanLocked allocates an mspan object. // -// h must be locked. +// h.lock must be held. // // allocMSpanLocked must be called on the system stack because // its caller holds the heap lock. See mheap for details. @@ -1044,6 +1053,8 @@ func (h *mheap) tryAllocMSpan() *mspan { // //go:systemstack func (h *mheap) allocMSpanLocked() *mspan { + assertLockHeld(&h.lock) + pp := getg().m.p.ptr() if pp == nil { // We don't have a p so just do the normal thing. @@ -1065,7 +1076,7 @@ func (h *mheap) allocMSpanLocked() *mspan { // freeMSpanLocked free an mspan object. // -// h must be locked. +// h.lock must be held. // // freeMSpanLocked must be called on the system stack because // its caller holds the heap lock. See mheap for details. @@ -1074,6 +1085,8 @@ func (h *mheap) allocMSpanLocked() *mspan { // //go:systemstack func (h *mheap) freeMSpanLocked(s *mspan) { + assertLockHeld(&h.lock) + pp := getg().m.p.ptr() // First try to free the mspan directly to the cache. if pp != nil && pp.mspancache.len < len(pp.mspancache.buf) { @@ -1097,7 +1110,7 @@ func (h *mheap) freeMSpanLocked(s *mspan) { // // The returned span is fully initialized. // -// h must not be locked. +// h.lock must not be held. // // allocSpan must be called on the system stack both because it acquires // the heap lock and because it must block GC transitions. @@ -1281,8 +1294,10 @@ HaveSpan: // Try to add at least npage pages of memory to the heap, // returning whether it worked. // -// h must be locked. +// h.lock must be held. func (h *mheap) grow(npage uintptr) bool { + assertLockHeld(&h.lock) + // We must grow the heap in whole palloc chunks. ask := alignUp(npage, pallocChunkPages) * pageSize @@ -1391,6 +1406,8 @@ func (h *mheap) freeManual(s *mspan, typ spanAllocType) { } func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { + assertLockHeld(&h.lock) + switch s.state.get() { case mSpanManual: if s.allocCount != 0 { diff --git a/src/runtime/mpagealloc.go b/src/runtime/mpagealloc.go index 2af1c97e0b..dac1f39969 100644 --- a/src/runtime/mpagealloc.go +++ b/src/runtime/mpagealloc.go @@ -349,6 +349,8 @@ func (p *pageAlloc) chunkOf(ci chunkIdx) *pallocData { // // p.mheapLock must be held. func (p *pageAlloc) grow(base, size uintptr) { + assertLockHeld(p.mheapLock) + // Round up to chunks, since we can't deal with increments smaller // than chunks. Also, sysGrow expects aligned values. limit := alignUp(base+size, pallocChunkBytes) @@ -413,6 +415,8 @@ func (p *pageAlloc) grow(base, size uintptr) { // // p.mheapLock must be held. func (p *pageAlloc) update(base, npages uintptr, contig, alloc bool) { + assertLockHeld(p.mheapLock) + // base, limit, start, and end are inclusive. limit := base + npages*pageSize - 1 sc, ec := chunkIndex(base), chunkIndex(limit) @@ -499,6 +503,8 @@ func (p *pageAlloc) update(base, npages uintptr, contig, alloc bool) { // // p.mheapLock must be held. func (p *pageAlloc) allocRange(base, npages uintptr) uintptr { + assertLockHeld(p.mheapLock) + limit := base + npages*pageSize - 1 sc, ec := chunkIndex(base), chunkIndex(limit) si, ei := chunkPageIndex(base), chunkPageIndex(limit) @@ -534,6 +540,8 @@ func (p *pageAlloc) allocRange(base, npages uintptr) uintptr { // // p.mheapLock must be held. func (p *pageAlloc) findMappedAddr(addr offAddr) offAddr { + assertLockHeld(p.mheapLock) + // If we're not in a test, validate first by checking mheap_.arenas. // This is a fast path which is only safe to use outside of testing. ai := arenaIndex(addr.addr()) @@ -568,6 +576,8 @@ func (p *pageAlloc) findMappedAddr(addr offAddr) offAddr { // // p.mheapLock must be held. func (p *pageAlloc) find(npages uintptr) (uintptr, offAddr) { + assertLockHeld(p.mheapLock) + // Search algorithm. // // This algorithm walks each level l of the radix tree from the root level @@ -786,7 +796,13 @@ nextLevel: // should be ignored. // // p.mheapLock must be held. +// +// Must run on the system stack because p.mheapLock must be held. +// +//go:systemstack func (p *pageAlloc) alloc(npages uintptr) (addr uintptr, scav uintptr) { + assertLockHeld(p.mheapLock) + // If the searchAddr refers to a region which has a higher address than // any known chunk, then we know we're out of memory. if chunkIndex(p.searchAddr.addr()) >= p.end { @@ -841,7 +857,13 @@ Found: // free returns npages worth of memory starting at base back to the page heap. // // p.mheapLock must be held. +// +// Must run on the system stack because p.mheapLock must be held. +// +//go:systemstack func (p *pageAlloc) free(base, npages uintptr) { + assertLockHeld(p.mheapLock) + // If we're freeing pages below the p.searchAddr, update searchAddr. if b := (offAddr{base}); b.lessThan(p.searchAddr) { p.searchAddr = b diff --git a/src/runtime/mpagecache.go b/src/runtime/mpagecache.go index 5f76501a1c..4b5c66d8d6 100644 --- a/src/runtime/mpagecache.go +++ b/src/runtime/mpagecache.go @@ -71,8 +71,14 @@ func (c *pageCache) allocN(npages uintptr) (uintptr, uintptr) { // into s. Then, it clears the cache, such that empty returns // true. // -// p.mheapLock must be held or the world must be stopped. +// p.mheapLock must be held. +// +// Must run on the system stack because p.mheapLock must be held. +// +//go:systemstack func (c *pageCache) flush(p *pageAlloc) { + assertLockHeld(p.mheapLock) + if c.empty() { return } @@ -103,7 +109,13 @@ func (c *pageCache) flush(p *pageAlloc) { // chunk. // // p.mheapLock must be held. +// +// Must run on the system stack because p.mheapLock must be held. +// +//go:systemstack func (p *pageAlloc) allocToCache() pageCache { + assertLockHeld(p.mheapLock) + // If the searchAddr refers to a region which has a higher address than // any known chunk, then we know we're out of memory. if chunkIndex(p.searchAddr.addr()) >= p.end { diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 82284e6cd6..ced27ceb3a 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -4603,7 +4603,9 @@ func (pp *p) destroy() { mheap_.spanalloc.free(unsafe.Pointer(pp.mspancache.buf[i])) } pp.mspancache.len = 0 + lock(&mheap_.lock) pp.pcache.flush(&mheap_.pages) + unlock(&mheap_.lock) }) freemcache(pp.mcache) pp.mcache = nil -- cgit v1.3 From ac766e37182f36cd0a3247e44a4143d2d2132e42 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 2 Nov 2020 16:58:38 +0000 Subject: runtime: make getMCache inlineable This change moves the responsibility of throwing if an mcache is not available to the caller, because the inlining cost of throw is set very high in the compiler. Even if it was reduced down to the cost of a usual function call, it would still be too expensive, so just move it out. This choice also makes sense in the context of #42339 since we're going to have to handle the case where we don't have an mcache to update stats in a few contexts anyhow. Also, add getMCache to the list of functions that should be inlined to prevent future regressions. getMCache is called on the allocation fast path and because its not inlined actually causes a significant regression (~10%) in some microbenchmarks. Fixes #42305. Change-Id: I64ac5e4f26b730bd4435ea1069a4a50f55411ced Reviewed-on: https://go-review.googlesource.com/c/go/+/267157 Trust: Michael Knyszek Run-TryBot: Michael Knyszek Reviewed-by: Michael Pratt TryBot-Result: Go Bot --- src/cmd/compile/internal/gc/inl_test.go | 1 + src/runtime/malloc.go | 9 ++++++++- src/runtime/mcache.go | 7 ++----- src/runtime/mgcscavenge.go | 3 +++ src/runtime/mheap.go | 12 ++++++++++++ 5 files changed, 26 insertions(+), 6 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/cmd/compile/internal/gc/inl_test.go b/src/cmd/compile/internal/gc/inl_test.go index afa6b98315..02735e50fb 100644 --- a/src/cmd/compile/internal/gc/inl_test.go +++ b/src/cmd/compile/internal/gc/inl_test.go @@ -51,6 +51,7 @@ func TestIntendedInlining(t *testing.T) { "funcPC", "getArgInfoFast", "getm", + "getMCache", "isDirectIface", "itabHashFunc", "noescape", diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 4b798d129c..551acd0796 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -975,6 +975,9 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { shouldhelpgc := false dataSize := size c := getMCache() + if c == nil { + throw("mallocgc called without a P or outside bootstrapping") + } var span *mspan var x unsafe.Pointer noscan := typ == nil || typ.ptrdata == 0 @@ -1202,7 +1205,11 @@ func reflect_unsafe_NewArray(typ *_type, n int) unsafe.Pointer { } func profilealloc(mp *m, x unsafe.Pointer, size uintptr) { - getMCache().nextSample = nextSample() + c := getMCache() + if c == nil { + throw("profilealloc called without a P or outside bootstrapping") + } + c.nextSample = nextSample() mProf_Malloc(x, size) } diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index c9342a41c9..847a5dedf3 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -124,8 +124,8 @@ func freemcache(c *mcache) { // getMCache is a convenience function which tries to obtain an mcache. // -// Must be running with a P when called (so the caller must be in a -// non-preemptible state) or must be called during bootstrapping. +// Returns nil if we're not bootstrapping or we don't have a P. The caller's +// P must not change, so we must be in a non-preemptible state. func getMCache() *mcache { // Grab the mcache, since that's where stats live. pp := getg().m.p.ptr() @@ -136,9 +136,6 @@ func getMCache() *mcache { // mcache0 is cleared when bootstrapping is complete, // by procresize. c = mcache0 - if c == nil { - throw("getMCache called with no P or outside bootstrapping") - } } else { c = pp.mcache } diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index a242577bd9..ab4e28a60b 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -734,6 +734,9 @@ func (p *pageAlloc) scavengeRangeLocked(ci chunkIdx, base, npages uint) uintptr // Update consistent accounting too. c := getMCache() + if c == nil { + throw("scavengeRangeLocked called without a P or outside bootstrapping") + } stats := memstats.heapStats.acquire(c) atomic.Xaddint64(&stats.committed, -nbytes) atomic.Xaddint64(&stats.released, nbytes) diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 66a59cb999..6b29f34a82 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1247,6 +1247,10 @@ HaveSpan: } // Update consistent stats. c := getMCache() + if c == nil { + // TODO(mknyszek): Remove this and handle this case to fix #42339. + throw("allocSpan called without P or outside bootstrapping") + } stats := memstats.heapStats.acquire(c) atomic.Xaddint64(&stats.committed, int64(scav)) atomic.Xaddint64(&stats.released, -int64(scav)) @@ -1341,6 +1345,10 @@ func (h *mheap) grow(npage uintptr) bool { // just add directly to heap_released. atomic.Xadd64(&memstats.heap_released, int64(asize)) c := getMCache() + if c == nil { + // TODO(mknyszek): Remove this and handle this case to fix #42339. + throw("grow called without P or outside bootstrapping") + } stats := memstats.heapStats.acquire(c) atomic.Xaddint64(&stats.released, int64(asize)) memstats.heapStats.release(c) @@ -1440,6 +1448,10 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { } // Update consistent stats. c := getMCache() + if c == nil { + // TODO(mknyszek): Remove this and handle this case to fix #42339. + throw("freeSpanLocked called without P or outside bootstrapping") + } stats := memstats.heapStats.acquire(c) switch typ { case spanAllocHeap: -- cgit v1.3 From 39a5ee52b9b41b1e4f4cf821c78ef5b7be68d181 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 2 Nov 2020 19:03:16 +0000 Subject: runtime: decouple consistent stats from mcache and allow P-less update This change modifies the consistent stats implementation to keep the per-P sequence counter on each P instead of each mcache. A valid mcache is not available everywhere that we want to call e.g. allocSpan, as per issue #42339. By decoupling these two, we can add a mechanism to allow contexts without a P to update stats consistently. In this CL, we achieve that with a mutex. In practice, it will be very rare for an M to update these stats without a P. Furthermore, the stats reader also only needs to hold the mutex across the update to "gen" since once that changes, writers are free to continue updating the new stats generation. Contention could thus only arise between writers without a P, and as mentioned earlier, those should be rare. A nice side-effect of this change is that the consistent stats acquire and release API becomes simpler. Fixes #42339. Change-Id: Ied74ab256f69abd54b550394c8ad7c4c40a5fe34 Reviewed-on: https://go-review.googlesource.com/c/go/+/267158 Run-TryBot: Michael Knyszek Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/mcache.go | 16 +++----- src/runtime/mgcscavenge.go | 8 +--- src/runtime/mgcsweep.go | 10 ++--- src/runtime/mheap.go | 27 +++---------- src/runtime/mstats.go | 95 +++++++++++++++++++++++++++++----------------- src/runtime/proc.go | 4 ++ src/runtime/runtime2.go | 8 +++- 7 files changed, 88 insertions(+), 80 deletions(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 847a5dedf3..bb7475b6f3 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -50,10 +50,6 @@ type mcache struct { // in this mcache are stale and need to the flushed so they // can be swept. This is done in acquirep. flushGen uint32 - - // statsSeq is a counter indicating whether this P is currently - // writing any stats. Its value is even when not, odd when it is. - statsSeq uint32 } // A gclink is a node in a linked list of blocks, like mlink, @@ -178,9 +174,9 @@ 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. - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xadduintptr(&stats.smallAllocCount[spc.sizeclass()], uintptr(s.nelems)-uintptr(s.allocCount)) - memstats.heapStats.release(c) + memstats.heapStats.release() // Update heap_live with the same assumption. usedBytes := uintptr(s.allocCount) * s.elemsize @@ -229,10 +225,10 @@ func (c *mcache) allocLarge(size uintptr, needzero bool, noscan bool) *mspan { if s == nil { throw("out of memory") } - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xadduintptr(&stats.largeAlloc, npages*pageSize) atomic.Xadduintptr(&stats.largeAllocCount, 1) - memstats.heapStats.release(c) + memstats.heapStats.release() // Update heap_live and revise pacing if needed. atomic.Xadd64(&memstats.heap_live, int64(npages*pageSize)) @@ -263,9 +259,9 @@ func (c *mcache) releaseAll() { if s != &emptymspan { // Adjust nsmallalloc in case the span wasn't fully allocated. n := uintptr(s.nelems) - uintptr(s.allocCount) - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xadduintptr(&stats.smallAllocCount[spanClass(i).sizeclass()], -n) - memstats.heapStats.release(c) + memstats.heapStats.release() if s.sweepgen != sg+1 { // refill conservatively counted unallocated slots in heap_live. // Undo this. diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index ab4e28a60b..38f09309dc 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -733,14 +733,10 @@ func (p *pageAlloc) scavengeRangeLocked(ci chunkIdx, base, npages uint) uintptr atomic.Xadd64(&memstats.heap_released, nbytes) // Update consistent accounting too. - c := getMCache() - if c == nil { - throw("scavengeRangeLocked called without a P or outside bootstrapping") - } - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xaddint64(&stats.committed, -nbytes) atomic.Xaddint64(&stats.released, nbytes) - memstats.heapStats.release(c) + memstats.heapStats.release() return addr } diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 8391435630..76bc4246e5 100644 --- a/src/runtime/mgcsweep.go +++ b/src/runtime/mgcsweep.go @@ -339,8 +339,6 @@ func (s *mspan) sweep(preserve bool) bool { spc := s.spanclass size := s.elemsize - c := _g_.m.p.ptr().mcache - // The allocBits indicate which unmarked objects don't need to be // processed since they were free at the end of the last GC cycle // and were not allocated since then. @@ -505,9 +503,9 @@ func (s *mspan) sweep(preserve bool) bool { // wasn't totally filled, but then swept, still has all of its // free slots zeroed. s.needzero = 1 - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xadduintptr(&stats.smallFreeCount[spc.sizeclass()], uintptr(nfreed)) - memstats.heapStats.release(c) + memstats.heapStats.release() } if !preserve { // The caller may not have removed this span from whatever @@ -552,10 +550,10 @@ func (s *mspan) sweep(preserve bool) bool { } else { mheap_.freeSpan(s) } - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xadduintptr(&stats.largeFreeCount, 1) atomic.Xadduintptr(&stats.largeFree, size) - memstats.heapStats.release(c) + memstats.heapStats.release() return true } diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index 6b29f34a82..b8429eee94 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -1246,12 +1246,7 @@ HaveSpan: memstats.heap_sys.add(-int64(nbytes)) } // Update consistent stats. - c := getMCache() - if c == nil { - // TODO(mknyszek): Remove this and handle this case to fix #42339. - throw("allocSpan called without P or outside bootstrapping") - } - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xaddint64(&stats.committed, int64(scav)) atomic.Xaddint64(&stats.released, -int64(scav)) switch typ { @@ -1264,7 +1259,7 @@ HaveSpan: case spanAllocWorkBuf: atomic.Xaddint64(&stats.inWorkBufs, int64(nbytes)) } - memstats.heapStats.release(c) + memstats.heapStats.release() // Publish the span in various locations. @@ -1344,14 +1339,9 @@ func (h *mheap) grow(npage uintptr) bool { // size which is always > physPageSize, so its safe to // just add directly to heap_released. atomic.Xadd64(&memstats.heap_released, int64(asize)) - c := getMCache() - if c == nil { - // TODO(mknyszek): Remove this and handle this case to fix #42339. - throw("grow called without P or outside bootstrapping") - } - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() atomic.Xaddint64(&stats.released, int64(asize)) - memstats.heapStats.release(c) + memstats.heapStats.release() // Recalculate nBase. // We know this won't overflow, because sysAlloc returned @@ -1447,12 +1437,7 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { memstats.heap_sys.add(int64(nbytes)) } // Update consistent stats. - c := getMCache() - if c == nil { - // TODO(mknyszek): Remove this and handle this case to fix #42339. - throw("freeSpanLocked called without P or outside bootstrapping") - } - stats := memstats.heapStats.acquire(c) + stats := memstats.heapStats.acquire() switch typ { case spanAllocHeap: atomic.Xaddint64(&stats.inHeap, -int64(nbytes)) @@ -1463,7 +1448,7 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) { case spanAllocWorkBuf: atomic.Xaddint64(&stats.inWorkBufs, -int64(nbytes)) } - memstats.heapStats.release(c) + memstats.heapStats.release() // Mark the space as free. h.pages.free(s.base(), s.npages) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 3829355d7b..6defaedabe 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -158,7 +158,7 @@ type mstats struct { // heapStats is a set of statistics heapStats consistentHeapStats - _ uint32 // ensure gcPauseDist is aligned + // _ uint32 // ensure gcPauseDist is aligned // gcPauseDist represents the distribution of all GC-related // application pauses in the runtime. @@ -818,10 +818,11 @@ type consistentHeapStats struct { // Writers always atomically update the delta at index gen. // // Readers operate by rotating gen (0 -> 1 -> 2 -> 0 -> ...) - // and synchronizing with writers by observing each mcache's - // statsSeq field. If the reader observes a P (to which the - // mcache is bound) not writing, it can be sure that it will - // pick up the new gen value the next time it writes. + // and synchronizing with writers by observing each P's + // statsSeq field. If the reader observes a P not writing, + // it can be sure that it will pick up the new gen value the + // next time it writes. + // // The reader then takes responsibility by clearing space // in the ring buffer for the next reader to rotate gen to // that space (i.e. it merges in values from index (gen-2) mod 3 @@ -830,7 +831,7 @@ type consistentHeapStats struct { // Note that this means only one reader can be reading at a time. // There is no way for readers to synchronize. // - // This process is why we need ring buffer of size 3 instead + // This process is why we need a ring buffer of size 3 instead // of 2: one is for the writers, one contains the most recent // data, and the last one is clear so writers can begin writing // to it the moment gen is updated. @@ -840,24 +841,34 @@ type consistentHeapStats struct { // are writing, and can take on the value of 0, 1, or 2. // This value is updated atomically. gen uint32 + + // noPLock is intended to provide mutual exclusion for updating + // stats when no P is available. It does not block other writers + // with a P, only other writers without a P and the reader. Because + // stats are usually updated when a P is available, contention on + // this lock should be minimal. + noPLock mutex } // acquire returns a heapStatsDelta to be updated. In effect, // it acquires the shard for writing. release must be called -// as soon as the relevant deltas are updated. c must be -// a valid mcache not being used by any other thread. +// as soon as the relevant deltas are updated. // // The returned heapStatsDelta must be updated atomically. // -// Note however, that this is unsafe to call concurrently -// with other writers and there must be only one writer -// at a time. -func (m *consistentHeapStats) acquire(c *mcache) *heapStatsDelta { - seq := atomic.Xadd(&c.statsSeq, 1) - if seq%2 == 0 { - // Should have been incremented to odd. - print("runtime: seq=", seq, "\n") - throw("bad sequence number") +// The caller's P must not change between acquire and +// release. This also means that the caller should not +// acquire a P or release its P in between. +func (m *consistentHeapStats) acquire() *heapStatsDelta { + if pp := getg().m.p.ptr(); pp != nil { + seq := atomic.Xadd(&pp.statsSeq, 1) + if seq%2 == 0 { + // Should have been incremented to odd. + print("runtime: seq=", seq, "\n") + throw("bad sequence number") + } + } else { + lock(&m.noPLock) } gen := atomic.Load(&m.gen) % 3 return &m.stats[gen] @@ -868,14 +879,19 @@ func (m *consistentHeapStats) acquire(c *mcache) *heapStatsDelta { // acquire must no longer be accessed or modified after // release is called. // -// The mcache passed here must be the same as the one -// passed to acquire. -func (m *consistentHeapStats) release(c *mcache) { - seq := atomic.Xadd(&c.statsSeq, 1) - if seq%2 != 0 { - // Should have been incremented to even. - print("runtime: seq=", seq, "\n") - throw("bad sequence number") +// The caller's P must not change between acquire and +// release. This also means that the caller should not +// acquire a P or release its P in between. +func (m *consistentHeapStats) release() { + if pp := getg().m.p.ptr(); pp != nil { + seq := atomic.Xadd(&pp.statsSeq, 1) + if seq%2 != 0 { + // Should have been incremented to even. + print("runtime: seq=", seq, "\n") + throw("bad sequence number") + } + } else { + unlock(&m.noPLock) } } @@ -916,25 +932,33 @@ func (m *consistentHeapStats) read(out *heapStatsDelta) { // so it doesn't change out from under us. mp := acquirem() + // Get the current generation. We can be confident that this + // will not change since read is serialized and is the only + // one that modifies currGen. + currGen := atomic.Load(&m.gen) + prevGen := currGen - 1 + if currGen == 0 { + prevGen = 2 + } + + // Prevent writers without a P from writing while we update gen. + lock(&m.noPLock) + // Rotate gen, effectively taking a snapshot of the state of // these statistics at the point of the exchange by moving // writers to the next set of deltas. // // This exchange is safe to do because we won't race // with anyone else trying to update this value. - currGen := atomic.Load(&m.gen) atomic.Xchg(&m.gen, (currGen+1)%3) - prevGen := currGen - 1 - if currGen == 0 { - prevGen = 2 - } + + // Allow P-less writers to continue. They'll be writing to the + // next generation now. + unlock(&m.noPLock) + for _, p := range allp { - c := p.mcache - if c == nil { - continue - } // Spin until there are no more writers. - for atomic.Load(&c.statsSeq)%2 != 0 { + for atomic.Load(&p.statsSeq)%2 != 0 { } } @@ -951,5 +975,6 @@ func (m *consistentHeapStats) read(out *heapStatsDelta) { // Finally, copy out the complete delta. *out = m.stats[currGen] + releasem(mp) } diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 79529ac7ec..87949a2694 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -577,6 +577,10 @@ func schedinit() { lockInit(&trace.lock, lockRankTrace) lockInit(&cpuprof.lock, lockRankCpuprof) lockInit(&trace.stackTab.lock, lockRankTraceStackTab) + // Enforce that this lock is always a leaf lock. + // All of this lock's critical sections should be + // extremely short. + lockInit(&memstats.heapStats.noPLock, lockRankLeafRank) // raceinit must be the first call to race detector. // In particular, it must be done before mallocinit below calls racemapshadow. diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 82fedd804b..c9376827da 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -654,8 +654,8 @@ type p struct { timerModifiedEarliest uint64 // Per-P GC state - gcAssistTime int64 // Nanoseconds in assistAlloc - gcFractionalMarkTime int64 // Nanoseconds in fractional mark worker (atomic) + gcAssistTime int64 // Nanoseconds in assistAlloc + gcFractionalMarkTime int64 // Nanoseconds in fractional mark worker (atomic) // gcMarkWorkerMode is the mode for the next mark worker to run in. // That is, this is used to communicate with the worker goroutine @@ -679,6 +679,10 @@ type p struct { runSafePointFn uint32 // if 1, run sched.safePointFn at next safe point + // statsSeq is a counter indicating whether this P is currently + // writing any stats. Its value is even when not, odd when it is. + statsSeq uint32 + // Lock for timers. We normally access the timers while running // on this P, but the scheduler can also do it from a different P. timersLock mutex -- cgit v1.3 From 5ca43acdb3e27117e6994141e518b8d55e4d32aa Mon Sep 17 00:00:00 2001 From: Joel Sing Date: Mon, 2 Nov 2020 03:58:08 +1100 Subject: runtime: allow physical page aligned stacks to be allocated Add a physPageAlignedStack boolean which if set, results in over allocation by a physical page, the allocation being rounded to physical page alignment and the unused memory surrounding the allocation being freed again. OpenBSD/octeon has 16KB physical pages and requires stacks to be physical page aligned in order for them to be remapped as MAP_STACK. This change allows Go to work on this platform. Based on a suggestion from mknyszek in issue #41008. Updates #40995 Fixes #41008 Change-Id: Ia5d652292b515916db473043b41f6030094461d8 Reviewed-on: https://go-review.googlesource.com/c/go/+/266919 Trust: Joel Sing Reviewed-by: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot --- src/runtime/mheap.go | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) (limited to 'src/runtime/mheap.go') diff --git a/src/runtime/mheap.go b/src/runtime/mheap.go index b8429eee94..1855330da5 100644 --- a/src/runtime/mheap.go +++ b/src/runtime/mheap.go @@ -44,6 +44,11 @@ const ( // Must be a multiple of the pageInUse bitmap element size and // must also evenly divide pagesPerArena. pagesPerReclaimerChunk = 512 + + // physPageAlignedStacks indicates whether stack allocations must be + // physical page aligned. This is a requirement for MAP_STACK on + // OpenBSD. + physPageAlignedStacks = GOOS == "openbsd" ) // Main malloc heap. @@ -1121,9 +1126,16 @@ func (h *mheap) allocSpan(npages uintptr, typ spanAllocType, spanclass spanClass gp := getg() base, scav := uintptr(0), uintptr(0) + // On some platforms we need to provide physical page aligned stack + // allocations. Where the page size is less than the physical page + // size, we already manage to do this by default. + needPhysPageAlign := physPageAlignedStacks && typ == spanAllocStack && pageSize < physPageSize + // If the allocation is small enough, try the page cache! + // The page cache does not support aligned allocations, so we cannot use + // it if we need to provide a physical page aligned stack allocation. pp := gp.m.p.ptr() - if pp != nil && npages < pageCachePages/4 { + if !needPhysPageAlign && pp != nil && npages < pageCachePages/4 { c := &pp.pcache // If the cache is empty, refill it. @@ -1149,6 +1161,11 @@ func (h *mheap) allocSpan(npages uintptr, typ spanAllocType, spanclass spanClass // whole job done without the heap lock. lock(&h.lock) + if needPhysPageAlign { + // Overallocate by a physical page to allow for later alignment. + npages += physPageSize / pageSize + } + if base == 0 { // Try to acquire a base address. base, scav = h.pages.alloc(npages) @@ -1168,6 +1185,23 @@ func (h *mheap) allocSpan(npages uintptr, typ spanAllocType, spanclass spanClass // one now that we have the heap lock. s = h.allocMSpanLocked() } + + if needPhysPageAlign { + allocBase, allocPages := base, npages + base = alignUp(allocBase, physPageSize) + npages -= physPageSize / pageSize + + // Return memory around the aligned allocation. + spaceBefore := base - allocBase + if spaceBefore > 0 { + h.pages.free(allocBase, spaceBefore/pageSize) + } + spaceAfter := (allocPages-npages)*pageSize - spaceBefore + if spaceAfter > 0 { + h.pages.free(base+npages*pageSize, spaceAfter/pageSize) + } + } + unlock(&h.lock) HaveSpan: -- cgit v1.3