Skip to content

Commit

Permalink
runtime: In Frames.Next, delay file/line lookup until just before return
Browse files Browse the repository at this point in the history
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 golang#32093:

name      old time/op  new time/op  delta
Helper-4   948ns ± 1%   836ns ± 3%  -11.89%  (p=0.000 n=9+9)

(golang#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 <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
randall77 committed May 20, 2019
1 parent 5a90306 commit c77a9e0
Showing 1 changed file with 17 additions and 3 deletions.
20 changes: 17 additions & 3 deletions src/runtime/symtab.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -111,16 +115,17 @@ 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
})
}

// Pop one frame from the frame list. Keep the rest.
// 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]
Expand All @@ -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
}

Expand All @@ -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
Expand Down

0 comments on commit c77a9e0

Please sign in to comment.