From 14c7caae5074fdf0d97a3ad995e20c63e4065cbf Mon Sep 17 00:00:00 2001 From: Martin Möhrmann Date: Mon, 13 Jul 2020 18:12:20 +0200 Subject: runtime: add 24 byte allocation size class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL introduces a 24 byte allocation size class which fits 3 pointers on 64 bit and 6 pointers on 32 bit architectures. Notably this new size class fits a slice header on 64 bit architectures exactly while previously a 32 byte size class would have been used for allocating a slice header on the heap. The main complexity added with this CL is that heapBitsSetType needs to handle objects that aren't 16-byte aligned but contain more than a single pointer on 64-bit architectures. Due to having a non 16 byte aligned size class on 32 bit a h.shift of 2 is now possible which means a heap bitmap byte might only be partially written. Due to this already having been possible on 64 bit before the heap bitmap code only needed minor adjustments for 32 bit doublecheck code paths. Note that this CL changes the slice capacity allocated by append for slice growth to a target capacity of 17 to 24 bytes. On 64 bit architectures the capacity of the slice returned by append([]byte{}, make([]byte, 24)...)) is 32 bytes before and 24 bytes after this CL. Depending on allocation patterns of the specific Go program this can increase the number of total alloctions as subsequent appends to the slice can trigger slice growth earlier than before. On the other side if the slice is never appended to again above its capacity this will lower heap usage by 8 bytes. This CL changes the set of size classes reported in the runtime.MemStats.BySize array due to it being limited to a total of 61 size classes. The new 24 byte size class is now included and the 20480 byte size class is not included anymore. Fixes #8885 name old time/op new time/op delta Template 196ms ± 3% 194ms ± 2% ~ (p=0.247 n=10+10) Unicode 85.6ms ±16% 88.1ms ± 1% ~ (p=0.165 n=10+10) GoTypes 673ms ± 2% 668ms ± 2% ~ (p=0.258 n=9+9) Compiler 3.14s ± 6% 3.08s ± 1% ~ (p=0.243 n=10+9) SSA 6.82s ± 1% 6.76s ± 1% -0.87% (p=0.006 n=9+10) Flate 128ms ± 7% 127ms ± 3% ~ (p=0.739 n=10+10) GoParser 154ms ± 3% 153ms ± 4% ~ (p=0.730 n=9+9) Reflect 404ms ± 1% 412ms ± 4% +1.99% (p=0.022 n=9+10) Tar 172ms ± 4% 170ms ± 4% ~ (p=0.065 n=10+9) XML 231ms ± 4% 230ms ± 3% ~ (p=0.912 n=10+10) LinkCompiler 341ms ± 1% 339ms ± 1% ~ (p=0.243 n=9+10) ExternalLinkCompiler 1.72s ± 1% 1.72s ± 1% ~ (p=0.661 n=9+10) LinkWithoutDebugCompiler 221ms ± 2% 221ms ± 2% ~ (p=0.529 n=10+10) StdCmd 18.4s ± 3% 18.2s ± 1% ~ (p=0.515 n=10+8) name old user-time/op new user-time/op delta Template 238ms ± 4% 243ms ± 6% ~ (p=0.661 n=9+10) Unicode 116ms ± 6% 113ms ± 3% -3.37% (p=0.035 n=9+10) GoTypes 854ms ± 2% 848ms ± 2% ~ (p=0.604 n=9+10) Compiler 4.10s ± 1% 4.11s ± 1% ~ (p=0.481 n=8+9) SSA 9.49s ± 1% 9.41s ± 1% -0.92% (p=0.001 n=9+10) Flate 149ms ± 6% 151ms ± 7% ~ (p=0.481 n=10+10) GoParser 189ms ± 2% 190ms ± 2% ~ (p=0.497 n=9+10) Reflect 511ms ± 2% 508ms ± 2% ~ (p=0.211 n=9+10) Tar 215ms ± 4% 212ms ± 3% ~ (p=0.105 n=10+10) XML 288ms ± 2% 288ms ± 2% ~ (p=0.971 n=10+10) LinkCompiler 559ms ± 4% 557ms ± 1% ~ (p=0.968 n=9+10) ExternalLinkCompiler 1.78s ± 1% 1.77s ± 1% ~ (p=0.055 n=8+10) LinkWithoutDebugCompiler 245ms ± 3% 245ms ± 2% ~ (p=0.684 n=10+10) name old alloc/op new alloc/op delta Template 34.8MB ± 0% 34.4MB ± 0% -0.95% (p=0.000 n=9+10) Unicode 28.6MB ± 0% 28.3MB ± 0% -0.95% (p=0.000 n=10+10) GoTypes 115MB ± 0% 114MB ± 0% -1.02% (p=0.000 n=10+9) Compiler 554MB ± 0% 549MB ± 0% -0.86% (p=0.000 n=9+10) SSA 1.28GB ± 0% 1.27GB ± 0% -0.83% (p=0.000 n=10+10) Flate 21.8MB ± 0% 21.6MB ± 0% -0.87% (p=0.000 n=8+10) GoParser 26.7MB ± 0% 26.4MB ± 0% -0.97% (p=0.000 n=10+9) Reflect 75.0MB ± 0% 74.1MB ± 0% -1.18% (p=0.000 n=10+10) Tar 32.6MB ± 0% 32.3MB ± 0% -0.94% (p=0.000 n=10+7) XML 41.5MB ± 0% 41.2MB ± 0% -0.90% (p=0.000 n=10+8) LinkCompiler 105MB ± 0% 104MB ± 0% -0.94% (p=0.000 n=10+10) ExternalLinkCompiler 153MB ± 0% 152MB ± 0% -0.69% (p=0.000 n=10+10) LinkWithoutDebugCompiler 63.7MB ± 0% 63.6MB ± 0% -0.13% (p=0.000 n=10+10) name old allocs/op new allocs/op delta Template 336k ± 0% 336k ± 0% +0.02% (p=0.002 n=10+10) Unicode 332k ± 0% 332k ± 0% ~ (p=0.447 n=10+10) GoTypes 1.16M ± 0% 1.16M ± 0% +0.01% (p=0.001 n=10+10) Compiler 4.92M ± 0% 4.92M ± 0% +0.01% (p=0.000 n=10+10) SSA 11.9M ± 0% 11.9M ± 0% +0.02% (p=0.000 n=9+10) Flate 214k ± 0% 214k ± 0% +0.02% (p=0.032 n=10+8) GoParser 270k ± 0% 270k ± 0% +0.02% (p=0.004 n=10+9) Reflect 877k ± 0% 877k ± 0% +0.01% (p=0.000 n=10+10) Tar 313k ± 0% 313k ± 0% ~ (p=0.075 n=9+10) XML 387k ± 0% 387k ± 0% +0.02% (p=0.007 n=10+10) LinkCompiler 455k ± 0% 456k ± 0% +0.08% (p=0.000 n=10+9) ExternalLinkCompiler 670k ± 0% 671k ± 0% +0.06% (p=0.000 n=10+10) LinkWithoutDebugCompiler 113k ± 0% 113k ± 0% ~ (p=0.149 n=10+10) name old maxRSS/op new maxRSS/op delta Template 34.1M ± 1% 34.1M ± 1% ~ (p=0.853 n=10+10) Unicode 35.1M ± 1% 34.6M ± 1% -1.43% (p=0.000 n=10+10) GoTypes 72.8M ± 3% 73.3M ± 2% ~ (p=0.724 n=10+10) Compiler 288M ± 3% 295M ± 4% ~ (p=0.393 n=10+10) SSA 630M ± 1% 622M ± 1% -1.18% (p=0.001 n=10+10) Flate 26.0M ± 1% 26.2M ± 2% ~ (p=0.493 n=10+10) GoParser 28.6M ± 1% 28.5M ± 2% ~ (p=0.256 n=10+10) Reflect 55.5M ± 2% 55.4M ± 1% ~ (p=0.436 n=10+10) Tar 33.0M ± 1% 32.8M ± 2% ~ (p=0.075 n=10+10) XML 38.7M ± 1% 39.0M ± 1% ~ (p=0.053 n=9+10) LinkCompiler 164M ± 1% 164M ± 1% -0.27% (p=0.029 n=10+10) ExternalLinkCompiler 174M ± 0% 173M ± 0% -0.33% (p=0.002 n=9+10) LinkWithoutDebugCompiler 137M ± 0% 136M ± 2% ~ (p=0.825 n=9+10) Change-Id: I9ecf2a10024513abef8fbfbe519e44e0b29b6167 Reviewed-on: https://go-review.googlesource.com/c/go/+/242258 Trust: Martin Möhrmann Trust: Michael Knyszek Run-TryBot: Martin Möhrmann TryBot-Result: Go Bot Reviewed-by: Michael Knyszek Reviewed-by: Keith Randall --- src/runtime/mstats.go | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 6a8a34d1ed..b95b332134 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -78,6 +78,10 @@ type mstats struct { nfree uint64 } + // Add an uint32 for even number of size classes to align below fields + // to 64 bits for atomic operations on 32 bit platforms. + _ [1 - _NumSizeClasses%2]uint32 + // Statistics below here are not exported to MemStats directly. last_gc_nanotime uint64 // last gc (monotonic time) -- cgit v1.3 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/mgc.go | 5 +++-- src/runtime/mheap.go | 4 ++-- src/runtime/mstats.go | 4 +++- 3 files changed, 8 insertions(+), 5 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index c42c7fbd29..94539dd770 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -494,6 +494,7 @@ func (c *gcControllerState) revise() { gcpercent = 100000 } live := atomic.Load64(&memstats.heap_live) + scan := atomic.Load64(&memstats.heap_scan) // Assume we're under the soft goal. Pace GC to complete at // next_gc assuming the heap is in steady-state. @@ -508,7 +509,7 @@ func (c *gcControllerState) revise() { // // (This is a float calculation to avoid overflowing on // 100*heap_scan.) - scanWorkExpected := int64(float64(memstats.heap_scan) * 100 / float64(100+gcpercent)) + scanWorkExpected := int64(float64(scan) * 100 / float64(100+gcpercent)) if live > memstats.next_gc || c.scanWork > scanWorkExpected { // We're past the soft goal, or we've already done more scan @@ -518,7 +519,7 @@ func (c *gcControllerState) revise() { heapGoal = int64(float64(memstats.next_gc) * maxOvershoot) // Compute the upper bound on the scan work remaining. - scanWorkExpected = int64(memstats.heap_scan) + scanWorkExpected = int64(scan) } // Compute the remaining scan work estimate. 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 diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index b95b332134..2c217ecf84 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -139,6 +139,8 @@ type mstats struct { // no-scan objects and no-scan tails of objects. // // Whenever this is updated, call gcController.revise(). + // + // Read and written atomically or with the world stopped. heap_scan uint64 // heap_marked is the number of bytes marked by the previous @@ -635,7 +637,7 @@ func flushallmcaches() { func purgecachedstats(c *mcache) { // Protected by either heap or GC lock. h := &mheap_ - 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 f5c6875f3228951afa1fcf2ec01c614e0fb7e2dd Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 20:24:56 +0000 Subject: runtime: make next_gc atomically accessed next_gc is mostly updated only during a STW, but may occasionally be updated by calls to e.g. debug.SetGCPercent. In this case the update is supposed to be protected by the heap lock, but in reality it's accessed by gcController.revise which may be called without the heap lock held (despite its documentation, which will be updated in a later change). Change the synchronization policy on next_gc so that it's atomically accessed when the world is not stopped to aid in making revise safe for concurrent use. Change-Id: I79657a72f91563f3241aaeda66e8a7757d399529 Reviewed-on: https://go-review.googlesource.com/c/go/+/246962 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/mgc.go | 13 +++++++------ src/runtime/mgcscavenge.go | 2 +- src/runtime/mstats.go | 10 ++++++++-- src/runtime/trace.go | 5 +++-- 4 files changed, 19 insertions(+), 11 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 4b9a6da3b3..5c565a5853 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -409,7 +409,8 @@ type gcControllerState struct { } // startCycle resets the GC controller's state and computes estimates -// for a new GC cycle. The caller must hold worldsema. +// for a new GC cycle. The caller must hold worldsema and the world +// must be stopped. func (c *gcControllerState) startCycle() { c.scanWork = 0 c.bgScanCredit = 0 @@ -499,7 +500,7 @@ func (c *gcControllerState) revise() { // Assume we're under the soft goal. Pace GC to complete at // next_gc assuming the heap is in steady-state. - heapGoal := int64(memstats.next_gc) + heapGoal := int64(atomic.Load64(&memstats.next_gc)) // Compute the expected scan work remaining. // @@ -512,12 +513,12 @@ func (c *gcControllerState) revise() { // 100*heap_scan.) scanWorkExpected := int64(float64(scan) * 100 / float64(100+gcpercent)) - if live > memstats.next_gc || work > scanWorkExpected { + if int64(live) > heapGoal || work > scanWorkExpected { // We're past the soft goal, or we've already done more scan // work than we expected. Pace GC so that in the worst case it // will complete by the hard goal. const maxOvershoot = 1.1 - heapGoal = int64(float64(memstats.next_gc) * maxOvershoot) + heapGoal = int64(float64(heapGoal) * maxOvershoot) // Compute the upper bound on the scan work remaining. scanWorkExpected = int64(scan) @@ -846,7 +847,7 @@ func gcSetTriggerRatio(triggerRatio float64) { // Commit to the trigger and goal. memstats.gc_trigger = trigger - memstats.next_gc = goal + atomic.Store64(&memstats.next_gc, goal) if trace.enabled { traceNextGC() } @@ -903,7 +904,7 @@ func gcSetTriggerRatio(triggerRatio float64) { // // mheap_.lock must be held or the world must be stopped. func gcEffectiveGrowthRatio() float64 { - egogc := float64(memstats.next_gc-memstats.heap_marked) / float64(memstats.heap_marked) + egogc := float64(atomic.Load64(&memstats.next_gc)-memstats.heap_marked) / float64(memstats.heap_marked) if egogc < 0 { // Shouldn't happen, but just in case. egogc = 0 diff --git a/src/runtime/mgcscavenge.go b/src/runtime/mgcscavenge.go index 34646828e5..6328b295ca 100644 --- a/src/runtime/mgcscavenge.go +++ b/src/runtime/mgcscavenge.go @@ -123,7 +123,7 @@ func gcPaceScavenger() { return } // Compute our scavenging goal. - goalRatio := float64(memstats.next_gc) / float64(memstats.last_next_gc) + goalRatio := float64(atomic.Load64(&memstats.next_gc)) / float64(memstats.last_next_gc) retainedGoal := uint64(float64(memstats.last_heap_inuse) * goalRatio) // Add retainExtraPercent overhead to retainedGoal. This calculation // looks strange but the purpose is to arrive at an integer division diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 2c217ecf84..8cc20552fb 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -57,9 +57,15 @@ type mstats struct { gc_sys uint64 // updated atomically or during STW other_sys uint64 // updated atomically or during STW - // Statistics about garbage collector. + // Statistics about the garbage collector. + + // next_gc is the goal heap_live for when next GC ends. + // Set to ^uint64(0) if disabled. + // + // Read and written atomically, unless the world is stopped. + next_gc uint64 + // Protected by mheap or stopping the world during GC. - next_gc uint64 // goal heap_live for when next GC ends; ^0 if disabled last_gc_unix uint64 // last gc (in unix time) pause_total_ns uint64 pause_ns [256]uint64 // circular buffer of recent gc pause lengths diff --git a/src/runtime/trace.go b/src/runtime/trace.go index 169b650eb4..d3ecd148be 100644 --- a/src/runtime/trace.go +++ b/src/runtime/trace.go @@ -13,6 +13,7 @@ package runtime import ( + "runtime/internal/atomic" "runtime/internal/sys" "unsafe" ) @@ -1146,11 +1147,11 @@ func traceHeapAlloc() { } func traceNextGC() { - if memstats.next_gc == ^uint64(0) { + if nextGC := atomic.Load64(&memstats.next_gc); nextGC == ^uint64(0) { // Heap-based triggering is disabled. traceEvent(traceEvNextGC, -1, 0) } else { - traceEvent(traceEvNextGC, -1, memstats.next_gc) + traceEvent(traceEvNextGC, -1, nextGC) } } -- 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/mstats.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/mstats.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 a5088e76f108f6470d2a9b3ac56a58ddb9376e4f Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 23 Jul 2020 22:07:44 +0000 Subject: runtime: remove mcentral.nmalloc and add mcache.local_nsmallalloc This change removes mcentral.nmalloc and adds mcache.local_nsmallalloc which fulfills the same role but may be accessed non-atomically. It also moves responsibility for updating heap_live and local_nsmallalloc into mcache functions. As a result of this change, mcache is now the sole source-of-truth for malloc stats. It is also solely responsible for updating heap_live and performing the various operations required as a result of updating heap_live. The overall improvement here is in code organization: previously malloc stats were fairly scattered, and now they have one single home, and nearly all the required manipulations exist in a single file. Change-Id: I7e93fa297c1debf17e3f2a0d68aeed28a9c6af00 Reviewed-on: https://go-review.googlesource.com/c/go/+/246966 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/mcache.go | 34 ++++++++++++++++++++++++++++++++++ src/runtime/mcentral.go | 41 +---------------------------------------- src/runtime/mstats.go | 18 ++++++------------ 3 files changed, 41 insertions(+), 52 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index 3657c0b86a..4d2ba6dff0 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -51,6 +51,7 @@ type mcache struct { // application. 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 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) @@ -138,6 +139,10 @@ func (c *mcache) donate(d *mcache) { c.local_largealloc = 0 d.local_nlargealloc += c.local_nlargealloc c.local_nlargealloc = 0 + for i := range c.local_nsmallalloc { + d.local_nsmallalloc[i] += c.local_nsmallalloc[i] + c.local_nsmallalloc[i] = 0 + } d.local_largefree += c.local_largefree c.local_largefree = 0 d.local_nlargefree += c.local_nlargefree @@ -182,6 +187,20 @@ func (c *mcache) refill(spc spanClass) { // sweeping in the next sweep phase. s.sweepgen = mheap_.sweepgen + 3 + // 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) + usedBytes := uintptr(s.allocCount) * s.elemsize + atomic.Xadd64(&memstats.heap_live, int64(s.npages*pageSize)-int64(usedBytes)) + if trace.enabled { + // heap_live changed. + traceHeapAlloc() + } + if gcBlackenEnabled != 0 { + // heap_live changed. + gcController.revise() + } + c.alloc[spc] = s } @@ -227,9 +246,24 @@ func (c *mcache) largeAlloc(size uintptr, needzero bool, noscan bool) *mspan { } func (c *mcache) releaseAll() { + sg := mheap_.sweepgen for i := range c.alloc { s := c.alloc[i] if s != &emptymspan { + // Adjust nsmallalloc in case the span wasn't fully allocated. + n := uintptr(s.nelems) - uintptr(s.allocCount) + c.local_nsmallalloc[spanClass(i).sizeclass()] -= n + if s.sweepgen != sg+1 { + // refill conservatively counted unallocated slots in heap_live. + // Undo this. + // + // If this span was cached before sweep, then + // heap_live was totally recomputed since + // caching this span, so we don't do this for + // stale spans. + atomic.Xadd64(&memstats.heap_live, -int64(n)*int64(s.elemsize)) + } + // Release the span to the mcentral. mheap_.central[i].mcentral.uncacheSpan(s) c.alloc[i] = &emptymspan } diff --git a/src/runtime/mcentral.go b/src/runtime/mcentral.go index ed49e01677..97fe92c2ab 100644 --- a/src/runtime/mcentral.go +++ b/src/runtime/mcentral.go @@ -44,11 +44,6 @@ type mcentral struct { // encounter swept spans, and these should be ignored. partial [2]spanSet // list of spans with a free object full [2]spanSet // list of spans with no free objects - - // nmalloc is the cumulative count of objects allocated from - // this mcentral, assuming all spans in mcaches are - // fully-allocated. Written atomically, read under STW. - nmalloc uint64 } // Initialize a single central free list. @@ -178,19 +173,6 @@ havespan: if n == 0 || s.freeindex == s.nelems || uintptr(s.allocCount) == s.nelems { throw("span has no free objects") } - // Assume all objects from this span will be allocated in the - // mcache. If it gets uncached, we'll adjust this. - atomic.Xadd64(&c.nmalloc, int64(n)) - usedBytes := uintptr(s.allocCount) * s.elemsize - atomic.Xadd64(&memstats.heap_live, int64(spanBytes)-int64(usedBytes)) - if trace.enabled { - // heap_live changed. - traceHeapAlloc() - } - if gcBlackenEnabled != 0 { - // heap_live changed. - gcController.revise() - } freeByteBase := s.freeindex &^ (64 - 1) whichByte := freeByteBase / 8 // Init alloc bits cache. @@ -228,27 +210,6 @@ func (c *mcentral) uncacheSpan(s *mspan) { // Indicate that s is no longer cached. atomic.Store(&s.sweepgen, sg) } - n := int(s.nelems) - int(s.allocCount) - - // Fix up statistics. - if n > 0 { - // cacheSpan updated alloc assuming all objects on s - // were going to be allocated. Adjust for any that - // weren't. We must do this before potentially - // sweeping the span. - atomic.Xadd64(&c.nmalloc, -int64(n)) - - if !stale { - // (*mcentral).cacheSpan conservatively counted - // unallocated slots in heap_live. Undo this. - // - // If this span was cached before sweep, then - // heap_live was totally recomputed since - // caching this span, so we don't do this for - // stale spans. - atomic.Xadd64(&memstats.heap_live, -int64(n)*int64(s.elemsize)) - } - } // Put the span in the appropriate place. if stale { @@ -256,7 +217,7 @@ func (c *mcentral) uncacheSpan(s *mspan) { // the right list. s.sweep(false) } else { - if n > 0 { + if int(s.nelems)-int(s.allocCount) > 0 { // Put it back on the partial swept list. c.partialSwept(sg).push(s) } else { diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index d9acb361d5..44cf17c85b 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -561,17 +561,6 @@ func updatememstats() { // Collect allocation stats. This is safe and consistent // because the world is stopped. var smallFree, totalAlloc, totalFree uint64 - // Collect per-spanclass stats. - for spc := range mheap_.central { - // The mcaches are now empty, so mcentral stats are - // up-to-date. - c := &mheap_.central[spc].mcentral - memstats.nmalloc += c.nmalloc - i := spanClass(spc).sizeclass() - memstats.by_size[i].nmalloc += c.nmalloc - totalAlloc += c.nmalloc * uint64(class_to_size[i]) - } - for _, p := range allp { c := p.mcache if c == nil { @@ -585,12 +574,17 @@ func updatememstats() { // Collect per-sizeclass stats. for i := 0; i < _NumSizeClasses; i++ { + // Malloc stats. + memstats.nmalloc += uint64(c.local_nsmallalloc[i]) + memstats.by_size[i].nmalloc += uint64(c.local_nsmallalloc[i]) + totalAlloc += uint64(c.local_nsmallalloc[i]) * uint64(class_to_size[i]) + + // Free stats. 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. 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/mstats.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/mstats.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 c8638498008f9874dc5a48734418e0fbea08cee9 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 24 Jul 2020 19:58:31 +0000 Subject: runtime: rename mcache fields to match Go style This change renames a bunch of malloc statistics stored in the mcache that are all named with the "local_" prefix. It also renames largeAlloc to allocLarge to prevent a naming conflict, and next_sample because it would be the last mcache field with the old C naming style. Change-Id: I29695cb83b397a435ede7e9ad5c3c9be72767ea3 Reviewed-on: https://go-review.googlesource.com/c/go/+/246969 Trust: Michael Knyszek Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt --- src/runtime/export_test.go | 14 ++++---- src/runtime/malloc.go | 12 +++---- src/runtime/mcache.go | 78 ++++++++++++++++++++--------------------- src/runtime/mgc.go | 8 ++--- src/runtime/mgcsweep.go | 6 ++-- src/runtime/mstats.go | 22 ++++++------ src/runtime/pprof/mprof_test.go | 2 +- 7 files changed, 71 insertions(+), 71 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index d71b180f76..47cbc286f6 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -346,18 +346,18 @@ func ReadMemStatsSlow() (base, slow MemStats) { continue } // Collect large allocation stats. - largeFree += uint64(c.local_largefree) - slow.Frees += uint64(c.local_nlargefree) + largeFree += uint64(c.largeFree) + slow.Frees += uint64(c.largeFreeCount) // Collect tiny allocation stats. - tinyAllocs += uint64(c.local_tinyallocs) + tinyAllocs += uint64(c.tinyAllocCount) // 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 += uint64(c.smallFreeCount[i]) + bySize[i].Frees += uint64(c.smallFreeCount[i]) + bySize[i].Mallocs += uint64(c.smallFreeCount[i]) + smallFree += uint64(c.smallFreeCount[i]) * uint64(class_to_size[i]) } } slow.Frees += tinyAllocs diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index ec601ccb39..0f48d7f68e 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -1040,7 +1040,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { // The object fits into existing tiny block. x = unsafe.Pointer(c.tiny + off) c.tinyoffset = off + size - c.local_tinyallocs++ + c.tinyAllocCount++ mp.mallocing = 0 releasem(mp) return x @@ -1082,7 +1082,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { } } else { shouldhelpgc = true - span = c.largeAlloc(size, needzero, noscan) + span = c.allocLarge(size, needzero, noscan) span.freeindex = 1 span.allocCount = 1 x = unsafe.Pointer(span.base()) @@ -1111,7 +1111,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { } else { scanSize = typ.ptrdata } - c.local_scan += scanSize + c.scanAlloc += scanSize } // Ensure that the stores above that initialize x to @@ -1153,8 +1153,8 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { } if rate := MemProfileRate; rate > 0 { - if rate != 1 && size < c.next_sample { - c.next_sample -= size + if rate != 1 && size < c.nextSample { + c.nextSample -= size } else { mp := acquirem() profilealloc(mp, x, size) @@ -1221,7 +1221,7 @@ func profilealloc(mp *m, x unsafe.Pointer, size uintptr) { throw("profilealloc called with no P") } } - c.next_sample = nextSample() + c.nextSample = nextSample() mProf_Malloc(x, size) } diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index b8e388cc4f..c3e0e5e1f7 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -20,8 +20,8 @@ import ( type mcache struct { // The following members are accessed on every malloc, // so they are grouped here for better caching. - next_sample uintptr // trigger heap sample after allocating this many bytes - local_scan uintptr // bytes of scannable heap allocated + nextSample uintptr // trigger heap sample after allocating this many bytes + scanAlloc uintptr // bytes of scannable heap allocated // Allocator cache for tiny objects w/o pointers. // See "Tiny allocator" comment in malloc.go. @@ -48,13 +48,13 @@ 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 - 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) + tinyAllocCount uintptr // number of tiny allocs not counted in other stats + largeAlloc uintptr // bytes allocated for large objects + largeAllocCount uintptr // number of large object allocations + smallAllocCount [_NumSizeClasses]uintptr // number of allocs for small objects + largeFree uintptr // bytes freed for large objects (>maxSmallSize) + largeFreeCount uintptr // number of frees for large objects (>maxSmallSize) + smallFreeCount [_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 @@ -103,7 +103,7 @@ func allocmcache() *mcache { for i := range c.alloc { c.alloc[i] = &emptymspan } - c.next_sample = nextSample() + c.nextSample = nextSample() return c } @@ -134,26 +134,26 @@ 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 + // scanAlloc 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 - c.local_nlargealloc = 0 - for i := range c.local_nsmallalloc { - d.local_nsmallalloc[i] += c.local_nsmallalloc[i] - c.local_nsmallalloc[i] = 0 + d.largeAlloc += c.largeAlloc + c.largeAlloc = 0 + d.largeAllocCount += c.largeAllocCount + c.largeAllocCount = 0 + for i := range c.smallAllocCount { + d.smallAllocCount[i] += c.smallAllocCount[i] + c.smallAllocCount[i] = 0 } - 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 + d.largeFree += c.largeFree + c.largeFree = 0 + d.largeFreeCount += c.largeFreeCount + c.largeFreeCount = 0 + for i := range c.smallFreeCount { + d.smallFreeCount[i] += c.smallFreeCount[i] + c.smallFreeCount[i] = 0 } - d.local_tinyallocs += c.local_tinyallocs - c.local_tinyallocs = 0 + d.tinyAllocCount += c.tinyAllocCount + c.tinyAllocCount = 0 } // refill acquires a new span of span class spc for c. This span will @@ -192,16 +192,16 @@ 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) + c.smallAllocCount[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 + // While we're here, flush scanAlloc, since we have to call // revise anyway. - atomic.Xadd64(&memstats.heap_scan, int64(c.local_scan)) - c.local_scan = 0 + atomic.Xadd64(&memstats.heap_scan, int64(c.scanAlloc)) + c.scanAlloc = 0 if trace.enabled { // heap_live changed. @@ -215,8 +215,8 @@ 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 { +// allocLarge allocates a span for a large object. +func (c *mcache) allocLarge(size uintptr, needzero bool, noscan bool) *mspan { if size+_PageSize < size { throw("out of memory") } @@ -235,8 +235,8 @@ func (c *mcache) largeAlloc(size uintptr, needzero bool, noscan bool) *mspan { if s == nil { throw("out of memory") } - c.local_largealloc += npages * pageSize - c.local_nlargealloc++ + c.largeAlloc += npages * pageSize + c.largeAllocCount++ // Update heap_live and revise pacing if needed. atomic.Xadd64(&memstats.heap_live, int64(npages*pageSize)) @@ -257,9 +257,9 @@ 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 + // Take this opportunity to flush scanAlloc. + atomic.Xadd64(&memstats.heap_scan, int64(c.scanAlloc)) + c.scanAlloc = 0 sg := mheap_.sweepgen for i := range c.alloc { @@ -267,7 +267,7 @@ func (c *mcache) releaseAll() { if s != &emptymspan { // Adjust nsmallalloc in case the span wasn't fully allocated. n := uintptr(s.nelems) - uintptr(s.allocCount) - c.local_nsmallalloc[spanClass(i).sizeclass()] -= n + c.smallAllocCount[spanClass(i).sizeclass()] -= n if s.sweepgen != sg+1 { // refill conservatively counted unallocated slots in heap_live. // Undo this. diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 55554c117c..540c376f1c 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -2086,16 +2086,16 @@ func gcMark(start_time int64) { // 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 + // Flush scanAlloc from each mcache since we're about to modify + // heap_scan directly. If we were to flush this later, then scanAlloc // 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 + memstats.heap_scan += uint64(c.scanAlloc) + c.scanAlloc = 0 } // Update other GC heap size stats. This must happen after diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 6b8c56ce35..7103b08455 100644 --- a/src/runtime/mgcsweep.go +++ b/src/runtime/mgcsweep.go @@ -503,7 +503,7 @@ 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 - c.local_nsmallfree[spc.sizeclass()] += uintptr(nfreed) + c.smallFreeCount[spc.sizeclass()] += uintptr(nfreed) } if !preserve { // The caller may not have removed this span from whatever @@ -548,8 +548,8 @@ func (s *mspan) sweep(preserve bool) bool { } else { mheap_.freeSpan(s) } - c.local_nlargefree++ - c.local_largefree += size + c.largeFreeCount++ + c.largeFree += size return true } diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 5eeb173640..64687c24e5 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -565,25 +565,25 @@ 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) + memstats.nmalloc += uint64(c.largeAllocCount) + totalAlloc += uint64(c.largeAlloc) + totalFree += uint64(c.largeFree) + memstats.nfree += uint64(c.largeFreeCount) // Collect tiny allocation stats. - memstats.tinyallocs += uint64(c.local_tinyallocs) + memstats.tinyallocs += uint64(c.tinyAllocCount) // Collect per-sizeclass stats. for i := 0; i < _NumSizeClasses; i++ { // Malloc stats. - memstats.nmalloc += uint64(c.local_nsmallalloc[i]) - memstats.by_size[i].nmalloc += uint64(c.local_nsmallalloc[i]) - totalAlloc += uint64(c.local_nsmallalloc[i]) * uint64(class_to_size[i]) + memstats.nmalloc += uint64(c.smallAllocCount[i]) + memstats.by_size[i].nmalloc += uint64(c.smallAllocCount[i]) + totalAlloc += uint64(c.smallAllocCount[i]) * uint64(class_to_size[i]) // Free stats. - 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]) + memstats.nfree += uint64(c.smallFreeCount[i]) + memstats.by_size[i].nfree += uint64(c.smallFreeCount[i]) + smallFree += uint64(c.smallFreeCount[i]) * uint64(class_to_size[i]) } } diff --git a/src/runtime/pprof/mprof_test.go b/src/runtime/pprof/mprof_test.go index f253f07def..c11a45fd69 100644 --- a/src/runtime/pprof/mprof_test.go +++ b/src/runtime/pprof/mprof_test.go @@ -70,7 +70,7 @@ func TestMemoryProfiler(t *testing.T) { runtime.MemProfileRate = oldRate }() - // Allocate a meg to ensure that mcache.next_sample is updated to 1. + // Allocate a meg to ensure that mcache.nextSample is updated to 1. for i := 0; i < 1024; i++ { memSink = make([]byte, 1024) } -- 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/mstats.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 39e335ac0618044bbd8ed2fca5e5b3583d8c444e Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 31 Jul 2020 21:32:26 +0000 Subject: runtime: copy in MemStats fields explicitly Currently MemStats is populated via an unsafe memmove from memstats, but this places unnecessary structural restrictions on memstats, is annoying to reason about, and tightly couples the two. Instead, just populate the fields of MemStats explicitly. Change-Id: I96f6a64326b1a91d4084e7b30169a4bbe6a331f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/246972 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/mstats.go | 70 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 23 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 571a9c9ce3..466f33836c 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -12,8 +12,6 @@ import ( ) // Statistics. -// If you edit this structure, also edit type MemStats below. -// Their layouts must match exactly. // // For detailed descriptions see the documentation for MemStats. // Fields that differ from MemStats are further documented here. @@ -87,8 +85,6 @@ type mstats struct { // to 64 bits for atomic operations on 32 bit platforms. _ [1 - _NumSizeClasses%2]uint32 - // Statistics below here are not exported to MemStats directly. - last_gc_nanotime uint64 // last gc (monotonic time) tinyallocs uint64 // number of tiny allocations that didn't cause actual allocation; not exported to go directly last_next_gc uint64 // next_gc for the previous GC @@ -430,20 +426,7 @@ type MemStats struct { } } -// Size of the trailing by_size array differs between mstats and MemStats, -// and all data after by_size is local to runtime, not exported. -// NumSizeClasses was changed, but we cannot change MemStats because of backward compatibility. -// sizeof_C_MStats is the size of the prefix of mstats that -// corresponds to MemStats. It should match Sizeof(MemStats{}). -var sizeof_C_MStats = unsafe.Offsetof(memstats.by_size) + 61*unsafe.Sizeof(memstats.by_size[0]) - func init() { - var memStats MemStats - if sizeof_C_MStats != unsafe.Sizeof(memStats) { - println(sizeof_C_MStats, unsafe.Sizeof(memStats)) - throw("MStats vs MemStatsType size mismatch") - } - if unsafe.Offsetof(memstats.heap_live)%8 != 0 { println(unsafe.Offsetof(memstats.heap_live)) throw("memstats.heap_live not aligned to 8 bytes") @@ -469,14 +452,55 @@ func ReadMemStats(m *MemStats) { func readmemstats_m(stats *MemStats) { updatememstats() - // The size of the trailing by_size array differs between - // mstats and MemStats. NumSizeClasses was changed, but we - // cannot change MemStats because of backward compatibility. - memmove(unsafe.Pointer(stats), unsafe.Pointer(&memstats), sizeof_C_MStats) - + stats.Alloc = memstats.alloc + stats.TotalAlloc = memstats.total_alloc + stats.Sys = memstats.sys + stats.Mallocs = memstats.nmalloc + stats.Frees = memstats.nfree + stats.HeapAlloc = memstats.heap_alloc + stats.HeapSys = memstats.heap_sys.load() + stats.HeapIdle = memstats.heap_idle + stats.HeapInuse = memstats.heap_inuse + stats.HeapReleased = memstats.heap_released + stats.HeapObjects = memstats.heap_objects + stats.StackInuse = memstats.stacks_inuse // memstats.stacks_sys is only memory mapped directly for OS stacks. // Add in heap-allocated stack memory for user consumption. - stats.StackSys += stats.StackInuse + stats.StackSys = memstats.stacks_inuse + memstats.stacks_sys.load() + stats.MSpanInuse = memstats.mspan_inuse + stats.MSpanSys = memstats.mspan_sys.load() + stats.MCacheInuse = memstats.mcache_inuse + stats.MCacheSys = memstats.mcache_sys.load() + stats.BuckHashSys = memstats.buckhash_sys.load() + stats.GCSys = memstats.gc_sys.load() + stats.OtherSys = memstats.other_sys.load() + stats.NextGC = memstats.next_gc + stats.LastGC = memstats.last_gc_unix + stats.PauseTotalNs = memstats.pause_total_ns + stats.PauseNs = memstats.pause_ns + stats.PauseEnd = memstats.pause_end + stats.NumGC = memstats.numgc + stats.NumForcedGC = memstats.numforcedgc + stats.GCCPUFraction = memstats.gc_cpu_fraction + stats.EnableGC = true + + // Handle BySize. Copy N values, where N is + // the minimum of the lengths of the two arrays. + // Unfortunately copy() won't work here because + // the arrays have different structs. + // + // TODO(mknyszek): Consider renaming the fields + // of by_size's elements to align so we can use + // the copy built-in. + bySizeLen := len(stats.BySize) + if l := len(memstats.by_size); l < bySizeLen { + bySizeLen = l + } + for i := 0; i < bySizeLen; i++ { + stats.BySize[i].Size = memstats.by_size[i].size + stats.BySize[i].Mallocs = memstats.by_size[i].nmalloc + stats.BySize[i].Frees = memstats.by_size[i].nfree + } } //go:linkname readGCStats runtime/debug.readGCStats -- 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/mstats.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/mstats.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 ae585ee52c2437bfd0e955ad6fc8911bf292f51d Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 3 Aug 2020 19:31:23 +0000 Subject: runtime: remove memstats.heap_alloc memstats.heap_alloc is 100% a duplicate and unnecessary copy of memstats.alloc which exists because MemStats used to be populated from memstats via a memmove. Change-Id: I995489f61be39786e573b8494a8ab6d4ea8bed9c Reviewed-on: https://go-review.googlesource.com/c/go/+/246975 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/heapdump.go | 2 +- src/runtime/mstats.go | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index f96475e848..6fcd9746af 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -550,7 +550,7 @@ func dumpmemstats() { dumpint(memstats.nlookup) dumpint(memstats.nmalloc) dumpint(memstats.nfree) - dumpint(memstats.heap_alloc) + dumpint(memstats.alloc) dumpint(memstats.heap_sys.load()) dumpint(memstats.heap_sys.load() - memstats.heap_inuse) dumpint(memstats.heap_inuse) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 43f74273f7..a6e38d1c1b 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -32,7 +32,6 @@ type mstats struct { // // Like MemStats, heap_sys and heap_inuse do not count memory // 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_inuse uint64 // bytes in mSpanInUse spans heap_released uint64 // bytes released to the os @@ -112,11 +111,10 @@ type mstats struct { // heap_live is the number of bytes considered live by the GC. // That is: retained by the most recent GC plus allocated - // since then. heap_live <= heap_alloc, since heap_alloc - // includes unmarked objects that have not yet been swept (and - // hence goes up as we allocate and down as we sweep) while - // heap_live excludes these objects (and hence only goes up - // between GCs). + // since then. heap_live <= alloc, since alloc includes unmarked + // objects that have not yet been swept (and hence goes up as we + // allocate and down as we sweep) while heap_live excludes these + // objects (and hence only goes up between GCs). // // This is updated atomically without locking. To reduce // contention, this is updated only when obtaining a span from @@ -458,7 +456,7 @@ func readmemstats_m(stats *MemStats) { stats.Sys = memstats.sys stats.Mallocs = memstats.nmalloc stats.Frees = memstats.nfree - stats.HeapAlloc = memstats.heap_alloc + stats.HeapAlloc = memstats.alloc stats.HeapSys = memstats.heap_sys.load() // By definition, HeapIdle is memory that was mapped // for the heap but is not currently used to hold heap @@ -639,7 +637,6 @@ func updatememstats() { // Calculate derived stats. memstats.total_alloc = totalAlloc memstats.alloc = totalAlloc - totalFree - memstats.heap_alloc = memstats.alloc memstats.heap_objects = memstats.nmalloc - memstats.nfree } -- 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/mstats.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/mstats.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 79781e8dd382ac34e502ed6a088dff6860a08c05 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Tue, 4 Aug 2020 17:29:03 +0000 Subject: runtime: move malloc stats into consistentHeapStats This change moves the mcache-local malloc stats into the consistentHeapStats structure so the malloc stats can be managed consistently with the memory stats. The one exception here is tinyAllocs for which moving that into the global stats would incur several atomic writes on the fast path. Microbenchmarks for just one CPU core have shown a 50% loss in throughput. Since tiny allocation counnt isn't exposed anyway and is always blindly added to both allocs and frees, let that stay inconsistent and flush the tiny allocation count every so often. Change-Id: I2a4b75f209c0e659b9c0db081a3287bf227c10ca Reviewed-on: https://go-review.googlesource.com/c/go/+/247039 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/export_test.go | 37 ++++++++-------------- src/runtime/malloc.go | 2 +- src/runtime/mcache.go | 70 ++++++++++++++--------------------------- src/runtime/mgcsweep.go | 10 ++++-- src/runtime/mstats.go | 78 ++++++++++++++++++++++++++-------------------- src/runtime/proc.go | 2 +- 6 files changed, 90 insertions(+), 109 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index cb753ee819..ff901fd7be 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -337,33 +337,22 @@ func ReadMemStatsSlow() (base, slow MemStats) { } } - // Add in frees. readmemstats_m flushed the cached stats, so - // these are up-to-date. - var tinyAllocs, largeFree, smallFree uint64 - for _, p := range allp { - c := p.mcache - if c == nil { - continue - } - // Collect large allocation stats. - largeFree += uint64(c.largeFree) - slow.Frees += uint64(c.largeFreeCount) - - // Collect tiny allocation stats. - tinyAllocs += uint64(c.tinyAllocCount) - - // Collect per-sizeclass stats. - for i := 0; i < _NumSizeClasses; i++ { - slow.Frees += uint64(c.smallFreeCount[i]) - bySize[i].Frees += uint64(c.smallFreeCount[i]) - bySize[i].Mallocs += uint64(c.smallFreeCount[i]) - smallFree += uint64(c.smallFreeCount[i]) * uint64(class_to_size[i]) - } + // Add in frees by just reading the stats for those directly. + var m heapStatsDelta + memstats.heapStats.unsafeRead(&m) + + // Collect per-sizeclass free stats. + var smallFree uint64 + for i := 0; i < _NumSizeClasses; i++ { + slow.Frees += uint64(m.smallFreeCount[i]) + bySize[i].Frees += uint64(m.smallFreeCount[i]) + bySize[i].Mallocs += uint64(m.smallFreeCount[i]) + smallFree += uint64(m.smallFreeCount[i]) * uint64(class_to_size[i]) } - slow.Frees += tinyAllocs + slow.Frees += memstats.tinyallocs + uint64(m.largeFreeCount) slow.Mallocs += slow.Frees - slow.TotalAlloc = slow.Alloc + largeFree + smallFree + slow.TotalAlloc = slow.Alloc + uint64(m.largeFree) + smallFree for i := range slow.BySize { slow.BySize[i].Mallocs = bySize[i].Mallocs diff --git a/src/runtime/malloc.go b/src/runtime/malloc.go index 6383c34817..d0b8c668c3 100644 --- a/src/runtime/malloc.go +++ b/src/runtime/malloc.go @@ -1028,7 +1028,7 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { // The object fits into existing tiny block. x = unsafe.Pointer(c.tiny + off) c.tinyoffset = off + size - c.tinyAllocCount++ + c.tinyAllocs++ mp.mallocing = 0 releasem(mp) return x diff --git a/src/runtime/mcache.go b/src/runtime/mcache.go index e27a1c9ec0..c9342a41c9 100644 --- a/src/runtime/mcache.go +++ b/src/runtime/mcache.go @@ -32,8 +32,12 @@ 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. + // + // tinyAllocs is the number of tiny allocations performed + // by the P that owns this mcache. tiny uintptr tinyoffset uintptr + tinyAllocs uintptr // The rest is not accessed on every malloc. @@ -41,21 +45,6 @@ type mcache struct { stackcache [_NumStackOrders]stackfreelist - // 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. - tinyAllocCount uintptr // number of tiny allocs not counted in other stats - largeAlloc uintptr // bytes allocated for large objects - largeAllocCount uintptr // number of large object allocations - smallAllocCount [_NumSizeClasses]uintptr // number of allocs for small objects - largeFree uintptr // bytes freed for large objects (>maxSmallSize) - largeFreeCount uintptr // number of frees for large objects (>maxSmallSize) - smallFreeCount [_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 // in this mcache are stale and need to the flushed so they @@ -117,7 +106,7 @@ func allocmcache() *mcache { // 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) { +func freemcache(c *mcache) { systemstack(func() { c.releaseAll() stackcache_clear(c) @@ -128,8 +117,6 @@ func freemcache(c *mcache, recipient *mcache) { // gcworkbuffree(c.gcworkbuf) lock(&mheap_.lock) - // Donate anything else that's left. - c.donate(recipient) mheap_.cachealloc.free(unsafe.Pointer(c)) unlock(&mheap_.lock) }) @@ -158,31 +145,6 @@ func getMCache() *mcache { return c } -// donate flushes data and resources which have no global -// pool to another mcache. -func (c *mcache) donate(d *mcache) { - // scanAlloc is handled separately because it's not - // like these stats -- it's used for GC pacing. - d.largeAlloc += c.largeAlloc - c.largeAlloc = 0 - d.largeAllocCount += c.largeAllocCount - c.largeAllocCount = 0 - for i := range c.smallAllocCount { - d.smallAllocCount[i] += c.smallAllocCount[i] - c.smallAllocCount[i] = 0 - } - d.largeFree += c.largeFree - c.largeFree = 0 - d.largeFreeCount += c.largeFreeCount - c.largeFreeCount = 0 - for i := range c.smallFreeCount { - d.smallFreeCount[i] += c.smallFreeCount[i] - c.smallFreeCount[i] = 0 - } - d.tinyAllocCount += c.tinyAllocCount - c.tinyAllocCount = 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. // @@ -219,12 +181,20 @@ 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.smallAllocCount[spc.sizeclass()] += uintptr(s.nelems) - uintptr(s.allocCount) + stats := memstats.heapStats.acquire(c) + atomic.Xadduintptr(&stats.smallAllocCount[spc.sizeclass()], uintptr(s.nelems)-uintptr(s.allocCount)) + memstats.heapStats.release(c) // Update heap_live with the same assumption. usedBytes := uintptr(s.allocCount) * s.elemsize atomic.Xadd64(&memstats.heap_live, int64(s.npages*pageSize)-int64(usedBytes)) + // Flush tinyAllocs. + if spc == tinySpanClass { + atomic.Xadd64(&memstats.tinyallocs, int64(c.tinyAllocs)) + c.tinyAllocs = 0 + } + // While we're here, flush scanAlloc, since we have to call // revise anyway. atomic.Xadd64(&memstats.heap_scan, int64(c.scanAlloc)) @@ -262,8 +232,10 @@ func (c *mcache) allocLarge(size uintptr, needzero bool, noscan bool) *mspan { if s == nil { throw("out of memory") } - c.largeAlloc += npages * pageSize - c.largeAllocCount++ + stats := memstats.heapStats.acquire(c) + atomic.Xadduintptr(&stats.largeAlloc, npages*pageSize) + atomic.Xadduintptr(&stats.largeAllocCount, 1) + memstats.heapStats.release(c) // Update heap_live and revise pacing if needed. atomic.Xadd64(&memstats.heap_live, int64(npages*pageSize)) @@ -294,7 +266,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) - c.smallAllocCount[spanClass(i).sizeclass()] -= n + stats := memstats.heapStats.acquire(c) + atomic.Xadduintptr(&stats.smallAllocCount[spanClass(i).sizeclass()], -n) + memstats.heapStats.release(c) if s.sweepgen != sg+1 { // refill conservatively counted unallocated slots in heap_live. // Undo this. @@ -313,6 +287,8 @@ func (c *mcache) releaseAll() { // Clear tinyalloc pool. c.tiny = 0 c.tinyoffset = 0 + atomic.Xadd64(&memstats.tinyallocs, int64(c.tinyAllocs)) + c.tinyAllocs = 0 // Updated heap_scan and possible heap_live. if gcBlackenEnabled != 0 { diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 7103b08455..9b77ce635c 100644 --- a/src/runtime/mgcsweep.go +++ b/src/runtime/mgcsweep.go @@ -503,7 +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 - c.smallFreeCount[spc.sizeclass()] += uintptr(nfreed) + stats := memstats.heapStats.acquire(c) + atomic.Xadduintptr(&stats.smallFreeCount[spc.sizeclass()], uintptr(nfreed)) + memstats.heapStats.release(c) } if !preserve { // The caller may not have removed this span from whatever @@ -548,8 +550,10 @@ func (s *mspan) sweep(preserve bool) bool { } else { mheap_.freeSpan(s) } - c.largeFreeCount++ - c.largeFree += size + stats := memstats.heapStats.acquire(c) + atomic.Xadduintptr(&stats.largeFreeCount, 1) + atomic.Xadduintptr(&stats.largeFree, size) + memstats.heapStats.release(c) return true } diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 4363eff1e0..a8eca85fe6 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -612,48 +612,36 @@ 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 } - - // Collect allocation stats. This is safe and consistent - // because the world is stopped. - var smallFree, totalAlloc, totalFree uint64 - for _, p := range allp { - c := p.mcache - if c == nil { - continue - } - // Collect large allocation stats. - memstats.nmalloc += uint64(c.largeAllocCount) - totalAlloc += uint64(c.largeAlloc) - totalFree += uint64(c.largeFree) - memstats.nfree += uint64(c.largeFreeCount) - - // Collect tiny allocation stats. - memstats.tinyallocs += uint64(c.tinyAllocCount) - - // Collect per-sizeclass stats. - for i := 0; i < _NumSizeClasses; i++ { - // Malloc stats. - memstats.nmalloc += uint64(c.smallAllocCount[i]) - memstats.by_size[i].nmalloc += uint64(c.smallAllocCount[i]) - totalAlloc += uint64(c.smallAllocCount[i]) * uint64(class_to_size[i]) - - // Free stats. - memstats.nfree += uint64(c.smallFreeCount[i]) - memstats.by_size[i].nfree += uint64(c.smallFreeCount[i]) - 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 + // Collect large allocation stats. + totalAlloc := uint64(consStats.largeAlloc) + memstats.nmalloc += uint64(consStats.largeAllocCount) + totalFree := uint64(consStats.largeFree) + memstats.nfree += uint64(consStats.largeFreeCount) + + // Collect per-sizeclass stats. + for i := 0; i < _NumSizeClasses; i++ { + // Malloc stats. + a := uint64(consStats.smallAllocCount[i]) + totalAlloc += a * uint64(class_to_size[i]) + memstats.nmalloc += a + memstats.by_size[i].nmalloc = a + + // Free stats. + f := uint64(consStats.smallFreeCount[i]) + totalFree += f * uint64(class_to_size[i]) + memstats.nfree += f + memstats.by_size[i].nfree = f + } + // Account for tiny allocations. memstats.nfree += memstats.tinyallocs memstats.nmalloc += memstats.tinyallocs @@ -752,12 +740,25 @@ func (s *sysMemStat) add(n int64) { // that need to be updated together in order for them to be kept // consistent with one another. type heapStatsDelta struct { + // Memory stats. 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 + + // Allocator stats. + largeAlloc uintptr // bytes allocated for large objects + largeAllocCount uintptr // number of large object allocations + smallAllocCount [_NumSizeClasses]uintptr // number of allocs for small objects + largeFree uintptr // bytes freed for large objects (>maxSmallSize) + largeFreeCount uintptr // number of frees for large objects (>maxSmallSize) + smallFreeCount [_NumSizeClasses]uintptr // number of frees for small objects (<=maxSmallSize) + + // Add a uint32 to ensure this struct is a multiple of 8 bytes in size. + // Only necessary on 32-bit platforms. + // _ [(sys.PtrSize / 4) % 2]uint32 } // merge adds in the deltas from b into a. @@ -768,6 +769,17 @@ func (a *heapStatsDelta) merge(b *heapStatsDelta) { a.inStacks += b.inStacks a.inWorkBufs += b.inWorkBufs a.inPtrScalarBits += b.inPtrScalarBits + + a.largeAlloc += b.largeAlloc + a.largeAllocCount += b.largeAllocCount + for i := range b.smallAllocCount { + a.smallAllocCount[i] += b.smallAllocCount[i] + } + a.largeFree += b.largeFree + a.largeFreeCount += b.largeFreeCount + for i := range b.smallFreeCount { + a.smallFreeCount[i] += b.smallFreeCount[i] + } } // consistentHeapStats represents a set of various memory statistics diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 4f4cff38aa..ebecc92745 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, allp[0].mcache) + freemcache(pp.mcache) pp.mcache = nil gfpurge(pp) traceProcFree(pp) -- cgit v1.3 From b08dfbaa439e4e396b979e02ea2e7d36972e8b7a Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Wed, 1 Jul 2020 16:02:42 +0000 Subject: runtime,runtime/metrics: add memory metrics This change adds support for a variety of runtime memory metrics and contains the base implementation of Read for the runtime/metrics package, which lives in the runtime. It also adds testing infrastructure for the metrics package, and a bunch of format and documentation tests. For #37112. Change-Id: I16a2c4781eeeb2de0abcb045c15105f1210e2d8a Reviewed-on: https://go-review.googlesource.com/c/go/+/247041 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Reviewed-by: Michael Pratt Trust: Michael Knyszek --- src/cmd/go/internal/work/gc.go | 6 +- src/runtime/export_test.go | 26 +++ src/runtime/metrics.go | 367 ++++++++++++++++++++++++++++++++ src/runtime/metrics/description.go | 80 ++++++- src/runtime/metrics/description_test.go | 125 +++++++++++ src/runtime/metrics/doc.go | 56 ++++- src/runtime/metrics/sample.go | 10 +- src/runtime/metrics_test.go | 114 ++++++++++ src/runtime/mstats.go | 3 +- 9 files changed, 781 insertions(+), 6 deletions(-) create mode 100644 src/runtime/metrics.go create mode 100644 src/runtime/metrics/description_test.go create mode 100644 src/runtime/metrics_test.go (limited to 'src/runtime/mstats.go') diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go index e93031431c..0c4a7fa6e3 100644 --- a/src/cmd/go/internal/work/gc.go +++ b/src/cmd/go/internal/work/gc.go @@ -89,7 +89,11 @@ func (gcToolchain) gc(b *Builder, a *Action, archive string, importcfg []byte, s extFiles := len(p.CgoFiles) + len(p.CFiles) + len(p.CXXFiles) + len(p.MFiles) + len(p.FFiles) + len(p.SFiles) + len(p.SysoFiles) + len(p.SwigFiles) + len(p.SwigCXXFiles) if p.Standard { switch p.ImportPath { - case "bytes", "internal/poll", "net", "os", "runtime/pprof", "runtime/trace", "sync", "syscall", "time": + case "bytes", "internal/poll", "net", "os": + fallthrough + case "runtime/metrics", "runtime/pprof", "runtime/trace": + fallthrough + case "sync", "syscall", "time": extFiles++ } } diff --git a/src/runtime/export_test.go b/src/runtime/export_test.go index ff901fd7be..d043fe3ee5 100644 --- a/src/runtime/export_test.go +++ b/src/runtime/export_test.go @@ -298,6 +298,32 @@ func (p *ProfBuf) Close() { (*profBuf)(p).close() } +func ReadMetricsSlow(memStats *MemStats, samplesp unsafe.Pointer, len, cap int) { + stopTheWorld("ReadMetricsSlow") + + // Initialize the metrics beforehand because this could + // allocate and skew the stats. + semacquire(&metricsSema) + initMetrics() + semrelease(&metricsSema) + + systemstack(func() { + // Read memstats first. It's going to flush + // the mcaches which readMetrics does not do, so + // going the other way around may result in + // inconsistent statistics. + readmemstats_m(memStats) + }) + + // Read metrics off the system stack. + // + // The only part of readMetrics that could allocate + // and skew the stats is initMetrics. + readMetrics(samplesp, len, cap) + + startTheWorld() +} + // ReadMemStatsSlow returns both the runtime-computed MemStats and // MemStats accumulated by scanning the heap. func ReadMemStatsSlow() (base, slow MemStats) { diff --git a/src/runtime/metrics.go b/src/runtime/metrics.go new file mode 100644 index 0000000000..44b5a29751 --- /dev/null +++ b/src/runtime/metrics.go @@ -0,0 +1,367 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package runtime + +// Metrics implementation exported to runtime/metrics. + +import ( + "unsafe" +) + +var ( + // metrics is a map of runtime/metrics keys to + // data used by the runtime to sample each metric's + // value. + metricsSema uint32 = 1 + metricsInit bool + metrics map[string]metricData +) + +type metricData struct { + // deps is the set of runtime statistics that this metric + // depends on. Before compute is called, the statAggregate + // which will be passed must ensure() these dependencies. + deps statDepSet + + // compute is a function that populates a metricValue + // given a populated statAggregate structure. + compute func(in *statAggregate, out *metricValue) +} + +// initMetrics initializes the metrics map if it hasn't been yet. +// +// metricsSema must be held. +func initMetrics() { + if metricsInit { + return + } + metrics = map[string]metricData{ + "/memory/classes/heap/free:bytes": { + deps: makeStatDepSet(heapStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = uint64(in.heapStats.committed - in.heapStats.inHeap - + in.heapStats.inStacks - in.heapStats.inWorkBufs - + in.heapStats.inPtrScalarBits) + }, + }, + "/memory/classes/heap/objects:bytes": { + deps: makeStatDepSet(heapStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.heapStats.inObjects + }, + }, + "/memory/classes/heap/released:bytes": { + deps: makeStatDepSet(heapStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = uint64(in.heapStats.released) + }, + }, + "/memory/classes/heap/stacks:bytes": { + deps: makeStatDepSet(heapStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = uint64(in.heapStats.inStacks) + }, + }, + "/memory/classes/heap/unused:bytes": { + deps: makeStatDepSet(heapStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = uint64(in.heapStats.inHeap) - in.heapStats.inObjects + }, + }, + "/memory/classes/metadata/mcache/free:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.mCacheSys - in.sysStats.mCacheInUse + }, + }, + "/memory/classes/metadata/mcache/inuse:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.mCacheInUse + }, + }, + "/memory/classes/metadata/mspan/free:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.mSpanSys - in.sysStats.mSpanInUse + }, + }, + "/memory/classes/metadata/mspan/inuse:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.mSpanInUse + }, + }, + "/memory/classes/metadata/other:bytes": { + deps: makeStatDepSet(heapStatsDep, sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = uint64(in.heapStats.inWorkBufs+in.heapStats.inPtrScalarBits) + in.sysStats.gcMiscSys + }, + }, + "/memory/classes/os-stacks:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.stacksSys + }, + }, + "/memory/classes/other:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.otherSys + }, + }, + "/memory/classes/profiling/buckets:bytes": { + deps: makeStatDepSet(sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = in.sysStats.buckHashSys + }, + }, + "/memory/classes/total:bytes": { + deps: makeStatDepSet(heapStatsDep, sysStatsDep), + compute: func(in *statAggregate, out *metricValue) { + out.kind = metricKindUint64 + out.scalar = uint64(in.heapStats.committed+in.heapStats.released) + + in.sysStats.stacksSys + in.sysStats.mSpanSys + + in.sysStats.mCacheSys + in.sysStats.buckHashSys + + in.sysStats.gcMiscSys + in.sysStats.otherSys + }, + }, + } + metricsInit = true +} + +// statDep is a dependency on a group of statistics +// that a metric might have. +type statDep uint + +const ( + heapStatsDep statDep = iota // corresponds to heapStatsAggregate + sysStatsDep // corresponds to sysStatsAggregate + numStatsDeps +) + +// statDepSet represents a set of statDeps. +// +// Under the hood, it's a bitmap. +type statDepSet [1]uint64 + +// makeStatDepSet creates a new statDepSet from a list of statDeps. +func makeStatDepSet(deps ...statDep) statDepSet { + var s statDepSet + for _, d := range deps { + s[d/64] |= 1 << (d % 64) + } + return s +} + +// differennce returns set difference of s from b as a new set. +func (s statDepSet) difference(b statDepSet) statDepSet { + var c statDepSet + for i := range s { + c[i] = s[i] &^ b[i] + } + return c +} + +// union returns the union of the two sets as a new set. +func (s statDepSet) union(b statDepSet) statDepSet { + var c statDepSet + for i := range s { + c[i] = s[i] | b[i] + } + return c +} + +// empty returns true if there are no dependencies in the set. +func (s *statDepSet) empty() bool { + for _, c := range s { + if c != 0 { + return false + } + } + return true +} + +// has returns true if the set contains a given statDep. +func (s *statDepSet) has(d statDep) bool { + return s[d/64]&(1<<(d%64)) != 0 +} + +// heapStatsAggregate represents memory stats obtained from the +// runtime. This set of stats is grouped together because they +// depend on each other in some way to make sense of the runtime's +// current heap memory use. They're also sharded across Ps, so it +// makes sense to grab them all at once. +type heapStatsAggregate struct { + heapStatsDelta + + // inObjects is the bytes of memory occupied by objects, + // derived from other values in heapStats. + inObjects uint64 +} + +// compute populates the heapStatsAggregate with values from the runtime. +func (a *heapStatsAggregate) compute() { + memstats.heapStats.read(&a.heapStatsDelta) + + // Calculate derived stats. + a.inObjects = uint64(a.largeAlloc - a.largeFree) + for i := range a.smallAllocCount { + a.inObjects += uint64(a.smallAllocCount[i]-a.smallFreeCount[i]) * uint64(class_to_size[i]) + } +} + +// sysStatsAggregate represents system memory stats obtained +// from the runtime. This set of stats is grouped together because +// they're all relatively cheap to acquire and generally independent +// of one another and other runtime memory stats. The fact that they +// may be acquired at different times, especially with respect to +// heapStatsAggregate, means there could be some skew, but because of +// these stats are independent, there's no real consistency issue here. +type sysStatsAggregate struct { + stacksSys uint64 + mSpanSys uint64 + mSpanInUse uint64 + mCacheSys uint64 + mCacheInUse uint64 + buckHashSys uint64 + gcMiscSys uint64 + otherSys uint64 +} + +// compute populates the sysStatsAggregate with values from the runtime. +func (a *sysStatsAggregate) compute() { + a.stacksSys = memstats.stacks_sys.load() + a.buckHashSys = memstats.buckhash_sys.load() + a.gcMiscSys = memstats.gcMiscSys.load() + a.otherSys = memstats.other_sys.load() + + systemstack(func() { + lock(&mheap_.lock) + a.mSpanSys = memstats.mspan_sys.load() + a.mSpanInUse = uint64(mheap_.spanalloc.inuse) + a.mCacheSys = memstats.mcache_sys.load() + a.mCacheInUse = uint64(mheap_.cachealloc.inuse) + unlock(&mheap_.lock) + }) +} + +// statAggregate is the main driver of the metrics implementation. +// +// It contains multiple aggregates of runtime statistics, as well +// as a set of these aggregates that it has populated. The aggergates +// are populated lazily by its ensure method. +type statAggregate struct { + ensured statDepSet + heapStats heapStatsAggregate + sysStats sysStatsAggregate +} + +// ensure populates statistics aggregates determined by deps if they +// haven't yet been populated. +func (a *statAggregate) ensure(deps *statDepSet) { + missing := deps.difference(a.ensured) + if missing.empty() { + return + } + for i := statDep(0); i < numStatsDeps; i++ { + if !missing.has(i) { + continue + } + switch i { + case heapStatsDep: + a.heapStats.compute() + case sysStatsDep: + a.sysStats.compute() + } + } + a.ensured = a.ensured.union(missing) +} + +// metricValidKind is a runtime copy of runtime/metrics.ValueKind and +// must be kept structurally identical to that type. +type metricKind int + +const ( + // These values must be kept identical to their corresponding Kind* values + // in the runtime/metrics package. + metricKindBad metricKind = iota + metricKindUint64 + metricKindFloat64 + metricKindFloat64Histogram +) + +// metricSample is a runtime copy of runtime/metrics.Sample and +// must be kept structurally identical to that type. +type metricSample struct { + name string + value metricValue +} + +// metricValue is a runtime copy of runtime/metrics.Sample and +// must be kept structurally identical to that type. +type metricValue struct { + kind metricKind + scalar uint64 // contains scalar values for scalar Kinds. + pointer unsafe.Pointer // contains non-scalar values. +} + +// agg is used by readMetrics, and is protected by metricsSema. +// +// Managed as a global variable because its pointer will be +// an argument to a dynamically-defined function, and we'd +// like to avoid it escaping to the heap. +var agg statAggregate + +// readMetrics is the implementation of runtime/metrics.Read. +// +//go:linkname readMetrics runtime/metrics.runtime_readMetrics +func readMetrics(samplesp unsafe.Pointer, len int, cap int) { + // Construct a slice from the args. + sl := slice{samplesp, len, cap} + samples := *(*[]metricSample)(unsafe.Pointer(&sl)) + + // Acquire the metricsSema but with handoff. This operation + // is expensive enough that queueing up goroutines and handing + // off between them will be noticably better-behaved. + semacquire1(&metricsSema, true, 0, 0) + + // Ensure the map is initialized. + initMetrics() + + // Clear agg defensively. + agg = statAggregate{} + + // Sample. + for i := range samples { + sample := &samples[i] + data, ok := metrics[sample.name] + if !ok { + sample.value.kind = metricKindBad + continue + } + // Ensure we have all the stats we need. + // agg is populated lazily. + agg.ensure(&data.deps) + + // Compute the value based on the stats we have. + data.compute(&agg, &sample.value) + } + + semrelease(&metricsSema) +} diff --git a/src/runtime/metrics/description.go b/src/runtime/metrics/description.go index 32bb950a72..2e7df7e09f 100644 --- a/src/runtime/metrics/description.go +++ b/src/runtime/metrics/description.go @@ -10,7 +10,7 @@ type Description struct { // // The format of the metric may be described by the following regular expression. // - // ^(?P/[^:]+):(?P[^:*\/]+(?:[*\/][^:*\/]+)*)$ + // ^(?P/[^:]+):(?P[^:*/]+(?:[*/][^:*/]+)*)$ // // The format splits the name into two components, separated by a colon: a path which always // starts with a /, and a machine-parseable unit. The name may contain any valid Unicode @@ -26,6 +26,9 @@ type Description struct { // A complete name might look like "/memory/heap/free:bytes". Name string + // Description is an English language sentence describing the metric. + Description string + // Kind is the kind of value for this metric. // // The purpose of this field is to allow users to filter out metrics whose values are @@ -44,7 +47,80 @@ type Description struct { StopTheWorld bool } -var allDesc = []Description{} +// The English language descriptions below must be kept in sync with the +// descriptions of each metric in doc.go. +var allDesc = []Description{ + { + Name: "/memory/classes/heap/free:bytes", + Description: "Memory that is available for allocation, and may be returned to the underlying system.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/heap/objects:bytes", + Description: "Memory occupied by live objects and dead objects that have not yet been collected.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/heap/released:bytes", + Description: "Memory that has been returned to the underlying system.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/heap/stacks:bytes", + Description: "Memory allocated from the heap that is occupied by stacks.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/heap/unused:bytes", + Description: "Memory that is unavailable for allocation, but cannot be returned to the underlying system.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/metadata/mcache/free:bytes", + Description: "Memory that is reserved for runtime mcache structures, but not in-use.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/metadata/mcache/inuse:bytes", + Description: "Memory that is occupied by runtime mcache structures that are currently being used.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/metadata/mspan/free:bytes", + Description: "Memory that is reserved for runtime mspan structures, but not in-use.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/metadata/mspan/inuse:bytes", + Description: "Memory that is occupied by runtime mspan structures that are currently being used.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/metadata/other:bytes", + Description: "Memory that is reserved for or used to hold runtime metadata.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/os-stacks:bytes", + Description: "Stack memory allocated by the underlying operating system.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/other:bytes", + Description: "Memory used by execution trace buffers, structures for debugging the runtime, finalizer and profiler specials, and more.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/profiling/buckets:bytes", + Description: "Memory that is used by the stack trace hash map used for profiling.", + Kind: KindUint64, + }, + { + Name: "/memory/classes/total:bytes", + Description: "All memory mapped by the Go runtime into the current process as read-write. Note that this does not include memory mapped by code called via cgo or via the syscall package. Sum of all metrics in /memory/classes.", + Kind: KindUint64, + }, +} // All returns a slice of containing metric descriptions for all supported metrics. func All() []Description { diff --git a/src/runtime/metrics/description_test.go b/src/runtime/metrics/description_test.go new file mode 100644 index 0000000000..e966a281a1 --- /dev/null +++ b/src/runtime/metrics/description_test.go @@ -0,0 +1,125 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package metrics_test + +import ( + "bufio" + "os" + "path/filepath" + "regexp" + "runtime" + "runtime/metrics" + "strings" + "testing" +) + +func TestDescriptionNameFormat(t *testing.T) { + r := regexp.MustCompile("^(?P/[^:]+):(?P[^:*/]+(?:[*/][^:*/]+)*)$") + descriptions := metrics.All() + for _, desc := range descriptions { + if !r.MatchString(desc.Name) { + t.Errorf("metrics %q does not match regexp %s", desc.Name, r) + } + } +} + +func extractMetricDocs(t *testing.T) map[string]string { + if runtime.GOOS == "android" { + t.Skip("no access to Go source on android") + } + + // Get doc.go. + _, filename, _, _ := runtime.Caller(0) + filename = filepath.Join(filepath.Dir(filename), "doc.go") + + f, err := os.Open(filename) + if err != nil { + t.Fatal(err) + } + const ( + stateSearch = iota // look for list of metrics + stateNextMetric // look for next metric + stateNextDescription // build description + ) + state := stateSearch + s := bufio.NewScanner(f) + result := make(map[string]string) + var metric string + var prevMetric string + var desc strings.Builder + for s.Scan() { + line := strings.TrimSpace(s.Text()) + switch state { + case stateSearch: + if line == "Supported metrics" { + state = stateNextMetric + } + case stateNextMetric: + // Ignore empty lines until we find a non-empty + // one. This will be our metric name. + if len(line) != 0 { + prevMetric = metric + metric = line + if prevMetric > metric { + t.Errorf("metrics %s and %s are out of lexicographical order", prevMetric, metric) + } + state = stateNextDescription + } + case stateNextDescription: + if len(line) == 0 || line == `*/` { + // An empty line means we're done. + // Write down the description and look + // for a new metric. + result[metric] = desc.String() + desc.Reset() + state = stateNextMetric + } else { + // As long as we're seeing data, assume that's + // part of the description and append it. + if desc.Len() != 0 { + // Turn previous newlines into spaces. + desc.WriteString(" ") + } + desc.WriteString(line) + } + } + if line == `*/` { + break + } + } + if state == stateSearch { + t.Fatalf("failed to find supported metrics docs in %s", filename) + } + return result +} + +func TestDescriptionDocs(t *testing.T) { + docs := extractMetricDocs(t) + descriptions := metrics.All() + for _, d := range descriptions { + want := d.Description + got, ok := docs[d.Name] + if !ok { + t.Errorf("no docs found for metric %s", d.Name) + continue + } + if got != want { + t.Errorf("mismatched description and docs for metric %s", d.Name) + t.Errorf("want: %q, got %q", want, got) + continue + } + } + if len(docs) > len(descriptions) { + docsLoop: + for name, _ := range docs { + for _, d := range descriptions { + if name == d.Name { + continue docsLoop + } + } + t.Errorf("stale documentation for non-existent metric: %s", name) + } + } +} diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index b48c22ba30..fb4e23a2b5 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -44,6 +44,60 @@ the documentation of the Name field of the Description struct. Supported metrics -TODO(mknyszek): List them here as they're added. + /memory/classes/heap/free:bytes + Memory that is available for allocation, and may be returned + to the underlying system. + + /memory/classes/heap/objects:bytes + Memory occupied by live objects and dead objects that have + not yet been collected. + + /memory/classes/heap/released:bytes + Memory that has been returned to the underlying system. + + /memory/classes/heap/stacks:bytes + Memory allocated from the heap that is occupied by stacks. + + /memory/classes/heap/unused:bytes + Memory that is unavailable for allocation, but cannot be + returned to the underlying system. + + /memory/classes/metadata/mcache/free:bytes + Memory that is reserved for runtime mcache structures, but + not in-use. + + /memory/classes/metadata/mcache/inuse:bytes + Memory that is occupied by runtime mcache structures that + are currently being used. + + /memory/classes/metadata/mspan/free:bytes + Memory that is reserved for runtime mspan structures, but + not in-use. + + /memory/classes/metadata/mspan/inuse:bytes + Memory that is occupied by runtime mspan structures that are + currently being used. + + /memory/classes/metadata/other:bytes + Memory that is reserved for or used to hold runtime + metadata. + + /memory/classes/os-stacks:bytes + Stack memory allocated by the underlying operating system. + + /memory/classes/other:bytes + Memory used by execution trace buffers, structures for + debugging the runtime, finalizer and profiler specials, and + more. + + /memory/classes/profiling/buckets:bytes + Memory that is used by the stack trace hash map used for + profiling. + + /memory/classes/total:bytes + All memory mapped by the Go runtime into the current process + as read-write. Note that this does not include memory mapped + by code called via cgo or via the syscall package. + Sum of all metrics in /memory/classes. */ package metrics diff --git a/src/runtime/metrics/sample.go b/src/runtime/metrics/sample.go index c7a3fc424a..b4b0979aa6 100644 --- a/src/runtime/metrics/sample.go +++ b/src/runtime/metrics/sample.go @@ -4,6 +4,11 @@ package metrics +import ( + _ "runtime" // depends on the runtime via a linkname'd function + "unsafe" +) + // Sample captures a single metric sample. type Sample struct { // Name is the name of the metric sampled. @@ -16,6 +21,9 @@ type Sample struct { Value Value } +// Implemented in the runtime. +func runtime_readMetrics(unsafe.Pointer, int, int) + // Read populates each Value field in the given slice of metric samples. // // Desired metrics should be present in the slice with the appropriate name. @@ -25,5 +33,5 @@ type Sample struct { // will have the value populated as KindBad to indicate that the name is // unknown. func Read(m []Sample) { - panic("unimplemented") + runtime_readMetrics(unsafe.Pointer(&m[0]), len(m), cap(m)) } diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go new file mode 100644 index 0000000000..f00aad07c4 --- /dev/null +++ b/src/runtime/metrics_test.go @@ -0,0 +1,114 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package runtime_test + +import ( + "runtime" + "runtime/metrics" + "strings" + "testing" + "unsafe" +) + +func prepareAllMetricsSamples() (map[string]metrics.Description, []metrics.Sample) { + all := metrics.All() + samples := make([]metrics.Sample, len(all)) + descs := make(map[string]metrics.Description) + for i := range all { + samples[i].Name = all[i].Name + descs[all[i].Name] = all[i] + } + return descs, samples +} + +func TestReadMetrics(t *testing.T) { + // Tests whether readMetrics produces values aligning + // with ReadMemStats while the world is stopped. + var mstats runtime.MemStats + _, samples := prepareAllMetricsSamples() + runtime.ReadMetricsSlow(&mstats, unsafe.Pointer(&samples[0]), len(samples), cap(samples)) + + checkUint64 := func(t *testing.T, m string, got, want uint64) { + t.Helper() + if got != want { + t.Errorf("metric %q: got %d, want %d", m, got, want) + } + } + + // Check to make sure the values we read line up with other values we read. + for i := range samples { + switch name := samples[i].Name; name { + case "/memory/classes/heap/free:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.HeapIdle-mstats.HeapReleased) + case "/memory/classes/heap/released:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.HeapReleased) + case "/memory/classes/heap/objects:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.HeapAlloc) + case "/memory/classes/heap/unused:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.HeapInuse-mstats.HeapAlloc) + case "/memory/classes/heap/stacks:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.StackInuse) + case "/memory/classes/metadata/mcache/free:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.MCacheSys-mstats.MCacheInuse) + case "/memory/classes/metadata/mcache/inuse:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.MCacheInuse) + case "/memory/classes/metadata/mspan/free:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.MSpanSys-mstats.MSpanInuse) + case "/memory/classes/metadata/mspan/inuse:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.MSpanInuse) + case "/memory/classes/metadata/other:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.GCSys) + case "/memory/classes/os-stacks:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.StackSys-mstats.StackInuse) + case "/memory/classes/other:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.OtherSys) + case "/memory/classes/profiling/buckets:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.BuckHashSys) + case "/memory/classes/total:bytes": + checkUint64(t, name, samples[i].Value.Uint64(), mstats.Sys) + } + } +} + +func TestReadMetricsConsistency(t *testing.T) { + // Tests whether readMetrics produces consistent, sensible values. + // The values are read concurrently with the runtime doing other + // things (e.g. allocating) so what we read can't reasonably compared + // to runtime values. + + // Read all the supported metrics through the metrics package. + descs, samples := prepareAllMetricsSamples() + metrics.Read(samples) + + // Check to make sure the values we read make sense. + var totalVirtual struct { + got, want uint64 + } + for i := range samples { + kind := samples[i].Value.Kind() + if want := descs[samples[i].Name].Kind; kind != want { + t.Errorf("supported metric %q has unexpected kind: got %d, want %d", samples[i].Name, kind, want) + continue + } + if samples[i].Name != "/memory/classes/total:bytes" && strings.HasPrefix(samples[i].Name, "/memory/classes") { + v := samples[i].Value.Uint64() + totalVirtual.want += v + + // None of these stats should ever get this big. + // If they do, there's probably overflow involved, + // usually due to bad accounting. + if int64(v) < 0 { + t.Errorf("%q has high/negative value: %d", samples[i].Name, v) + } + } + switch samples[i].Name { + case "/memory/classes/total:bytes": + totalVirtual.got = samples[i].Value.Uint64() + } + } + if totalVirtual.got != totalVirtual.want { + t.Errorf(`"/memory/classes/total:bytes" does not match sum of /memory/classes/**: got %d, want %d`, totalVirtual.got, totalVirtual.want) + } +} diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index a8eca85fe6..512a06cffa 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -882,7 +882,8 @@ func (m *consistentHeapStats) unsafeClear() { // heapStatsDelta, the resulting values should be complete and // valid statistic values. // -// Not safe to call concurrently. +// Not safe to call concurrently. The world must be stopped +// or metricsSema must be held. 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 -- cgit v1.3 From 22d2b984a680900ebbec6268f93a839286b6f130 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 26 Oct 2020 19:35:23 +0000 Subject: runtime: make sysMemStats' methods nosplit sysMemStats are updated early on in runtime initialization, so triggering a stack growth would be bad. Mark them nosplit. Thank you so much to cherryyz@google.com for finding this fix! Fixes #42218. Change-Id: Ic62db76e6a4f829355d7eaabed1727c51adfbd0f Reviewed-on: https://go-review.googlesource.com/c/go/+/265157 Trust: Michael Knyszek Run-TryBot: Michael Knyszek Reviewed-by: Michael Pratt Reviewed-by: Cherry Zhang Reviewed-by: Austin Clements TryBot-Result: Go Bot --- src/runtime/mstats.go | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 512a06cffa..07f466ec49 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -720,11 +720,17 @@ func flushallmcaches() { type sysMemStat uint64 // load atomically reads the value of the stat. +// +// Must be nosplit as it is called in runtime initialization, e.g. newosproc0. +//go:nosplit func (s *sysMemStat) load() uint64 { return atomic.Load64((*uint64)(s)) } // add atomically adds the sysMemStat by n. +// +// Must be nosplit as it is called in runtime initialization, e.g. newosproc0. +//go:nosplit func (s *sysMemStat) add(n int64) { if s == nil { return -- cgit v1.3 From d39a89fd5843f535d634620d27110b320431f584 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Thu, 6 Aug 2020 21:59:13 +0000 Subject: runtime,runtime/metrics: add metric for distribution of GC pauses For #37112. Change-Id: Ibb0425c9c582ae3da3b2662d5bbe830d7df9079c Reviewed-on: https://go-review.googlesource.com/c/go/+/247047 Run-TryBot: Michael Knyszek TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt --- src/runtime/metrics.go | 9 +++++++++ src/runtime/metrics/description.go | 5 +++++ src/runtime/metrics/doc.go | 3 +++ src/runtime/metrics_test.go | 22 ++++++++++++++++++++++ src/runtime/mgc.go | 3 +++ src/runtime/mstats.go | 12 ++++++++++++ 6 files changed, 54 insertions(+) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/metrics.go b/src/runtime/metrics.go index 2be38ccaaa..0e391472b2 100644 --- a/src/runtime/metrics.go +++ b/src/runtime/metrics.go @@ -102,6 +102,15 @@ func initMetrics() { out.scalar = in.heapStats.numObjects }, }, + "/gc/pauses:seconds": { + compute: func(_ *statAggregate, out *metricValue) { + hist := out.float64HistOrInit(timeHistBuckets) + hist.counts[len(hist.counts)-1] = atomic.Load64(&memstats.gcPauseDist.overflow) + for i := range hist.buckets { + hist.counts[i] = atomic.Load64(&memstats.gcPauseDist.counts[i]) + } + }, + }, "/memory/classes/heap/free:bytes": { deps: makeStatDepSet(heapStatsDep), compute: func(in *statAggregate, out *metricValue) { diff --git a/src/runtime/metrics/description.go b/src/runtime/metrics/description.go index e43904fc7d..47959e467c 100644 --- a/src/runtime/metrics/description.go +++ b/src/runtime/metrics/description.go @@ -88,6 +88,11 @@ var allDesc = []Description{ Description: "Number of objects, live or unswept, occupying heap memory.", Kind: KindUint64, }, + { + Name: "/gc/pauses:seconds", + Description: "Distribution individual GC-related stop-the-world pause latencies.", + Kind: KindFloat64Histogram, + }, { Name: "/memory/classes/heap/free:bytes", Description: "Memory that is available for allocation, and may be returned to the underlying system.", diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index 5045a5b4c1..1e12ade5a1 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -65,6 +65,9 @@ Supported metrics /gc/heap/objects:objects Number of objects, live or unswept, occupying heap memory. + /gc/pauses:seconds + Distribution individual GC-related stop-the-world pause latencies. + /memory/classes/heap/free:bytes Memory that is available for allocation, and may be returned to the underlying system. diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go index 1a30810544..7b3132bc30 100644 --- a/src/runtime/metrics_test.go +++ b/src/runtime/metrics_test.go @@ -90,6 +90,11 @@ func TestReadMetricsConsistency(t *testing.T) { // things (e.g. allocating) so what we read can't reasonably compared // to runtime values. + // Run a few GC cycles to get some of the stats to be non-zero. + runtime.GC() + runtime.GC() + runtime.GC() + // Read all the supported metrics through the metrics package. descs, samples := prepareAllMetricsSamples() metrics.Read(samples) @@ -102,6 +107,10 @@ func TestReadMetricsConsistency(t *testing.T) { alloc, free *metrics.Float64Histogram total uint64 } + var gc struct { + numGC uint64 + pauses uint64 + } for i := range samples { kind := samples[i].Value.Kind() if want := descs[samples[i].Name].Kind; kind != want { @@ -128,6 +137,14 @@ func TestReadMetricsConsistency(t *testing.T) { objects.alloc = samples[i].Value.Float64Histogram() case "/gc/heap/frees-by-size:objects": objects.free = samples[i].Value.Float64Histogram() + case "/gc/cycles:gc-cycles": + gc.numGC = samples[i].Value.Uint64() + case "/gc/pauses:seconds": + h := samples[i].Value.Float64Histogram() + gc.pauses = 0 + for i := range h.Counts { + gc.pauses += h.Counts[i] + } } } if totalVirtual.got != totalVirtual.want { @@ -159,6 +176,11 @@ func TestReadMetricsConsistency(t *testing.T) { } } } + // The current GC has at least 2 pauses per GC. + // Check to see if that value makes sense. + if gc.pauses < gc.numGC*2 { + t.Errorf("fewer pauses than expected: got %d, want at least %d", gc.pauses, gc.numGC*2) + } } func BenchmarkReadMetricsLatency(b *testing.B) { diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 540c376f1c..b0ab0ae6bb 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1418,6 +1418,7 @@ func gcStart(trigger gcTrigger) { now = startTheWorldWithSema(trace.enabled) work.pauseNS += now - work.pauseStart work.tMark = now + memstats.gcPauseDist.record(now - work.pauseStart) }) // Release the world sema before Gosched() in STW mode @@ -1565,6 +1566,7 @@ top: systemstack(func() { now := startTheWorldWithSema(true) work.pauseNS += now - work.pauseStart + memstats.gcPauseDist.record(now - work.pauseStart) }) semrelease(&worldsema) goto top @@ -1677,6 +1679,7 @@ func gcMarkTermination(nextTriggerRatio float64) { unixNow := sec*1e9 + int64(nsec) work.pauseNS += now - work.pauseStart work.tEnd = now + memstats.gcPauseDist.record(now - work.pauseStart) atomic.Store64(&memstats.last_gc_unix, uint64(unixNow)) // must be Unix time to make sense to user atomic.Store64(&memstats.last_gc_nanotime, uint64(now)) // monotonic time for us memstats.pause_ns[memstats.numgc%uint32(len(memstats.pause_ns))] = uint64(work.pauseNS) diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index 07f466ec49..e0a417d213 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -157,6 +157,14 @@ type mstats struct { // heapStats is a set of statistics heapStats consistentHeapStats + + _ uint32 // ensure gcPauseDist is aligned + + // gcPauseDist represents the distribution of all GC-related + // application pauses in the runtime. + // + // Each individual pause is counted separately, unlike pause_ns. + gcPauseDist timeHistogram } var memstats mstats @@ -443,6 +451,10 @@ func init() { println(offset) throw("memstats.heapStats not aligned to 8 bytes") } + if offset := unsafe.Offsetof(memstats.gcPauseDist); offset%8 != 0 { + println(offset) + throw("memstats.gcPauseDist 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 { -- cgit v1.3 From 6abbfc17c255c07134a69c3ca305231db80530ec Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Wed, 28 Oct 2020 18:06:05 -0400 Subject: runtime: add world-stopped assertions Stopping the world is an implicit lock for many operations, so we should assert the world is stopped in functions that require it. This is enabled along with the rest of lock ranking, though it is a bit orthogonal and likely cheap enough to enable all the time should we choose. Requiring a lock _or_ world stop is common, so that can be expressed as well. Updates #40677 Change-Id: If0a58544f4251d367f73c4120c9d39974c6cd091 Reviewed-on: https://go-review.googlesource.com/c/go/+/248577 Run-TryBot: Michael Pratt TryBot-Result: Go Bot Reviewed-by: Austin Clements Trust: Michael Pratt --- src/runtime/heapdump.go | 15 +++++++++ src/runtime/lockrank_off.go | 16 +++++++++ src/runtime/lockrank_on.go | 79 +++++++++++++++++++++++++++++++++++++++++++++ src/runtime/mcheckmark.go | 2 ++ src/runtime/mgc.go | 2 ++ src/runtime/mgcmark.go | 4 +++ src/runtime/mgcsweep.go | 2 ++ src/runtime/mstats.go | 14 ++++++-- src/runtime/proc.go | 14 ++++++++ 9 files changed, 145 insertions(+), 3 deletions(-) (limited to 'src/runtime/mstats.go') diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go index 33e224d587..2d531571aa 100644 --- a/src/runtime/heapdump.go +++ b/src/runtime/heapdump.go @@ -431,6 +431,9 @@ func finq_callback(fn *funcval, obj unsafe.Pointer, nret uintptr, fint *_type, o } func dumproots() { + // To protect mheap_.allspans. + assertWorldStopped() + // TODO(mwhudson): dump datamask etc from all objects // data segment dumpint(tagData) @@ -468,6 +471,9 @@ func dumproots() { var freemark [_PageSize / 8]bool func dumpobjs() { + // To protect mheap_.allspans. + assertWorldStopped() + for _, s := range mheap_.allspans { if s.state.get() != mSpanInUse { continue @@ -552,6 +558,8 @@ func dumpms() { //go:systemstack func dumpmemstats(m *MemStats) { + assertWorldStopped() + // These ints should be identical to the exported // MemStats structure and should be ordered the same // way too. @@ -634,6 +642,9 @@ func dumpmemprof_callback(b *bucket, nstk uintptr, pstk *uintptr, size, allocs, } func dumpmemprof() { + // To protect mheap_.allspans. + assertWorldStopped() + iterate_memprof(dumpmemprof_callback) for _, s := range mheap_.allspans { if s.state.get() != mSpanInUse { @@ -655,6 +666,8 @@ func dumpmemprof() { var dumphdr = []byte("go1.7 heap dump\n") func mdump(m *MemStats) { + assertWorldStopped() + // make sure we're done sweeping for _, s := range mheap_.allspans { if s.state.get() == mSpanInUse { @@ -676,6 +689,8 @@ func mdump(m *MemStats) { } func writeheapdump_m(fd uintptr, m *MemStats) { + assertWorldStopped() + _g_ := getg() casgstatus(_g_.m.curg, _Grunning, _Gwaiting) _g_.waitreason = waitReasonDumpingHeap diff --git a/src/runtime/lockrank_off.go b/src/runtime/lockrank_off.go index 40edf882ee..7dcd8f5fe9 100644 --- a/src/runtime/lockrank_off.go +++ b/src/runtime/lockrank_off.go @@ -46,3 +46,19 @@ func assertLockHeld(l *mutex) { //go:nosplit func assertRankHeld(r lockRank) { } + +//go:nosplit +func worldStopped() { +} + +//go:nosplit +func worldStarted() { +} + +//go:nosplit +func assertWorldStopped() { +} + +//go:nosplit +func assertWorldStoppedOrLockHeld(l *mutex) { +} diff --git a/src/runtime/lockrank_on.go b/src/runtime/lockrank_on.go index db7ff23a58..c25b3a4656 100644 --- a/src/runtime/lockrank_on.go +++ b/src/runtime/lockrank_on.go @@ -7,9 +7,14 @@ package runtime import ( + "runtime/internal/atomic" "unsafe" ) +// worldIsStopped is accessed atomically to track world-stops. 1 == world +// stopped. +var worldIsStopped uint32 + // lockRankStruct is embedded in mutex type lockRankStruct struct { // static lock ranking of the lock @@ -284,3 +289,77 @@ func assertRankHeld(r lockRank) { throw("not holding required lock!") }) } + +// worldStopped notes that the world is stopped. +// +// Caller must hold worldsema. +// +// nosplit to ensure it can be called in as many contexts as possible. +//go:nosplit +func worldStopped() { + if stopped := atomic.Xadd(&worldIsStopped, 1); stopped != 1 { + print("world stop count=", stopped, "\n") + throw("recursive world stop") + } +} + +// worldStarted that the world is starting. +// +// Caller must hold worldsema. +// +// nosplit to ensure it can be called in as many contexts as possible. +//go:nosplit +func worldStarted() { + if stopped := atomic.Xadd(&worldIsStopped, -1); stopped != 0 { + print("world stop count=", stopped, "\n") + throw("released non-stopped world stop") + } +} + +// nosplit to ensure it can be called in as many contexts as possible. +//go:nosplit +func checkWorldStopped() bool { + stopped := atomic.Load(&worldIsStopped) + if stopped > 1 { + print("inconsistent world stop count=", stopped, "\n") + throw("inconsistent world stop count") + } + + return stopped == 1 +} + +// assertWorldStopped throws if the world is not stopped. It does not check +// which M stopped the world. +// +// nosplit to ensure it can be called in as many contexts as possible. +//go:nosplit +func assertWorldStopped() { + if checkWorldStopped() { + return + } + + throw("world not stopped") +} + +// assertWorldStoppedOrLockHeld throws if the world is not stopped and the +// passed lock is not held. +// +// nosplit to ensure it can be called in as many contexts as possible. +//go:nosplit +func assertWorldStoppedOrLockHeld(l *mutex) { + if checkWorldStopped() { + return + } + + gp := getg() + systemstack(func() { + held := checkLockHeld(gp, l) + if !held { + printlock() + print("caller requires world stop or lock ", l, " (rank ", l.rank.String(), "), holding:\n") + println("") + printHeldLocks(gp) + throw("no world stop or required lock!") + } + }) +} diff --git a/src/runtime/mcheckmark.go b/src/runtime/mcheckmark.go index c0b028d715..ba80ac1bdf 100644 --- a/src/runtime/mcheckmark.go +++ b/src/runtime/mcheckmark.go @@ -34,6 +34,8 @@ var useCheckmark = false // // The world must be stopped. func startCheckmarks() { + assertWorldStopped() + // Clear all checkmarks. for _, ai := range mheap_.allArenas { arena := mheap_.arenas[ai.l1()][ai.l2()] diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 9d2682f03c..fb3c149942 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -2164,6 +2164,8 @@ func gcMark(start_time int64) { // //go:systemstack func gcSweep(mode gcMode) { + assertWorldStopped() + if gcphase != _GCoff { throw("gcSweep being done but phase is not GCoff") } diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index c71c0e58d3..5a24cdac88 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -54,6 +54,8 @@ const ( // // The world must be stopped. func gcMarkRootPrepare() { + assertWorldStopped() + work.nFlushCacheRoots = 0 // Compute how many data and BSS root blocks there are. @@ -1535,6 +1537,8 @@ func gcmarknewobject(span *mspan, obj, size, scanSize uintptr) { // // The world must be stopped. func gcMarkTinyAllocs() { + assertWorldStopped() + for _, p := range allp { c := p.mcache if c == nil || c.tiny == 0 { diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go index 9b77ce635c..8391435630 100644 --- a/src/runtime/mgcsweep.go +++ b/src/runtime/mgcsweep.go @@ -123,6 +123,8 @@ func (h *mheap) nextSpanForSweep() *mspan { // //go:nowritebarrier func finishsweep_m() { + assertWorldStopped() + // Sweeping must be complete before marking commences, so // sweep any unswept spans. If this is a concurrent GC, there // shouldn't be any spans left to sweep, so this should finish diff --git a/src/runtime/mstats.go b/src/runtime/mstats.go index e0a417d213..3829355d7b 100644 --- a/src/runtime/mstats.go +++ b/src/runtime/mstats.go @@ -601,6 +601,8 @@ func readGCStats_m(pauses *[]uint64) { // //go:nowritebarrier func updatememstats() { + assertWorldStopped() + // Flush mcaches to mcentral before doing anything else. // // Flushing to the mcentral may in general cause stats to @@ -706,6 +708,8 @@ func updatememstats() { // //go:nowritebarrier func flushmcache(i int) { + assertWorldStopped() + p := allp[i] c := p.mcache if c == nil { @@ -721,6 +725,8 @@ func flushmcache(i int) { // //go:nowritebarrier func flushallmcaches() { + assertWorldStopped() + for i := 0; i < int(gomaxprocs); i++ { flushmcache(i) } @@ -876,10 +882,10 @@ func (m *consistentHeapStats) release(c *mcache) { // 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). +// world must be stopped. func (m *consistentHeapStats) unsafeRead(out *heapStatsDelta) { + assertWorldStopped() + for i := range m.stats { out.merge(&m.stats[i]) } @@ -890,6 +896,8 @@ func (m *consistentHeapStats) unsafeRead(out *heapStatsDelta) { // Unsafe because the world must be stopped and values should // be donated elsewhere before clearing. func (m *consistentHeapStats) unsafeClear() { + assertWorldStopped() + for i := range m.stats { m.stats[i] = heapStatsDelta{} } diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 939757f3a7..82284e6cd6 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -587,6 +587,9 @@ func schedinit() { sched.maxmcount = 10000 + // The world starts stopped. + worldStopped() + moduledataverify() stackinit() mallocinit() @@ -617,6 +620,9 @@ func schedinit() { } unlock(&sched.lock) + // World is effectively started now, as P's can run. + worldStarted() + // For cgocheck > 1, we turn on the write barrier at all times // and check all pointer writes. We can't do this until after // procresize because the write barrier needs a P. @@ -1082,9 +1088,13 @@ func stopTheWorldWithSema() { if bad != "" { throw(bad) } + + worldStopped() } func startTheWorldWithSema(emitTraceEvent bool) int64 { + assertWorldStopped() + mp := acquirem() // disable preemption because it can be holding p in a local var if netpollinited() { list := netpoll(0) // non-blocking @@ -1105,6 +1115,8 @@ func startTheWorldWithSema(emitTraceEvent bool) int64 { } unlock(&sched.lock) + worldStarted() + for p1 != nil { p := p1 p1 = p1.link.ptr() @@ -4539,6 +4551,7 @@ func (pp *p) init(id int32) { // sched.lock must be held and the world must be stopped. func (pp *p) destroy() { assertLockHeld(&sched.lock) + assertWorldStopped() // Move all runnable goroutines to the global queue for pp.runqhead != pp.runqtail { @@ -4629,6 +4642,7 @@ func (pp *p) destroy() { // Returns list of Ps with local work, they need to be scheduled by the caller. func procresize(nprocs int32) *p { assertLockHeld(&sched.lock) + assertWorldStopped() old := gomaxprocs if old < 0 || nprocs <= 0 { -- 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/mstats.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