Skip to content

Commit 7c404d5

Browse files
mknyszekgopherbot
authored andcommitted
runtime: store consistent total allocation stats as uint64
Currently the consistent total allocation stats are managed as uintptrs, which means they can easily overflow on 32-bit systems. Fix this by storing these stats as uint64s. This will cause some minor performance degradation on 32-bit systems, but there really isn't a way around this, and it affects the correctness of the metrics we export. Fixes #52680. Change-Id: I7e6ca44047d46b4bd91c6f87c2d29f730e0d6191 Reviewed-on: https://go-review.googlesource.com/c/go/+/403758 Run-TryBot: Michael Knyszek <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Auto-Submit: Michael Knyszek <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent bccce90 commit 7c404d5

File tree

4 files changed

+41
-39
lines changed

4 files changed

+41
-39
lines changed

src/runtime/mcache.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,18 +159,18 @@ func (c *mcache) refill(spc spanClass) {
159159

160160
// Count up how many slots were used and record it.
161161
stats := memstats.heapStats.acquire()
162-
slotsUsed := uintptr(s.allocCount) - uintptr(s.allocCountBeforeCache)
163-
atomic.Xadduintptr(&stats.smallAllocCount[spc.sizeclass()], slotsUsed)
162+
slotsUsed := int64(s.allocCount) - int64(s.allocCountBeforeCache)
163+
atomic.Xadd64(&stats.smallAllocCount[spc.sizeclass()], slotsUsed)
164164

165165
// Flush tinyAllocs.
166166
if spc == tinySpanClass {
167-
atomic.Xadduintptr(&stats.tinyAllocCount, c.tinyAllocs)
167+
atomic.Xadd64(&stats.tinyAllocCount, int64(c.tinyAllocs))
168168
c.tinyAllocs = 0
169169
}
170170
memstats.heapStats.release()
171171

172172
// Count the allocs in inconsistent, internal stats.
173-
bytesAllocated := int64(slotsUsed * s.elemsize)
173+
bytesAllocated := slotsUsed * int64(s.elemsize)
174174
gcController.totalAlloc.Add(bytesAllocated)
175175

176176
// Update heapLive and flush scanAlloc.
@@ -224,8 +224,8 @@ func (c *mcache) allocLarge(size uintptr, noscan bool) *mspan {
224224

225225
// Count the alloc in consistent, external stats.
226226
stats := memstats.heapStats.acquire()
227-
atomic.Xadduintptr(&stats.largeAlloc, npages*pageSize)
228-
atomic.Xadduintptr(&stats.largeAllocCount, 1)
227+
atomic.Xadd64(&stats.largeAlloc, int64(npages*pageSize))
228+
atomic.Xadd64(&stats.largeAllocCount, 1)
229229
memstats.heapStats.release()
230230

231231
// Count the alloc in inconsistent, internal stats.
@@ -250,17 +250,17 @@ func (c *mcache) releaseAll() {
250250
for i := range c.alloc {
251251
s := c.alloc[i]
252252
if s != &emptymspan {
253-
slotsUsed := uintptr(s.allocCount) - uintptr(s.allocCountBeforeCache)
253+
slotsUsed := int64(s.allocCount) - int64(s.allocCountBeforeCache)
254254
s.allocCountBeforeCache = 0
255255

256256
// Adjust smallAllocCount for whatever was allocated.
257257
stats := memstats.heapStats.acquire()
258-
atomic.Xadduintptr(&stats.smallAllocCount[spanClass(i).sizeclass()], slotsUsed)
258+
atomic.Xadd64(&stats.smallAllocCount[spanClass(i).sizeclass()], slotsUsed)
259259
memstats.heapStats.release()
260260

261261
// Adjust the actual allocs in inconsistent, internal stats.
262262
// We assumed earlier that the full span gets allocated.
263-
gcController.totalAlloc.Add(int64(slotsUsed * s.elemsize))
263+
gcController.totalAlloc.Add(slotsUsed * int64(s.elemsize))
264264

265265
// Release the span to the mcentral.
266266
mheap_.central[i].mcentral.uncacheSpan(s)
@@ -273,7 +273,7 @@ func (c *mcache) releaseAll() {
273273

274274
// Flush tinyAllocs.
275275
stats := memstats.heapStats.acquire()
276-
atomic.Xadduintptr(&stats.tinyAllocCount, c.tinyAllocs)
276+
atomic.Xadd64(&stats.tinyAllocCount, int64(c.tinyAllocs))
277277
c.tinyAllocs = 0
278278
memstats.heapStats.release()
279279

src/runtime/metrics.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,13 +388,13 @@ func (a *heapStatsAggregate) compute() {
388388
memstats.heapStats.read(&a.heapStatsDelta)
389389

390390
// Calculate derived stats.
391-
a.totalAllocs = uint64(a.largeAllocCount)
392-
a.totalFrees = uint64(a.largeFreeCount)
393-
a.totalAllocated = uint64(a.largeAlloc)
394-
a.totalFreed = uint64(a.largeFree)
391+
a.totalAllocs = a.largeAllocCount
392+
a.totalFrees = a.largeFreeCount
393+
a.totalAllocated = a.largeAlloc
394+
a.totalFreed = a.largeFree
395395
for i := range a.smallAllocCount {
396-
na := uint64(a.smallAllocCount[i])
397-
nf := uint64(a.smallFreeCount[i])
396+
na := a.smallAllocCount[i]
397+
nf := a.smallFreeCount[i]
398398
a.totalAllocs += na
399399
a.totalFrees += nf
400400
a.totalAllocated += na * uint64(class_to_size[i])

src/runtime/mgcsweep.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ func (sl *sweepLocked) sweep(preserve bool) bool {
668668
// free slots zeroed.
669669
s.needzero = 1
670670
stats := memstats.heapStats.acquire()
671-
atomic.Xadduintptr(&stats.smallFreeCount[spc.sizeclass()], uintptr(nfreed))
671+
atomic.Xadd64(&stats.smallFreeCount[spc.sizeclass()], int64(nfreed))
672672
memstats.heapStats.release()
673673

674674
// Count the frees in the inconsistent, internal stats.
@@ -720,8 +720,8 @@ func (sl *sweepLocked) sweep(preserve bool) bool {
720720

721721
// Count the free in the consistent, external stats.
722722
stats := memstats.heapStats.acquire()
723-
atomic.Xadduintptr(&stats.largeFreeCount, 1)
724-
atomic.Xadduintptr(&stats.largeFree, size)
723+
atomic.Xadd64(&stats.largeFreeCount, 1)
724+
atomic.Xadd64(&stats.largeFree, int64(size))
725725
memstats.heapStats.release()
726726

727727
// Count the free in the inconsistent, internal stats.

src/runtime/mstats.go

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
package runtime
88

99
import (
10-
"internal/goarch"
1110
"runtime/internal/atomic"
1211
"unsafe"
1312
)
@@ -388,10 +387,10 @@ func readmemstats_m(stats *MemStats) {
388387
memstats.heapStats.unsafeRead(&consStats)
389388

390389
// Collect large allocation stats.
391-
totalAlloc := uint64(consStats.largeAlloc)
392-
nMalloc := uint64(consStats.largeAllocCount)
393-
totalFree := uint64(consStats.largeFree)
394-
nFree := uint64(consStats.largeFreeCount)
390+
totalAlloc := consStats.largeAlloc
391+
nMalloc := consStats.largeAllocCount
392+
totalFree := consStats.largeFree
393+
nFree := consStats.largeFreeCount
395394

396395
// Collect per-sizeclass stats.
397396
var bySize [_NumSizeClasses]struct {
@@ -403,13 +402,13 @@ func readmemstats_m(stats *MemStats) {
403402
bySize[i].Size = uint32(class_to_size[i])
404403

405404
// Malloc stats.
406-
a := uint64(consStats.smallAllocCount[i])
405+
a := consStats.smallAllocCount[i]
407406
totalAlloc += a * uint64(class_to_size[i])
408407
nMalloc += a
409408
bySize[i].Mallocs = a
410409

411410
// Free stats.
412-
f := uint64(consStats.smallFreeCount[i])
411+
f := consStats.smallFreeCount[i]
413412
totalFree += f * uint64(class_to_size[i])
414413
nFree += f
415414
bySize[i].Frees = f
@@ -421,8 +420,8 @@ func readmemstats_m(stats *MemStats) {
421420
// memory in some sense because their tiny allocation block is also
422421
// counted. Tracking the lifetime of individual tiny allocations is
423422
// currently not done because it would be too expensive.
424-
nFree += uint64(consStats.tinyAllocCount)
425-
nMalloc += uint64(consStats.tinyAllocCount)
423+
nFree += consStats.tinyAllocCount
424+
nMalloc += consStats.tinyAllocCount
426425

427426
// Calculate derived stats.
428427

@@ -663,17 +662,20 @@ type heapStatsDelta struct {
663662
inPtrScalarBits int64 // byte delta of memory reserved for unrolled GC prog bits
664663

665664
// Allocator stats.
666-
tinyAllocCount uintptr // number of tiny allocations
667-
largeAlloc uintptr // bytes allocated for large objects
668-
largeAllocCount uintptr // number of large object allocations
669-
smallAllocCount [_NumSizeClasses]uintptr // number of allocs for small objects
670-
largeFree uintptr // bytes freed for large objects (>maxSmallSize)
671-
largeFreeCount uintptr // number of frees for large objects (>maxSmallSize)
672-
smallFreeCount [_NumSizeClasses]uintptr // number of frees for small objects (<=maxSmallSize)
673-
674-
// Add a uint32 to ensure this struct is a multiple of 8 bytes in size.
675-
// Only necessary on 32-bit platforms.
676-
_ [(goarch.PtrSize / 4) % 2]uint32
665+
//
666+
// These are all uint64 because they're cumulative, and could quickly wrap
667+
// around otherwise.
668+
tinyAllocCount uint64 // number of tiny allocations
669+
largeAlloc uint64 // bytes allocated for large objects
670+
largeAllocCount uint64 // number of large object allocations
671+
smallAllocCount [_NumSizeClasses]uint64 // number of allocs for small objects
672+
largeFree uint64 // bytes freed for large objects (>maxSmallSize)
673+
largeFreeCount uint64 // number of frees for large objects (>maxSmallSize)
674+
smallFreeCount [_NumSizeClasses]uint64 // number of frees for small objects (<=maxSmallSize)
675+
676+
// NOTE: This struct must be a multiple of 8 bytes in size because it
677+
// is stored in an array. If it's not, atomic accesses to the above
678+
// fields may be unaligned and fail on 32-bit platforms.
677679
}
678680

679681
// merge adds in the deltas from b into a.

0 commit comments

Comments
 (0)