runtime: replace all callback uses of gentraceback with unwinder

This is a really nice simplification for all of these call sites.

It also achieves a nice performance improvement for stack copying:

goos: linux
goarch: amd64
pkg: runtime
cpu: Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz
                       │   before    │                after                │
                       │   sec/op    │   sec/op     vs base                │
StackCopyPtr-48          89.25m ± 1%   79.78m ± 1%  -10.62% (p=0.000 n=20)
StackCopy-48             83.48m ± 2%   71.88m ± 1%  -13.90% (p=0.000 n=20)
StackCopyNoCache-48      2.504m ± 2%   2.195m ± 1%  -12.32% (p=0.000 n=20)
StackCopyWithStkobj-48   21.66m ± 1%   21.02m ± 2%   -2.95% (p=0.000 n=20)
geomean                  25.21m        22.68m       -10.04%

Updates #54466.

Change-Id: I31715b7b6efd65726940041d3052bb1c0a1186f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/468297
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
This commit is contained in:
Austin Clements 2023-02-13 16:20:54 -05:00
parent da384766a0
commit 2d99109cfc
6 changed files with 100 additions and 110 deletions

View file

@ -249,8 +249,7 @@ func dumpbv(cbv *bitvector, offset uintptr) {
} }
} }
func dumpframe(s *stkframe, arg unsafe.Pointer) bool { func dumpframe(s *stkframe, child *childInfo) {
child := (*childInfo)(arg)
f := s.fn f := s.fn
// Figure out what we can about our stack map // Figure out what we can about our stack map
@ -333,7 +332,7 @@ func dumpframe(s *stkframe, arg unsafe.Pointer) bool {
} else { } else {
child.args.n = -1 child.args.n = -1
} }
return true return
} }
func dumpgoroutine(gp *g) { func dumpgoroutine(gp *g) {
@ -369,7 +368,10 @@ func dumpgoroutine(gp *g) {
child.arglen = 0 child.arglen = 0
child.sp = nil child.sp = nil
child.depth = 0 child.depth = 0
gentraceback(pc, sp, lr, gp, 0, nil, 0x7fffffff, dumpframe, noescape(unsafe.Pointer(&child)), 0) var u unwinder
for u.initAt(pc, sp, lr, gp, 0); u.valid(); u.next() {
dumpframe(&u.frame, &child)
}
// dump defer & panic records // dump defer & panic records
for d := gp._defer; d != nil; d = d.link { for d := gp._defer; d != nil; d = d.link {

View file

@ -1397,15 +1397,6 @@ func dumpGCProg(p *byte) {
// Testing. // Testing.
func getgcmaskcb(frame *stkframe, ctxt unsafe.Pointer) bool {
target := (*stkframe)(ctxt)
if frame.sp <= target.sp && target.sp < frame.varp {
*target = *frame
return false
}
return true
}
// reflect_gcbits returns the GC type info for x, for testing. // reflect_gcbits returns the GC type info for x, for testing.
// The result is the bitmap entries (0 or 1), one entry per byte. // The result is the bitmap entries (0 or 1), one entry per byte.
// //
@ -1472,11 +1463,16 @@ func getgcmask(ep any) (mask []byte) {
// stack // stack
if gp := getg(); gp.m.curg.stack.lo <= uintptr(p) && uintptr(p) < gp.m.curg.stack.hi { if gp := getg(); gp.m.curg.stack.lo <= uintptr(p) && uintptr(p) < gp.m.curg.stack.hi {
var frame stkframe found := false
frame.sp = uintptr(p) var u unwinder
gentraceback(gp.m.curg.sched.pc, gp.m.curg.sched.sp, 0, gp.m.curg, 0, nil, 1000, getgcmaskcb, noescape(unsafe.Pointer(&frame)), 0) for u.initAt(gp.m.curg.sched.pc, gp.m.curg.sched.sp, 0, gp.m.curg, 0); u.valid(); u.next() {
if frame.fn.valid() { if u.frame.sp <= uintptr(p) && uintptr(p) < u.frame.varp {
locals, _, _ := frame.getStackMap(nil, false) found = true
break
}
}
if found {
locals, _, _ := u.frame.getStackMap(nil, false)
if locals.n == 0 { if locals.n == 0 {
return return
} }
@ -1484,7 +1480,7 @@ func getgcmask(ep any) (mask []byte) {
n := (*ptrtype)(unsafe.Pointer(t)).elem.size n := (*ptrtype)(unsafe.Pointer(t)).elem.size
mask = make([]byte, n/goarch.PtrSize) mask = make([]byte, n/goarch.PtrSize)
for i := uintptr(0); i < n; i += goarch.PtrSize { for i := uintptr(0); i < n; i += goarch.PtrSize {
off := (uintptr(p) + i - frame.varp + size) / goarch.PtrSize off := (uintptr(p) + i - u.frame.varp + size) / goarch.PtrSize
mask[i/goarch.PtrSize] = locals.ptrbit(off) mask[i/goarch.PtrSize] = locals.ptrbit(off)
} }
} }

View file

@ -797,11 +797,10 @@ func scanstack(gp *g, gcw *gcWork) int64 {
} }
// Scan the stack. Accumulate a list of stack objects. // Scan the stack. Accumulate a list of stack objects.
scanframe := func(frame *stkframe, unused unsafe.Pointer) bool { var u unwinder
scanframeworker(frame, &state, gcw) for u.init(gp, 0); u.valid(); u.next() {
return true scanframeworker(&u.frame, &state, gcw)
} }
gentraceback(^uintptr(0), ^uintptr(0), 0, gp, 0, nil, 0x7fffffff, scanframe, nil, 0)
// Find additional pointers that point into the stack from the heap. // Find additional pointers that point into the stack from the heap.
// Currently this includes defers and panics. See also function copystack. // Currently this includes defers and panics. See also function copystack.

View file

@ -642,77 +642,78 @@ func addOneOpenDeferFrame(gp *g, pc uintptr, sp unsafe.Pointer) {
sp = unsafe.Pointer(prevDefer.sp) sp = unsafe.Pointer(prevDefer.sp)
} }
systemstack(func() { systemstack(func() {
gentraceback(pc, uintptr(sp), 0, gp, 0, nil, 0x7fffffff, var u unwinder
func(frame *stkframe, unused unsafe.Pointer) bool { frames:
if prevDefer != nil && prevDefer.sp == frame.sp { for u.initAt(pc, uintptr(sp), 0, gp, 0); u.valid(); u.next() {
// Skip the frame for the previous defer that frame := &u.frame
// we just finished (and was used to set if prevDefer != nil && prevDefer.sp == frame.sp {
// where we restarted the stack scan) // Skip the frame for the previous defer that
return true // we just finished (and was used to set
// where we restarted the stack scan)
continue
}
f := frame.fn
fd := funcdata(f, _FUNCDATA_OpenCodedDeferInfo)
if fd == nil {
continue
}
// Insert the open defer record in the
// chain, in order sorted by sp.
d := gp._defer
var prev *_defer
for d != nil {
dsp := d.sp
if frame.sp < dsp {
break
} }
f := frame.fn if frame.sp == dsp {
fd := funcdata(f, _FUNCDATA_OpenCodedDeferInfo) if !d.openDefer {
if fd == nil { throw("duplicated defer entry")
return true
}
// Insert the open defer record in the
// chain, in order sorted by sp.
d := gp._defer
var prev *_defer
for d != nil {
dsp := d.sp
if frame.sp < dsp {
break
} }
if frame.sp == dsp { // Don't add any record past an
if !d.openDefer { // in-progress defer entry. We don't
throw("duplicated defer entry") // need it, and more importantly, we
} // want to keep the invariant that
// Don't add any record past an // there is no open defer entry
// in-progress defer entry. We don't // passed an in-progress entry (see
// need it, and more importantly, we // header comment).
// want to keep the invariant that if d.started {
// there is no open defer entry break frames
// passed an in-progress entry (see
// header comment).
if d.started {
return false
}
return true
} }
prev = d continue frames
d = d.link
}
if frame.fn.deferreturn == 0 {
throw("missing deferreturn")
} }
prev = d
d = d.link
}
if frame.fn.deferreturn == 0 {
throw("missing deferreturn")
}
d1 := newdefer() d1 := newdefer()
d1.openDefer = true d1.openDefer = true
d1._panic = nil d1._panic = nil
// These are the pc/sp to set after we've // These are the pc/sp to set after we've
// run a defer in this frame that did a // run a defer in this frame that did a
// recover. We return to a special // recover. We return to a special
// deferreturn that runs any remaining // deferreturn that runs any remaining
// defers and then returns from the // defers and then returns from the
// function. // function.
d1.pc = frame.fn.entry() + uintptr(frame.fn.deferreturn) d1.pc = frame.fn.entry() + uintptr(frame.fn.deferreturn)
d1.varp = frame.varp d1.varp = frame.varp
d1.fd = fd d1.fd = fd
// Save the SP/PC associated with current frame, // Save the SP/PC associated with current frame,
// so we can continue stack trace later if needed. // so we can continue stack trace later if needed.
d1.framepc = frame.pc d1.framepc = frame.pc
d1.sp = frame.sp d1.sp = frame.sp
d1.link = d d1.link = d
if prev == nil { if prev == nil {
gp._defer = d1 gp._defer = d1
} else { } else {
prev.link = d1 prev.link = d1
} }
// Stop stack scanning after adding one open defer record // Stop stack scanning after adding one open defer record
return false break
}, }
nil, 0)
}) })
} }

View file

@ -649,11 +649,10 @@ func adjustpointers(scanp unsafe.Pointer, bv *bitvector, adjinfo *adjustinfo, f
} }
// Note: the argument/return area is adjusted by the callee. // Note: the argument/return area is adjusted by the callee.
func adjustframe(frame *stkframe, arg unsafe.Pointer) bool { func adjustframe(frame *stkframe, adjinfo *adjustinfo) {
adjinfo := (*adjustinfo)(arg)
if frame.continpc == 0 { if frame.continpc == 0 {
// Frame is dead. // Frame is dead.
return true return
} }
f := frame.fn f := frame.fn
if stackDebug >= 2 { if stackDebug >= 2 {
@ -663,7 +662,7 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool {
// A special routine at the bottom of stack of a goroutine that does a systemstack call. // A special routine at the bottom of stack of a goroutine that does a systemstack call.
// We will allow it to be copied even though we don't // We will allow it to be copied even though we don't
// have full GC info for it (because it is written in asm). // have full GC info for it (because it is written in asm).
return true return
} }
locals, args, objs := frame.getStackMap(&adjinfo.cache, true) locals, args, objs := frame.getStackMap(&adjinfo.cache, true)
@ -736,8 +735,6 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool {
} }
} }
} }
return true
} }
func adjustctxt(gp *g, adjinfo *adjustinfo) { func adjustctxt(gp *g, adjinfo *adjustinfo) {
@ -931,7 +928,10 @@ func copystack(gp *g, newsize uintptr) {
gp.stktopsp += adjinfo.delta gp.stktopsp += adjinfo.delta
// Adjust pointers in the new stack. // Adjust pointers in the new stack.
gentraceback(^uintptr(0), ^uintptr(0), 0, gp, 0, nil, 0x7fffffff, adjustframe, noescape(unsafe.Pointer(&adjinfo)), 0) var u unwinder
for u.init(gp, 0); u.valid(); u.next() {
adjustframe(&u.frame, &adjinfo)
}
// free old stack // free old stack
if stackPoisonCopy != 0 { if stackPoisonCopy != 0 {

View file

@ -542,16 +542,16 @@ func (u *unwinder) finishInternal() {
} }
// Generic traceback. Handles runtime stack prints (pcbuf == nil), // Generic traceback. Handles runtime stack prints (pcbuf == nil),
// the runtime.Callers function (pcbuf != nil), as well as the garbage // and the runtime.Callers function (pcbuf != nil).
// collector (callback != nil). A little clunky to merge these, but avoids // A little clunky to merge these, but avoids
// duplicating the code and all its subtlety. // duplicating the code and all its subtlety.
// //
// The skip argument is only valid with pcbuf != nil and counts the number // The skip argument is only valid with pcbuf != nil and counts the number
// of logical frames to skip rather than physical frames (with inlining, a // of logical frames to skip rather than physical frames (with inlining, a
// PC in pcbuf can represent multiple calls). // PC in pcbuf can represent multiple calls).
func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) int { func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) int {
if skip > 0 && callback != nil { if callback != nil {
throw("gentraceback callback cannot be used with non-zero skip") throw("callback argument no longer supported")
} }
// Translate flags // Translate flags
@ -559,7 +559,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
printing := pcbuf == nil && callback == nil printing := pcbuf == nil && callback == nil
if printing { if printing {
uflags |= unwindPrintErrors uflags |= unwindPrintErrors
} else if callback == nil { } else {
uflags |= unwindSilentErrors uflags |= unwindSilentErrors
} }
if flags&_TraceTrap != 0 { if flags&_TraceTrap != 0 {
@ -581,12 +581,6 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
frame := &u.frame frame := &u.frame
f := frame.fn f := frame.fn
if callback != nil {
if !callback((*stkframe)(noescape(unsafe.Pointer(frame))), v) {
return n
}
}
// Backup to the CALL instruction to read inlining info // Backup to the CALL instruction to read inlining info
// //
// Normally, pc is a return address. In that case, we want to look up // Normally, pc is a return address. In that case, we want to look up
@ -670,9 +664,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
u.cgoCtxt-- u.cgoCtxt--
// skip only applies to Go frames. // skip only applies to Go frames.
// callback != nil only used when we only care if skip == 0 {
// about Go frames.
if skip == 0 && callback == nil {
n = tracebackCgoContext(pcbuf, printing, ctxt, n, max) n = tracebackCgoContext(pcbuf, printing, ctxt, n, max)
} }
} }