From c77a9e0aa5a8a238d68aa82b3b7e052a314a0060 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Sun, 19 May 2019 12:07:45 -0700 Subject: [PATCH] runtime: In Frames.Next, delay file/line lookup until just before return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That way we will never have to look up the file/line for the frame that's next to be returned when the user stops calling Next. For the benchmark from #32093: name old time/op new time/op delta Helper-4 948ns ± 1% 836ns ± 3% -11.89% (p=0.000 n=9+9) (#32093 was fixed with a more specific, and better, fix, but this fix is much more general.) Change-Id: I89e796f80c9706706d8d8b30eb14be3a8a442846 Reviewed-on: https://go-review.googlesource.com/c/go/+/178077 Run-TryBot: Keith Randall Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/runtime/symtab.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/runtime/symtab.go b/src/runtime/symtab.go index c0e8dc279b8..c2f32e0e5d2 100644 --- a/src/runtime/symtab.go +++ b/src/runtime/symtab.go @@ -52,6 +52,11 @@ type Frame struct { // if not known. If Func is not nil then Entry == // Func.Entry(). Entry uintptr + + // The runtime's internal view of the function. This field + // is set (funcInfo.valid() returns true) only for Go functions, + // not for C functions. + funcInfo funcInfo } // CallersFrames takes a slice of PC values returned by Callers and @@ -95,7 +100,6 @@ func (ci *Frames) Next() (frame Frame, more bool) { pc-- } name := funcname(funcInfo) - file, line := funcline1(funcInfo, pc, false) if inldata := funcdata(funcInfo, _FUNCDATA_InlTree); inldata != nil { inltree := (*[1 << 20]inlinedCall)(inldata) ix := pcdatavalue(funcInfo, _PCDATA_InlTreeIndex, pc, nil) @@ -111,9 +115,9 @@ func (ci *Frames) Next() (frame Frame, more bool) { PC: pc, Func: f, Function: name, - File: file, - Line: int(line), Entry: entry, + funcInfo: funcInfo, + // Note: File,Line set below }) } @@ -121,6 +125,7 @@ func (ci *Frames) Next() (frame Frame, more bool) { // Avoid allocation in the common case, which is 1 or 2 frames. switch len(ci.frames) { case 0: // In the rare case when there are no frames at all, we return Frame{}. + return case 1: frame = ci.frames[0] ci.frames = ci.frameStore[:0] @@ -133,6 +138,13 @@ func (ci *Frames) Next() (frame Frame, more bool) { ci.frames = ci.frames[1:] } more = len(ci.frames) > 0 + if frame.funcInfo.valid() { + // Compute file/line just before we need to return it, + // as it can be expensive. This avoids computing file/line + // for the Frame we find but don't return. See issue 32093. + file, line := funcline1(frame.funcInfo, frame.PC, false) + frame.File, frame.Line = file, int(line) + } return } @@ -157,6 +169,8 @@ func expandCgoFrames(pc uintptr) []Frame { File: gostring(arg.file), Line: int(arg.lineno), Entry: arg.entry, + // funcInfo is zero, which implies !funcInfo.valid(). + // That ensures that we use the File/Line info given here. }) if arg.more == 0 { break