Skip to content

Commit 45137ab

Browse files
committed
stack: refactor Call
It's not costly to precalculate and it makes the code simpler, and reduces memory usage. Reduces largely the API surface.
1 parent be9cde2 commit 45137ab

9 files changed

Lines changed: 116 additions & 91 deletions

File tree

internal/ui.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (pf pathFormat) formatCall(c *stack.Call) string {
5555
}
5656
return fmt.Sprintf("%s:%d", c.SrcPath, c.Line)
5757
default:
58-
return fmt.Sprintf("%s:%d", c.SrcName(), c.Line)
58+
return fmt.Sprintf("%s:%d", c.SrcName, c.Line)
5959
}
6060
}
6161

internal/ui_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
package internal
66

77
import (
8+
"path"
9+
"path/filepath"
810
"strings"
911
"testing"
1012

@@ -169,6 +171,9 @@ func newFunc(s string) stack.Func {
169171

170172
func newCallLocal(f string, a stack.Args, s string, l int) stack.Call {
171173
c := stack.Call{Func: newFunc(f), Args: a, SrcPath: s, Line: l}
174+
// Do the equivalent of Call.init().
175+
c.SrcName = filepath.Base(c.SrcPath)
176+
c.DirSrc = path.Join(filepath.Base(c.SrcPath[:len(c.SrcPath)-len(c.SrcName)-1]), c.SrcName)
172177
const goroot = "/goroot/src/"
173178
const gopath = "/home/user/go/src/"
174179
const gopathmod = "/home/user/go/pkg/mod/"

stack/bucket.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ type Bucket struct {
7878
// First is true if this Bucket contains the first goroutine, e.g. the one
7979
// Signature that likely generated the panic() call, if any.
8080
First bool
81+
82+
// Disallow initialization with unnamed parameters.
83+
_ struct{}
8184
}
8285

8386
// less does reverse sort.

stack/context.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -742,8 +742,9 @@ func parseFile(c *Call, line string) (bool, error) {
742742
if err != nil {
743743
return true, fmt.Errorf("failed to parse int on line: %q", strings.TrimSpace(line))
744744
}
745-
c.SrcPath = match[1]
746-
c.Line = num
745+
// TODO(maruel): This returns a string slice inside line. We may want to
746+
// trim memory further.
747+
c.init(match[1], num)
747748
return true, nil
748749
}
749750
return false, nil

stack/context_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,11 +1597,11 @@ func identifyPanicwebSignature(t *testing.T, b *Bucket, pwebDir string) panicweb
15971597
t.Fatalf("suspicious: %#v", b)
15981598
return pstUnknown
15991599
}
1600-
if b.Stack.Calls[0].SrcName() != "internal.go" {
1600+
if b.Stack.Calls[0].SrcName != "internal.go" {
16011601
t.Fatalf("suspicious: %#v", b)
16021602
return pstUnknown
16031603
}
1604-
if b.CreatedBy.SrcName() != "server.go" {
1604+
if b.CreatedBy.SrcName != "server.go" {
16051605
t.Fatalf("suspicious: %#v", b)
16061606
return pstUnknown
16071607
}

stack/example_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func Example() {
4242
pkgLen := 0
4343
for _, bucket := range buckets {
4444
for _, line := range bucket.Signature.Stack.Calls {
45-
if l := len(fmt.Sprintf("%s:%d", line.SrcName(), line.Line)); l > srcLen {
45+
if l := len(fmt.Sprintf("%s:%d", line.SrcName, line.Line)); l > srcLen {
4646
srcLen = l
4747
}
4848
if l := len(filepath.Base(line.Func.ImportPath)); l > pkgLen {
@@ -62,7 +62,7 @@ func Example() {
6262
}
6363

6464
if bucket.CreatedBy.Func.DirName != "" {
65-
extra += fmt.Sprintf(" [Created by %s.%s @ %s:%d]", bucket.CreatedBy.Func.DirName, bucket.CreatedBy.Func.Name, bucket.CreatedBy.SrcName(), bucket.CreatedBy.Line)
65+
extra += fmt.Sprintf(" [Created by %s.%s @ %s:%d]", bucket.CreatedBy.Func.DirName, bucket.CreatedBy.Func.Name, bucket.CreatedBy.SrcName, bucket.CreatedBy.Line)
6666
}
6767
fmt.Printf("%d: %s%s\n", len(bucket.IDs), bucket.State, extra)
6868

@@ -71,7 +71,7 @@ func Example() {
7171
fmt.Printf(
7272
" %-*s %-*s %s(%s)\n",
7373
pkgLen, line.Func.DirName, srcLen,
74-
fmt.Sprintf("%s:%d", line.SrcName(), line.Line),
74+
fmt.Sprintf("%s:%d", line.SrcName, line.Line),
7575
line.Func.Name, &line.Args)
7676
}
7777
if bucket.Stack.Elided {

stack/source_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,8 @@ func zapPointers(t *testing.T, want, got *Stack) {
640640
func zapPaths(s *Stack) {
641641
for j := range s.Calls {
642642
s.Calls[j].SrcPath = filepath.Base(s.Calls[j].SrcPath)
643+
s.Calls[j].SrcName = ""
644+
s.Calls[j].DirSrc = ""
643645
s.Calls[j].LocalSrcPath = ""
644646
}
645647
}

stack/stack.go

Lines changed: 76 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"fmt"
1414
"net/url"
1515
"os"
16-
"path/filepath"
1716
"sort"
1817
"strings"
1918
"unicode"
@@ -236,63 +235,37 @@ func (a *Args) merge(r *Args) Args {
236235

237236
// Call is an item in the stack trace.
238237
type Call struct {
239-
// SrcPath is the full path name of the source file as seen in the trace.
240-
SrcPath string
241-
// LocalSrcPath is the full path name of the source file as seen in the host,
242-
// if found.
243-
LocalSrcPath string
244-
// Line is the line number.
245-
Line int
238+
// The following are initialized on the first line of the call stack.
246239
// Func is the fully qualified function name (encoded).
247240
Func Func
248241
// Args is the call arguments.
249242
Args Args
250243

244+
// The following are initialized on the second line of the call stack.
245+
// SrcPath is the full path name of the source file as seen in the trace.
246+
SrcPath string
247+
// Line is the line number.
248+
Line int
249+
// SrcName returns the base file name of the source file. It is a subset of
250+
// SrcPath.
251+
SrcName string
252+
// DirSrc is one directory plus the file name of the source file. It is a
253+
// subset of SrcPath.
254+
DirSrc string
255+
251256
// The following are only set if guesspaths is set to true in ParseDump().
252-
// IsStdlib is true if it is a Go standard library function. This includes
253-
// the 'go test' generated main executable.
254-
IsStdlib bool
257+
// LocalSrcPath is the full path name of the source file as seen in the host,
258+
// if found.
259+
LocalSrcPath string
255260
// RelSrcPath is the relative path to GOROOT or GOPATH. Only set when
256261
// Augment() is called.
257262
RelSrcPath string
258-
}
259-
260-
// equal returns true only if both calls are exactly equal.
261-
func (c *Call) equal(r *Call) bool {
262-
return c.Line == r.Line && c.Func.Complete == r.Func.Complete && c.SrcPath == r.SrcPath && c.Args.equal(&r.Args)
263-
}
264-
265-
// similar returns true if the two Call are equal or almost but not quite
266-
// equal.
267-
func (c *Call) similar(r *Call, similar Similarity) bool {
268-
return c.Line == r.Line && c.Func.Complete == r.Func.Complete && c.SrcPath == r.SrcPath && c.Args.similar(&r.Args, similar)
269-
}
270-
271-
// merge merges two similar Call, zapping out differences.
272-
func (c *Call) merge(r *Call) Call {
273-
return Call{
274-
SrcPath: c.SrcPath,
275-
LocalSrcPath: c.LocalSrcPath,
276-
Line: c.Line,
277-
Func: c.Func,
278-
Args: c.Args.merge(&r.Args),
279-
IsStdlib: c.IsStdlib,
280-
RelSrcPath: c.RelSrcPath,
281-
}
282-
}
283-
284-
// SrcName returns the base file name of the source file.
285-
func (c *Call) SrcName() string {
286-
return filepath.Base(c.SrcPath)
287-
}
263+
// IsStdlib is true if it is a Go standard library function. This includes
264+
// the 'go test' generated main executable.
265+
IsStdlib bool
288266

289-
// PkgSrc returns one directory plus the file name of the source file.
290-
//
291-
// Since the package name can differ from the package import path, the result
292-
// is incorrect when there's a mismatch between the directory name containing
293-
// the package and the package name.
294-
func (c *Call) PkgSrc() string {
295-
return pathJoin(filepath.Base(filepath.Dir(c.SrcPath)), c.SrcName())
267+
// Disallow initialization with unnamed parameters.
268+
_ struct{}
296269
}
297270

298271
// ImportPath returns the fully qualified package import path.
@@ -315,6 +288,20 @@ func (c *Call) ImportPath() string {
315288
return ""
316289
}
317290

291+
// Init initializes SrcPath, SrcName, DirName, Line and IsStdlib for test main.
292+
func (c *Call) init(srcPath string, line int) {
293+
c.SrcPath = srcPath
294+
c.Line = line
295+
if i := strings.LastIndexByte(c.SrcPath, '/'); i != -1 {
296+
c.SrcName = c.SrcPath[i+1:]
297+
if i = strings.LastIndexByte(c.SrcPath[:i], '/'); i != -1 {
298+
c.DirSrc = c.SrcPath[i+1:]
299+
}
300+
}
301+
// Consider _test/_testmain.go as stdlib since it's injected by "go test".
302+
c.IsStdlib = c.DirSrc == testMainSrc
303+
}
304+
318305
const testMainSrc = "_test" + string(os.PathSeparator) + "_testmain.go"
319306

320307
// updateLocations initializes LocalSrcPath, RelSrcPath and IsStdlib.
@@ -332,7 +319,7 @@ func (c *Call) updateLocations(goroot, localgoroot, localgomod, gomodImportPath
332319
c.RelSrcPath = c.SrcPath[len(prefix):]
333320
c.LocalSrcPath = pathJoin(localgoroot, "src", c.RelSrcPath)
334321
c.IsStdlib = true
335-
goto done
322+
return
336323
}
337324
}
338325
// Check GOPATH.
@@ -341,13 +328,13 @@ func (c *Call) updateLocations(goroot, localgoroot, localgomod, gomodImportPath
341328
if p := prefix + "/src/"; strings.HasPrefix(c.SrcPath, p) {
342329
c.RelSrcPath = c.SrcPath[len(p):]
343330
c.LocalSrcPath = pathJoin(dest, "src", c.RelSrcPath)
344-
goto done
331+
return
345332
}
346333
// For modules, the path has to be altered, as it contains the version.
347334
if p := prefix + "/pkg/mod/"; strings.HasPrefix(c.SrcPath, p) {
348335
c.RelSrcPath = c.SrcPath[len(p):]
349336
c.LocalSrcPath = pathJoin(dest, "pkg/mod", c.RelSrcPath)
350-
goto done
337+
return
351338
}
352339
}
353340
// Go module path detection only works with stack traces created on the local
@@ -356,13 +343,34 @@ func (c *Call) updateLocations(goroot, localgoroot, localgomod, gomodImportPath
356343
if prefix := localgomod + "/"; strings.HasPrefix(c.SrcPath, prefix) {
357344
c.RelSrcPath = gomodImportPath + "/" + c.SrcPath[len(prefix):]
358345
c.LocalSrcPath = c.SrcPath
359-
goto done
346+
return
360347
}
361348
}
362-
done:
363-
if !c.IsStdlib {
364-
// Consider _test/_testmain.go as stdlib since it's injected by "go test".
365-
c.IsStdlib = c.PkgSrc() == testMainSrc
349+
}
350+
351+
// equal returns true only if both calls are exactly equal.
352+
func (c *Call) equal(r *Call) bool {
353+
return c.Line == r.Line && c.Func.Complete == r.Func.Complete && c.SrcPath == r.SrcPath && c.Args.equal(&r.Args)
354+
}
355+
356+
// similar returns true if the two Call are equal or almost but not quite
357+
// equal.
358+
func (c *Call) similar(r *Call, similar Similarity) bool {
359+
return c.Line == r.Line && c.Func.Complete == r.Func.Complete && c.SrcPath == r.SrcPath && c.Args.similar(&r.Args, similar)
360+
}
361+
362+
// merge merges two similar Call, zapping out differences.
363+
func (c *Call) merge(r *Call) Call {
364+
return Call{
365+
Func: c.Func,
366+
Args: c.Args.merge(&r.Args),
367+
SrcPath: c.SrcPath,
368+
Line: c.Line,
369+
SrcName: c.SrcName,
370+
DirSrc: c.DirSrc,
371+
LocalSrcPath: c.LocalSrcPath,
372+
RelSrcPath: c.RelSrcPath,
373+
IsStdlib: c.IsStdlib,
366374
}
367375
}
368376

@@ -374,6 +382,9 @@ type Stack struct {
374382
// Elided is set when there's >100 items in Stack, currently hardcoded in
375383
// package runtime.
376384
Elided bool
385+
386+
// Disallow initialization with unnamed parameters.
387+
_ struct{}
377388
}
378389

379390
// equal returns true on if both call stacks are exactly equal.
@@ -462,16 +473,17 @@ func (s *Stack) less(r *Stack) bool {
462473
if s.Calls[x].Func.Complete > r.Calls[x].Func.Complete {
463474
return true
464475
}
465-
if s.Calls[x].PkgSrc() < r.Calls[x].PkgSrc() {
476+
if s.Calls[x].DirSrc < r.Calls[x].DirSrc {
466477
return true
467478
}
468-
if s.Calls[x].PkgSrc() > r.Calls[x].PkgSrc() {
479+
if s.Calls[x].DirSrc > r.Calls[x].DirSrc {
469480
return true
470481
}
471482
if s.Calls[x].Line < r.Calls[x].Line {
472483
return true
473484
}
474485
if s.Calls[x].Line > r.Calls[x].Line {
486+
// ??
475487
return true
476488
}
477489
}
@@ -519,6 +531,9 @@ type Signature struct {
519531
Stack Stack
520532
// Locked is set if the goroutine was locked to an OS thread.
521533
Locked bool
534+
535+
// Disallow initialization with unnamed parameters.
536+
_ struct{}
522537
}
523538

524539
// equal returns true only if both signatures are exactly equal.
@@ -615,6 +630,9 @@ type Goroutine struct {
615630
ID int
616631
// First is the goroutine first printed, normally the one that crashed.
617632
First bool
633+
634+
// Disallow initialization with unnamed parameters.
635+
_ struct{}
618636
}
619637

620638
// Private stuff.

0 commit comments

Comments
 (0)