Skip to content

Commit ac766e3

Browse files
committed
runtime: make getMCache inlineable
This change moves the responsibility of throwing if an mcache is not available to the caller, because the inlining cost of throw is set very high in the compiler. Even if it was reduced down to the cost of a usual function call, it would still be too expensive, so just move it out. This choice also makes sense in the context of #42339 since we're going to have to handle the case where we don't have an mcache to update stats in a few contexts anyhow. Also, add getMCache to the list of functions that should be inlined to prevent future regressions. getMCache is called on the allocation fast path and because its not inlined actually causes a significant regression (~10%) in some microbenchmarks. Fixes #42305. Change-Id: I64ac5e4f26b730bd4435ea1069a4a50f55411ced Reviewed-on: https://go-review.googlesource.com/c/go/+/267157 Trust: Michael Knyszek <[email protected]> Run-TryBot: Michael Knyszek <[email protected]> Reviewed-by: Michael Pratt <[email protected]> TryBot-Result: Go Bot <[email protected]>
1 parent 4fcb506 commit ac766e3

File tree

5 files changed

+26
-6
lines changed

5 files changed

+26
-6
lines changed

src/cmd/compile/internal/gc/inl_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ func TestIntendedInlining(t *testing.T) {
5151
"funcPC",
5252
"getArgInfoFast",
5353
"getm",
54+
"getMCache",
5455
"isDirectIface",
5556
"itabHashFunc",
5657
"noescape",

src/runtime/malloc.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,6 +975,9 @@ func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer {
975975
shouldhelpgc := false
976976
dataSize := size
977977
c := getMCache()
978+
if c == nil {
979+
throw("mallocgc called without a P or outside bootstrapping")
980+
}
978981
var span *mspan
979982
var x unsafe.Pointer
980983
noscan := typ == nil || typ.ptrdata == 0
@@ -1202,7 +1205,11 @@ func reflect_unsafe_NewArray(typ *_type, n int) unsafe.Pointer {
12021205
}
12031206

12041207
func profilealloc(mp *m, x unsafe.Pointer, size uintptr) {
1205-
getMCache().nextSample = nextSample()
1208+
c := getMCache()
1209+
if c == nil {
1210+
throw("profilealloc called without a P or outside bootstrapping")
1211+
}
1212+
c.nextSample = nextSample()
12061213
mProf_Malloc(x, size)
12071214
}
12081215

src/runtime/mcache.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ func freemcache(c *mcache) {
124124

125125
// getMCache is a convenience function which tries to obtain an mcache.
126126
//
127-
// Must be running with a P when called (so the caller must be in a
128-
// non-preemptible state) or must be called during bootstrapping.
127+
// Returns nil if we're not bootstrapping or we don't have a P. The caller's
128+
// P must not change, so we must be in a non-preemptible state.
129129
func getMCache() *mcache {
130130
// Grab the mcache, since that's where stats live.
131131
pp := getg().m.p.ptr()
@@ -136,9 +136,6 @@ func getMCache() *mcache {
136136
// mcache0 is cleared when bootstrapping is complete,
137137
// by procresize.
138138
c = mcache0
139-
if c == nil {
140-
throw("getMCache called with no P or outside bootstrapping")
141-
}
142139
} else {
143140
c = pp.mcache
144141
}

src/runtime/mgcscavenge.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,9 @@ func (p *pageAlloc) scavengeRangeLocked(ci chunkIdx, base, npages uint) uintptr
734734

735735
// Update consistent accounting too.
736736
c := getMCache()
737+
if c == nil {
738+
throw("scavengeRangeLocked called without a P or outside bootstrapping")
739+
}
737740
stats := memstats.heapStats.acquire(c)
738741
atomic.Xaddint64(&stats.committed, -nbytes)
739742
atomic.Xaddint64(&stats.released, nbytes)

src/runtime/mheap.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,10 @@ HaveSpan:
12471247
}
12481248
// Update consistent stats.
12491249
c := getMCache()
1250+
if c == nil {
1251+
// TODO(mknyszek): Remove this and handle this case to fix #42339.
1252+
throw("allocSpan called without P or outside bootstrapping")
1253+
}
12501254
stats := memstats.heapStats.acquire(c)
12511255
atomic.Xaddint64(&stats.committed, int64(scav))
12521256
atomic.Xaddint64(&stats.released, -int64(scav))
@@ -1341,6 +1345,10 @@ func (h *mheap) grow(npage uintptr) bool {
13411345
// just add directly to heap_released.
13421346
atomic.Xadd64(&memstats.heap_released, int64(asize))
13431347
c := getMCache()
1348+
if c == nil {
1349+
// TODO(mknyszek): Remove this and handle this case to fix #42339.
1350+
throw("grow called without P or outside bootstrapping")
1351+
}
13441352
stats := memstats.heapStats.acquire(c)
13451353
atomic.Xaddint64(&stats.released, int64(asize))
13461354
memstats.heapStats.release(c)
@@ -1440,6 +1448,10 @@ func (h *mheap) freeSpanLocked(s *mspan, typ spanAllocType) {
14401448
}
14411449
// Update consistent stats.
14421450
c := getMCache()
1451+
if c == nil {
1452+
// TODO(mknyszek): Remove this and handle this case to fix #42339.
1453+
throw("freeSpanLocked called without P or outside bootstrapping")
1454+
}
14431455
stats := memstats.heapStats.acquire(c)
14441456
switch typ {
14451457
case spanAllocHeap:

0 commit comments

Comments
 (0)