Skip to content

Commit

Permalink
Revamp how source listing is produced so it works for inlined functio…
Browse files Browse the repository at this point in the history 
…ns. (#599)

* Revamp how source listing is produced so it works for inlined functions.

Some other things improved due to these changes:
1. Produced output does not contain long runs of uninteresting source.
2. Speed of producing weblist page for a large binary goes from
   ~ 57s to ~ 5.5s.

* use keyed literals to satisfy extra checks

* Fix up file names for Windows (to use backslash instead of slash
as separator).

* Fix nil dereference when we attempt to close after encountering a missing object file

* Limit number of address ranges we process to avoid unbounded hangs

Stop printing address ranges after processing 25 of them. These ranges
are sorted by the number of samples that fell within them.

Change back to printing inner-most file:line next to an instruction
to reduce caller/callee confusion.

* Fix comment typo

Co-authored-by: Alexey Alexandrov <aalexand@users.noreply.github.com>
  • Loading branch information
@ghemawat @aalexand
ghemawat and aalexand authored Feb 26, 2021
1 parent c5db671 commit cbba55b
Show file tree
Hide file tree
Showing 8 changed files with 669 additions and 217 deletions.
6 changes: 5 additions & 1 deletion internal/driver/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func applyCommandOverrides(cmd string, outputFormat int, cfg config) config {
trim := cfg.Trim

switch cmd {
case "disasm", "weblist":
case "disasm":
trim = false
cfg.Granularity = "addresses"
// Force the 'noinlines' mode so that source locations for a given address
Expand All @@ -172,6 +172,10 @@ func applyCommandOverrides(cmd string, outputFormat int, cfg config) config {
// This is because the merge is done by address and in case of an inlined
// stack each of the inlined entries is a separate callgraph node.
cfg.NoInlines = true
case "weblist":
trim = false
cfg.Granularity = "addresses"
cfg.NoInlines = false // Need inline info to support call expansion
case "peek":
trim = false
case "list":
Expand Down
87 changes: 68 additions & 19 deletions internal/driver/driver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestParse(t *testing.T) {
{"dot,inuse_space,flat,tagfocus=30kb:,tagignore=1mb:2mb", "heap"},
{"disasm=line[13],addresses,flat", "cpu"},
{"peek=line.*01", "cpu"},
{"weblist=line[13],addresses,flat", "cpu"},
{"weblist=line(1000|3000)$,addresses,flat", "cpu"},
{"tags,tagfocus=400kb:", "heap_request"},
{"tags,tagfocus=+400kb:", "heap_request"},
{"dot", "long_name_funcs"},
Expand Down Expand Up @@ -1585,24 +1585,28 @@ func (*mockObjTool) Open(file string, start, limit, offset uint64) (plugin.ObjFi
}

func (m *mockObjTool) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
switch start {
case 0x1000:
return []plugin.Inst{
{Addr: 0x1000, Text: "instruction one", File: "file1000.src", Line: 1},
{Addr: 0x1001, Text: "instruction two", File: "file1000.src", Line: 1},
{Addr: 0x1002, Text: "instruction three", File: "file1000.src", Line: 2},
{Addr: 0x1003, Text: "instruction four", File: "file1000.src", Line: 1},
}, nil
case 0x3000:
return []plugin.Inst{
{Addr: 0x3000, Text: "instruction one"},
{Addr: 0x3001, Text: "instruction two"},
{Addr: 0x3002, Text: "instruction three"},
{Addr: 0x3003, Text: "instruction four"},
{Addr: 0x3004, Text: "instruction five"},
}, nil
const fn1 = "line1000"
const fn3 = "line3000"
const file1 = "testdata/file1000.src"
const file3 = "testdata/file3000.src"
data := []plugin.Inst{
{Addr: 0x1000, Text: "instruction one", Function: fn1, File: file1, Line: 1},
{Addr: 0x1001, Text: "instruction two", Function: fn1, File: file1, Line: 1},
{Addr: 0x1002, Text: "instruction three", Function: fn1, File: file1, Line: 2},
{Addr: 0x1003, Text: "instruction four", Function: fn1, File: file1, Line: 1},
{Addr: 0x3000, Text: "instruction one", Function: fn3, File: file3},
{Addr: 0x3001, Text: "instruction two", Function: fn3, File: file3},
{Addr: 0x3002, Text: "instruction three", Function: fn3, File: file3},
{Addr: 0x3003, Text: "instruction four", Function: fn3, File: file3},
{Addr: 0x3004, Text: "instruction five", Function: fn3, File: file3},
}
return nil, fmt.Errorf("unimplemented")
var result []plugin.Inst
for _, inst := range data {
if inst.Addr >= start && inst.Addr <= end {
result = append(result, inst)
}
}
return result, nil
}

type mockFile struct {
Expand Down Expand Up @@ -1630,7 +1634,52 @@ func (m *mockFile) BuildID() string {
// is in general a list of positions representing a call stack,
// with the leaf function first.
func (*mockFile) SourceLine(addr uint64) ([]plugin.Frame, error) {
return nil, fmt.Errorf("unimplemented")
// Return enough data to support the SourceLine() calls needed for
// weblist on cpuProfile() contents.
frame := func(fn, file string, line int) plugin.Frame {
return plugin.Frame{Func: fn, File: file, Line: line}
}
switch addr {
case 0x1000:
return []plugin.Frame{
frame("mangled1000", "testdata/file1000.src", 1),
}, nil
case 0x1001:
return []plugin.Frame{
frame("mangled1000", "testdata/file1000.src", 1),
}, nil
case 0x1002:
return []plugin.Frame{
frame("mangled1000", "testdata/file1000.src", 2),
}, nil
case 0x1003:
return []plugin.Frame{
frame("mangled1000", "testdata/file1000.src", 1),
}, nil
case 0x2000:
return []plugin.Frame{
frame("mangled2001", "testdata/file2000.src", 9),
frame("mangled2000", "testdata/file2000.src", 4),
}, nil
case 0x3000:
return []plugin.Frame{
frame("mangled3002", "testdata/file3000.src", 2),
frame("mangled3001", "testdata/file3000.src", 5),
frame("mangled3000", "testdata/file3000.src", 6),
}, nil
case 0x3001:
return []plugin.Frame{
frame("mangled3001", "testdata/file3000.src", 8),
frame("mangled3000", "testdata/file3000.src", 9),
}, nil
case 0x3002:
return []plugin.Frame{
frame("mangled3002", "testdata/file3000.src", 5),
frame("mangled3000", "testdata/file3000.src", 9),
}, nil
}

return nil, nil
}

// Symbols returns a list of symbols in the object file.
Expand Down
2 changes: 1 addition & 1 deletion internal/driver/interactive_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func TestInteractiveCommands(t *testing.T) {
"weblist find -test",
map[string]string{
"granularity": "addresses",
"noinlines": "true",
"noinlines": "false",
"nodecount": "0",
"sort": "flat",
"ignore": "test",
Expand Down
8 changes: 4 additions & 4 deletions internal/driver/testdata/pprof.cpu.flat.addresses.disasm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ Total: 1.12s
ROUTINE ======================== line1000
1.10s 1.10s (flat, cum) 98.21% of Total
1.10s 1.10s 1000: instruction one ;line1000 file1000.src:1
. . 1001: instruction two ;file1000.src:1
. . 1002: instruction three ;file1000.src:2
. . 1003: instruction four ;file1000.src:1
. . 1001: instruction two
. . 1002: instruction three ;line1000 file1000.src:2
. . 1003: instruction four ;line1000 file1000.src:1
ROUTINE ======================== line3000
10ms 1.12s (flat, cum) 100% of Total
10ms 1.01s 3000: instruction one ;line3000 file3000.src:6
. 100ms 3001: instruction two ;line3000 file3000.src:9
. 10ms 3002: instruction three
. . 3003: instruction four
. . 3003: instruction four ;line3000 file3000.src
. . 3004: instruction five
14 changes: 9 additions & 5 deletions internal/driver/testdata/pprof.cpu.flat.addresses.weblist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,18 @@ Duration: 10s, Total samples = 1.12s (11.20%)<br>Total: 1.12s</div><h2>line1000<
<span class=line> 3</span> <span class=nop> . . line3 </span>
<span class=line> 4</span> <span class=nop> . . line4 </span>
<span class=line> 5</span> <span class=nop> . . line5 </span>
<span class=line> 6</span> <span class=deadsrc> 10ms 1.01s line6 </span><span class=asm> 10ms 1.01s 3000: instruction one <span class=unimportant>file3000.src:6</span>
<span class=line> 6</span> <span class=deadsrc> 10ms 1.01s line6 </span><span class=asm> <span class=inlinesrc> line5 </span> <span class=unimportant>file3000.src:5</span>
<span class=inlinesrc> line2 </span> <span class=unimportant>file3000.src:2</span>
10ms 1.01s 3000: instruction one <span class=unimportant>file3000.src:2</span>
</span>
<span class=line> 7</span> <span class=nop> . . line7 </span>
<span class=line> 8</span> <span class=nop> . . line8 </span>
<span class=line> 9</span> <span class=deadsrc> . 110ms line9 </span><span class=asm> . 100ms 3001: instruction two <span class=unimportant>file3000.src:9</span>
. 10ms 3002: instruction three <span class=unimportant>file3000.src:9</span>
. . 3003: instruction four <span class=unimportant></span>
. . 3004: instruction five <span class=unimportant></span>
<span class=line> 9</span> <span class=deadsrc> . 110ms line9 </span><span class=asm> <span class=inlinesrc> line8 </span> <span class=unimportant>file3000.src:8</span>
. 100ms 3001: instruction two <span class=unimportant>file3000.src:8</span>
<span class=inlinesrc> line5 </span> <span class=unimportant>file3000.src:5</span>
. 10ms 3002: instruction three <span class=unimportant>file3000.src:5</span>
. . 3003: instruction four <span class=unimportant></span>
. . 3004: instruction five <span class=unimportant></span>
</span>
<span class=line> 10</span> <span class=nop> . . line0 </span>
<span class=line> 11</span> <span class=nop> . . line1 </span>
Expand Down
16 changes: 10 additions & 6 deletions internal/driver/webui_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,12 @@ func TestWebInterface(t *testing.T) {
testcases := []testCase{
{"/", []string{"F1", "F2", "F3", "testbin", "cpu"}, true},
{"/top", []string{`"Name":"F2","InlineLabel":"","Flat":200,"Cum":300,"FlatFormat":"200ms","CumFormat":"300ms"}`}, false},
{"/source?f=" + url.QueryEscape("F[12]"),
[]string{"F1", "F2", "300ms +line1"}, false},
{"/source?f=" + url.QueryEscape("F[12]"), []string{
"F1",
"F2",
`\. +300ms .*f1:asm`, // Cumulative count for F1
"200ms +300ms .*f2:asm", // Flat + cumulative count for F2
}, false},
{"/peek?f=" + url.QueryEscape("F[12]"),
[]string{"300ms.*F1", "200ms.*300ms.*F2"}, false},
{"/disasm?f=" + url.QueryEscape("F[12]"),
Expand Down Expand Up @@ -174,9 +178,9 @@ func (obj fakeObjTool) Open(file string, start, limit, offset uint64) (plugin.Ob

func (obj fakeObjTool) Disasm(file string, start, end uint64, intelSyntax bool) ([]plugin.Inst, error) {
return []plugin.Inst{
{Addr: addrBase + 0, Text: "f1:asm", Function: "F1"},
{Addr: addrBase + 10, Text: "f2:asm", Function: "F2"},
{Addr: addrBase + 20, Text: "d3:asm", Function: "F3"},
{Addr: addrBase + 10, Text: "f1:asm", Function: "F1", Line: 3},
{Addr: addrBase + 20, Text: "f2:asm", Function: "F2", Line: 11},
{Addr: addrBase + 30, Text: "d3:asm", Function: "F3", Line: 22},
}, nil
}

Expand All @@ -196,7 +200,7 @@ func makeFakeProfile() *profile.Profile {
{
ID: 1,
Start: addrBase,
Limit: addrBase + 10,
Limit: addrBase + 100,
Offset: 0,
File: "testbin",
HasFunctions: true,
Expand Down
Loading

0 comments on commit cbba55b

Please sign in to comment.