Commit 01d13726 authored by Keith Randall's avatar Keith Randall Committed by Keith Randall

runtime: use uintptr instead of int32 for counting to next heap profile sample

Overflow of the comparison caused very large (>=1<<32) allocations to
sometimes not get sampled at all. Use uintptr so the comparison will
never overflow.

Fixes #33342

Tested on the example in 33342. I don't want to check a test in that
needs that much memory, however.

Change-Id: I51fe77a9117affed8094da93c0bc5f445ac2d3d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/188017
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarAustin Clements <austin@google.com>
parent 7b8234b4
...@@ -1075,8 +1075,8 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer { ...@@ -1075,8 +1075,8 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
} }
if rate := MemProfileRate; rate > 0 { if rate := MemProfileRate; rate > 0 {
if rate != 1 && int32(size) < c.next_sample { if rate != 1 && size < c.next_sample {
c.next_sample -= int32(size) c.next_sample -= size
} else { } else {
mp := acquirem() mp := acquirem()
profilealloc(mp, x, size) profilealloc(mp, x, size)
...@@ -1170,7 +1170,7 @@ func profilealloc(mp *m, x unsafe.Pointer, size uintptr) { ...@@ -1170,7 +1170,7 @@ func profilealloc(mp *m, x unsafe.Pointer, size uintptr) {
// processes, the distance between two samples follows the exponential // processes, the distance between two samples follows the exponential
// distribution (exp(MemProfileRate)), so the best return value is a random // distribution (exp(MemProfileRate)), so the best return value is a random
// number taken from an exponential distribution whose mean is MemProfileRate. // number taken from an exponential distribution whose mean is MemProfileRate.
func nextSample() int32 { func nextSample() uintptr {
if GOOS == "plan9" { if GOOS == "plan9" {
// Plan 9 doesn't support floating point in note handler. // Plan 9 doesn't support floating point in note handler.
if g := getg(); g == g.m.gsignal { if g := getg(); g == g.m.gsignal {
...@@ -1178,7 +1178,7 @@ func nextSample() int32 { ...@@ -1178,7 +1178,7 @@ func nextSample() int32 {
} }
} }
return fastexprand(MemProfileRate) return uintptr(fastexprand(MemProfileRate))
} }
// fastexprand returns a random number from an exponential distribution with // fastexprand returns a random number from an exponential distribution with
...@@ -1213,14 +1213,14 @@ func fastexprand(mean int) int32 { ...@@ -1213,14 +1213,14 @@ func fastexprand(mean int) int32 {
// nextSampleNoFP is similar to nextSample, but uses older, // nextSampleNoFP is similar to nextSample, but uses older,
// simpler code to avoid floating point. // simpler code to avoid floating point.
func nextSampleNoFP() int32 { func nextSampleNoFP() uintptr {
// Set first allocation sample size. // Set first allocation sample size.
rate := MemProfileRate rate := MemProfileRate
if rate > 0x3fffffff { // make 2*rate not overflow if rate > 0x3fffffff { // make 2*rate not overflow
rate = 0x3fffffff rate = 0x3fffffff
} }
if rate != 0 { if rate != 0 {
return int32(fastrand() % uint32(2*rate)) return uintptr(fastrand() % uint32(2*rate))
} }
return 0 return 0
} }
......
...@@ -19,7 +19,7 @@ import ( ...@@ -19,7 +19,7 @@ import (
type mcache struct { type mcache struct {
// The following members are accessed on every malloc, // The following members are accessed on every malloc,
// so they are grouped here for better caching. // so they are grouped here for better caching.
next_sample int32 // trigger heap sample after allocating this many bytes next_sample uintptr // trigger heap sample after allocating this many bytes
local_scan uintptr // bytes of scannable heap allocated local_scan uintptr // bytes of scannable heap allocated
// Allocator cache for tiny objects w/o pointers. // Allocator cache for tiny objects w/o pointers.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment