runtime: drop stack-allocated pcvalueCaches

Now that pcvalue keeps its cache on the M, we can drop all of the
stack-allocated pcvalueCaches and stop carefully passing them around
between lots of operations. This significantly simplifies a fair
amount of code and makes several structures smaller.

This series of changes has no statistically significant effect on any
runtime Stack benchmarks.

I also experimented with making the cache larger, now that the impact
is limited to the M struct, but wasn't able to measure any
improvements.

Change-Id: I4719ebf347c7150a05e887e75a238e23647c20cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/515277
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

diff --git a/src/runtime/debugcall.go b/src/runtime/debugcall.go
index ea413bd0c5644a980d34e70bbf34821648e8b547..f8b3494ec3053ff7cef33e751523a1d1e2a58679 100644 (file)
--- a/src/runtime/debugcall.go
+++ b/src/runtime/debugcall.go
@@ -83,7 +83,7 @@ func debugCallCheck(pc uintptr) string {
                if pc != f.entry() {
                        pc--
                }
-               up := pcdatavalue(f, abi.PCDATA_UnsafePoint, pc, nil)
+               up := pcdatavalue(f, abi.PCDATA_UnsafePoint, pc)
                if up != abi.UnsafePointSafe {
                        // Not at a safe point.
                        ret = debugCallUnsafePoint

diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go
index 8ddec8b2d5d9193297247c8f558fe191e09d0877..430e4bccb5e89d6d853c07da81c04f45b3498044 100644 (file)
--- a/src/runtime/heapdump.go
+++ b/src/runtime/heapdump.go
@@ -259,7 +259,7 @@ func dumpframe(s *stkframe, child *childInfo) {
        pcdata := int32(-1) // Use the entry map at function entry
        if pc != f.entry() {
                pc--
-               pcdata = pcdatavalue(f, abi.PCDATA_StackMapIndex, pc, nil)
+               pcdata = pcdatavalue(f, abi.PCDATA_StackMapIndex, pc)
        }
        if pcdata == -1 {
                // We do not have a valid pcdata value but there might be a

diff --git a/src/runtime/mbitmap.go b/src/runtime/mbitmap.go
index a24287288472282d388274b3ba110cb31c380e4b..4ba25901d43077bdb0557d645e9e81801eeaa8d4 100644 (file)
--- a/src/runtime/mbitmap.go
+++ b/src/runtime/mbitmap.go
@@ -1479,7 +1479,7 @@ func getgcmask(ep any) (mask []byte) {
                        }
                }
                if found {
-                       locals, _, _ := u.frame.getStackMap(nil, false)
+                       locals, _, _ := u.frame.getStackMap(false)
                        if locals.n == 0 {
                                return
                        }

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index c49eba030225d3b0e1bfb2243195a1ea85fe7b44..2b4e23823b96edb2fed4bdd07797233778613f72 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -964,7 +964,7 @@ func scanframeworker(frame *stkframe, state *stackScanState, gcw *gcWork) {
                return
        }
 
-       locals, args, objs := frame.getStackMap(&state.cache, false)
+       locals, args, objs := frame.getStackMap(false)
 
        // Scan local variables if stack frame has been allocated.
        if locals.n > 0 {

diff --git a/src/runtime/mgcstack.go b/src/runtime/mgcstack.go
index 6b552203ee42437691cc207bedf8777b0c65dbb1..f4a83f5f59c27ab350895faec0980687444e7880 100644 (file)
--- a/src/runtime/mgcstack.go
+++ b/src/runtime/mgcstack.go
@@ -166,8 +166,6 @@ func (obj *stackObject) setRecord(r *stackObjectRecord) {
 // A stackScanState keeps track of the state used during the GC walk
 // of a goroutine.
 type stackScanState struct {
-       cache pcvalueCache
-
        // stack limits
        stack stack
 

diff --git a/src/runtime/preempt.go b/src/runtime/preempt.go
index 76d8ba4cdf49e0daf74dd9a8df24f733e319e2bc..82d85cd707619a5e140c018ad005f7e9e2c5779f 100644 (file)
--- a/src/runtime/preempt.go
+++ b/src/runtime/preempt.go
@@ -385,7 +385,7 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
                // Not Go code.
                return false, 0
        }
-       if (GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "mips64" || GOARCH == "mips64le") && lr == pc+8 && funcspdelta(f, pc, nil) == 0 {
+       if (GOARCH == "mips" || GOARCH == "mipsle" || GOARCH == "mips64" || GOARCH == "mips64le") && lr == pc+8 && funcspdelta(f, pc) == 0 {
                // We probably stopped at a half-executed CALL instruction,
                // where the LR is updated but the PC has not. If we preempt
                // here we'll see a seemingly self-recursive call, which is in
@@ -414,7 +414,7 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
                return false, 0
        }
        // Check the inner-most name
-       u, uf := newInlineUnwinder(f, pc, nil)
+       u, uf := newInlineUnwinder(f, pc)
        name := u.srcFunc(uf).name()
        if hasPrefix(name, "runtime.") ||
                hasPrefix(name, "runtime/internal/") ||

diff --git a/src/runtime/race.go b/src/runtime/race.go
index e2767f0324e7b2f0985d506312d82242adb4b441..496e3725bf9712f41d2760de461433cb5ea883cd 100644 (file)
--- a/src/runtime/race.go
+++ b/src/runtime/race.go
@@ -172,7 +172,7 @@ func raceSymbolizeCode(ctx *symbolizeCodeContext) {
        pc := ctx.pc
        fi := findfunc(pc)
        if fi.valid() {
-               u, uf := newInlineUnwinder(fi, pc, nil)
+               u, uf := newInlineUnwinder(fi, pc)
                for ; uf.valid(); uf = u.next(uf) {
                        sf := u.srcFunc(uf)
                        if sf.funcID == abi.FuncIDWrapper && u.isInlined(uf) {

diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index 903b096f08279311c1fcf5b6ad1f467a6db33408..61cd0a0fddd6dbebd45b60743c839537fd4ec2ac 100644 (file)
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -555,7 +555,6 @@ var ptrnames = []string{
 type adjustinfo struct {
        old   stack
        delta uintptr // ptr distance from old to new stack (newbase - oldbase)
-       cache pcvalueCache
 
        // sghi is the highest sudog.elem on the stack.
        sghi uintptr
@@ -676,7 +675,7 @@ func adjustframe(frame *stkframe, adjinfo *adjustinfo) {
                adjustpointer(adjinfo, unsafe.Pointer(frame.varp))
        }
 
-       locals, args, objs := frame.getStackMap(&adjinfo.cache, true)
+       locals, args, objs := frame.getStackMap(true)
 
        // Adjust local variables if stack frame has been allocated.
        if locals.n > 0 {

diff --git a/src/runtime/stkframe.go b/src/runtime/stkframe.go
index 5caacbacbab8b6d1b409ea73df737253706db248..bfd9eac2b0e2e8e3c85676d990fe00ed88a422be 100644 (file)
--- a/src/runtime/stkframe.go
+++ b/src/runtime/stkframe.go
@@ -154,7 +154,7 @@ func (frame *stkframe) argMapInternal() (argMap bitvector, hasReflectStackObj bo
 
 // getStackMap returns the locals and arguments live pointer maps, and
 // stack object list for frame.
-func (frame *stkframe) getStackMap(cache *pcvalueCache, debug bool) (locals, args bitvector, objs []stackObjectRecord) {
+func (frame *stkframe) getStackMap(debug bool) (locals, args bitvector, objs []stackObjectRecord) {
        targetpc := frame.continpc
        if targetpc == 0 {
                // Frame is dead. Return empty bitvectors.
@@ -169,7 +169,7 @@ func (frame *stkframe) getStackMap(cache *pcvalueCache, debug bool) (locals, arg
                // the first instruction of the function changes the
                // stack map.
                targetpc--
-               pcdata = pcdatavalue(f, abi.PCDATA_StackMapIndex, targetpc, cache)
+               pcdata = pcdatavalue(f, abi.PCDATA_StackMapIndex, targetpc)
        }
        if pcdata == -1 {
                // We do not have a valid pcdata value but there might be a

diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go
index 18ba683d69ddc867c0815033b5056f3b1f132589..0397b6ecf5fda5c7d6ad23018d3b78b37261318a 100644 (file)
--- a/src/runtime/symtab.go
+++ b/src/runtime/symtab.go
@@ -119,7 +119,7 @@ func (ci *Frames) Next() (frame Frame, more bool) {
                }
                // It's important that interpret pc non-strictly as cgoTraceback may
                // have added bogus PCs with a valid funcInfo but invalid PCDATA.
-               u, uf := newInlineUnwinder(funcInfo, pc, nil)
+               u, uf := newInlineUnwinder(funcInfo, pc)
                sf := u.srcFunc(uf)
                if u.isInlined(uf) {
                        // Note: entry is not modified. It always refers to a real frame, not an inlined one.
@@ -180,7 +180,7 @@ func runtime_FrameSymbolName(f *Frame) string {
        if !f.funcInfo.valid() {
                return f.Function
        }
-       u, uf := newInlineUnwinder(f.funcInfo, f.PC, nil)
+       u, uf := newInlineUnwinder(f.funcInfo, f.PC)
        sf := u.srcFunc(uf)
        return sf.name()
 }
@@ -204,8 +204,7 @@ func runtime_expandFinalInlineFrame(stk []uintptr) []uintptr {
                return stk
        }
 
-       var cache pcvalueCache
-       u, uf := newInlineUnwinder(f, tracepc, &cache)
+       u, uf := newInlineUnwinder(f, tracepc)
        if !u.isInlined(uf) {
                // Nothing inline at tracepc.
                return stk
@@ -658,7 +657,7 @@ func FuncForPC(pc uintptr) *Func {
        // We just report the preceding function in that situation. See issue 29735.
        // TODO: Perhaps we should report no function at all in that case.
        // The runtime currently doesn't have function end info, alas.
-       u, uf := newInlineUnwinder(f, pc, nil)
+       u, uf := newInlineUnwinder(f, pc)
        if !u.isInlined(uf) {
                return f._Func()
        }
@@ -843,7 +842,7 @@ func pcvalueCacheKey(targetpc uintptr) uintptr {
 }
 
 // Returns the PCData value, and the PC where this value starts.
-func pcvalue(f funcInfo, off uint32, targetpc uintptr, _ *pcvalueCache, strict bool) (int32, uintptr) {
+func pcvalue(f funcInfo, off uint32, targetpc uintptr, strict bool) (int32, uintptr) {
        if off == 0 {
                return -1, 0
        }
@@ -979,8 +978,8 @@ func funcline1(f funcInfo, targetpc uintptr, strict bool) (file string, line int
        if !f.valid() {
                return "?", 0
        }
-       fileno, _ := pcvalue(f, f.pcfile, targetpc, nil, strict)
-       line, _ = pcvalue(f, f.pcln, targetpc, nil, strict)
+       fileno, _ := pcvalue(f, f.pcfile, targetpc, strict)
+       line, _ = pcvalue(f, f.pcln, targetpc, strict)
        if fileno == -1 || line == -1 || int(fileno) >= len(datap.filetab) {
                // print("looking for ", hex(targetpc), " in ", funcname(f), " got file=", fileno, " line=", lineno, "\n")
                return "?", 0
@@ -993,8 +992,8 @@ func funcline(f funcInfo, targetpc uintptr) (file string, line int32) {
        return funcline1(f, targetpc, true)
 }
 
-func funcspdelta(f funcInfo, targetpc uintptr, cache *pcvalueCache) int32 {
-       x, _ := pcvalue(f, f.pcsp, targetpc, cache, true)
+func funcspdelta(f funcInfo, targetpc uintptr) int32 {
+       x, _ := pcvalue(f, f.pcsp, targetpc, true)
        if debugPcln && x&(goarch.PtrSize-1) != 0 {
                print("invalid spdelta ", funcname(f), " ", hex(f.entry()), " ", hex(targetpc), " ", hex(f.pcsp), " ", x, "\n")
                throw("bad spdelta")
@@ -1025,29 +1024,28 @@ func pcdatastart(f funcInfo, table uint32) uint32 {
        return *(*uint32)(add(unsafe.Pointer(&f.nfuncdata), unsafe.Sizeof(f.nfuncdata)+uintptr(table)*4))
 }
 
-func pcdatavalue(f funcInfo, table uint32, targetpc uintptr, cache *pcvalueCache) int32 {
+func pcdatavalue(f funcInfo, table uint32, targetpc uintptr) int32 {
        if table >= f.npcdata {
                return -1
        }
-       r, _ := pcvalue(f, pcdatastart(f, table), targetpc, cache, true)
+       r, _ := pcvalue(f, pcdatastart(f, table), targetpc, true)
        return r
 }
 
-func pcdatavalue1(f funcInfo, table uint32, targetpc uintptr, cache *pcvalueCache, strict bool) int32 {
+func pcdatavalue1(f funcInfo, table uint32, targetpc uintptr, strict bool) int32 {
        if table >= f.npcdata {
                return -1
        }
-       r, _ := pcvalue(f, pcdatastart(f, table), targetpc, cache, strict)
+       r, _ := pcvalue(f, pcdatastart(f, table), targetpc, strict)
        return r
 }
 
 // Like pcdatavalue, but also return the start PC of this PCData value.
-// It doesn't take a cache.
 func pcdatavalue2(f funcInfo, table uint32, targetpc uintptr) (int32, uintptr) {
        if table >= f.npcdata {
                return -1, 0
        }
-       return pcvalue(f, pcdatastart(f, table), targetpc, nil, true)
+       return pcvalue(f, pcdatastart(f, table), targetpc, true)
 }
 
 // funcdata returns a pointer to the ith funcdata for f.

diff --git a/src/runtime/symtabinl.go b/src/runtime/symtabinl.go
index 2bb1c4bc6af5c1f34a98fa097e3fa34867378492..9273b49b11e51f4a6e6e63d29d9588b769df514c 100644 (file)
--- a/src/runtime/symtabinl.go
+++ b/src/runtime/symtabinl.go
@@ -30,7 +30,6 @@ type inlinedCall struct {
 // code.
 type inlineUnwinder struct {
        f       funcInfo
-       cache   *pcvalueCache
        inlTree *[1 << 20]inlinedCall
 }
 
@@ -52,13 +51,13 @@ type inlineFrame struct {
 // This unwinder uses non-strict handling of PC because it's assumed this is
 // only ever used for symbolic debugging. If things go really wrong, it'll just
 // fall back to the outermost frame.
-func newInlineUnwinder(f funcInfo, pc uintptr, cache *pcvalueCache) (inlineUnwinder, inlineFrame) {
+func newInlineUnwinder(f funcInfo, pc uintptr) (inlineUnwinder, inlineFrame) {
        inldata := funcdata(f, abi.FUNCDATA_InlTree)
        if inldata == nil {
                return inlineUnwinder{f: f}, inlineFrame{pc: pc, index: -1}
        }
        inlTree := (*[1 << 20]inlinedCall)(inldata)
-       u := inlineUnwinder{f: f, cache: cache, inlTree: inlTree}
+       u := inlineUnwinder{f: f, inlTree: inlTree}
        return u, u.resolveInternal(pc)
 }
 
@@ -67,7 +66,7 @@ func (u *inlineUnwinder) resolveInternal(pc uintptr) inlineFrame {
                pc: pc,
                // Conveniently, this returns -1 if there's an error, which is the same
                // value we use for the outermost frame.
-               index: pcdatavalue1(u.f, abi.PCDATA_InlTreeIndex, pc, u.cache, false),
+               index: pcdatavalue1(u.f, abi.PCDATA_InlTreeIndex, pc, false),
        }
 }
 

diff --git a/src/runtime/symtabinl_test.go b/src/runtime/symtabinl_test.go
index 9e75f79281fa4e55cb7d653ae2145ca247743c7c..df524aec4a039b5378679001caf2844bf23c3947 100644 (file)
--- a/src/runtime/symtabinl_test.go
+++ b/src/runtime/symtabinl_test.go
@@ -34,10 +34,9 @@ func XTestInlineUnwinder(t TestingT) {
 
        // Iterate over the PCs in tiuTest and walk the inline stack for each.
        prevStack := "x"
-       var cache pcvalueCache
        for pc := pc1; pc < pc1+1024 && findfunc(pc) == f; pc += sys.PCQuantum {
                stack := ""
-               u, uf := newInlineUnwinder(f, pc, &cache)
+               u, uf := newInlineUnwinder(f, pc)
                if file, _ := u.fileLine(uf); file == "?" {
                        // We're probably in the trailing function padding, where findfunc
                        // still returns f but there's no symbolic information. Just keep

diff --git a/src/runtime/trace.go b/src/runtime/trace.go
index a4d50d77a0102d880dd9b0d0efc5df6917b05bbc..125b8678db3460037d5eba752490968c97f5cb74 100644 (file)
--- a/src/runtime/trace.go
+++ b/src/runtime/trace.go
@@ -1345,7 +1345,6 @@ func fpunwindExpand(pcBuf []uintptr) []uintptr {
        }
 
        var (
-               cache      pcvalueCache
                lastFuncID = abi.FuncIDNormal
                newPCBuf   = make([]uintptr, 0, traceStackSize)
                skip       = pcBuf[0]
@@ -1374,7 +1373,7 @@ outer:
                        continue
                }
 
-               u, uf := newInlineUnwinder(fi, callPC, &cache)
+               u, uf := newInlineUnwinder(fi, callPC)
                for ; uf.valid(); uf = u.next(uf) {
                        sf := u.srcFunc(uf)
                        if sf.funcID == abi.FuncIDWrapper && elideWrapperCalling(lastFuncID) {

diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 86df1155b5c114bdcf3588039b1109e675b1d07b..5dfac4fa011d01dd7879cdaf119463fe09dd7639 100644 (file)
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -112,9 +112,6 @@ type unwinder struct {
        // flags are the flags to this unwind. Some of these are updated as we
        // unwind (see the flags documentation).
        flags unwindFlags
-
-       // cache is used to cache pcvalue lookups.
-       cache pcvalueCache
 }
 
 // init initializes u to start unwinding gp's stack and positions the
@@ -307,7 +304,7 @@ func (u *unwinder) resolveInternal(innermost, isSyscall bool) {
                        case abi.FuncID_systemstack:
                                // systemstack returns normally, so just follow the
                                // stack transition.
-                               if usesLR && funcspdelta(f, frame.pc, &u.cache) == 0 {
+                               if usesLR && funcspdelta(f, frame.pc) == 0 {
                                        // We're at the function prologue and the stack
                                        // switch hasn't happened, or epilogue where we're
                                        // about to return. Just unwind normally.
@@ -325,7 +322,7 @@ func (u *unwinder) resolveInternal(innermost, isSyscall bool) {
                                flag &^= abi.FuncFlagSPWrite
                        }
                }
-               frame.fp = frame.sp + uintptr(funcspdelta(f, frame.pc, &u.cache))
+               frame.fp = frame.sp + uintptr(funcspdelta(f, frame.pc))
                if !usesLR {
                        // On x86, call instruction pushes return PC before entering new function.
                        frame.fp += goarch.PtrSize
@@ -500,7 +497,7 @@ func (u *unwinder) next() {
                frame.fn = f
                if !f.valid() {
                        frame.pc = x
-               } else if funcspdelta(f, frame.pc, &u.cache) == 0 {
+               } else if funcspdelta(f, frame.pc) == 0 {
                        frame.lr = x
                }
        }
@@ -620,7 +617,7 @@ func tracebackPCs(u *unwinder, skip int, pcBuf []uintptr) int {
                cgoN := u.cgoCallers(cgoBuf[:])
 
                // TODO: Why does &u.cache cause u to escape? (Same in traceback2)
-               for iu, uf := newInlineUnwinder(f, u.symPC(), noEscapePtr(&u.cache)); n < len(pcBuf) && uf.valid(); uf = iu.next(uf) {
+               for iu, uf := newInlineUnwinder(f, u.symPC()); n < len(pcBuf) && uf.valid(); uf = iu.next(uf) {
                        sf := iu.srcFunc(uf)
                        if sf.funcID == abi.FuncIDWrapper && elideWrapperCalling(u.calleeFuncID) {
                                // ignore wrappers
@@ -670,7 +667,7 @@ func printArgs(f funcInfo, argp unsafe.Pointer, pc uintptr) {
        }
 
        liveInfo := funcdata(f, abi.FUNCDATA_ArgLiveInfo)
-       liveIdx := pcdatavalue(f, abi.PCDATA_ArgLiveIndex, pc, nil)
+       liveIdx := pcdatavalue(f, abi.PCDATA_ArgLiveIndex, pc)
        startOffset := uint8(0xff) // smallest offset that needs liveness info (slots with a lower offset is always live)
        if liveInfo != nil {
                startOffset = *(*uint8)(liveInfo)
@@ -977,7 +974,7 @@ func traceback2(u *unwinder, showRuntime bool, skip, max int) (n, lastN int) {
        for ; u.valid(); u.next() {
                lastN = 0
                f := u.frame.fn
-               for iu, uf := newInlineUnwinder(f, u.symPC(), noEscapePtr(&u.cache)); uf.valid(); uf = iu.next(uf) {
+               for iu, uf := newInlineUnwinder(f, u.symPC()); uf.valid(); uf = iu.next(uf) {
                        sf := iu.srcFunc(uf)
                        callee := u.calleeFuncID
                        u.calleeFuncID = sf.funcID
@@ -1078,7 +1075,7 @@ func printAncestorTraceback(ancestor ancestorInfo) {
 // due to only have access to the pcs at the time of the caller
 // goroutine being created.
 func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) {
-       u, uf := newInlineUnwinder(f, pc, nil)
+       u, uf := newInlineUnwinder(f, pc)
        file, line := u.fileLine(uf)
        printFuncName(u.srcFunc(uf).name())
        print("(...)\n")