From 0776b4675bf467ff22e7b986e260ec38e881235c Mon Sep 17 00:00:00 2001 From: orie Date: Wed, 2 May 2018 13:44:06 -0700 Subject: [PATCH 1/7] moved the showFrom filter from branch filter2 to branch filters3. It is implemented as a separate function with it's own test. --- profile/filter.go | 91 ++++++++++++++++++++++++++++ profile/filter_test.go | 131 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 221 insertions(+), 1 deletion(-) diff --git a/profile/filter.go b/profile/filter.go index e68e93ae..a9d83d70 100644 --- a/profile/filter.go +++ b/profile/filter.go @@ -74,6 +74,97 @@ func (p *Profile) FilterSamplesByName(focus, ignore, hide, show *regexp.Regexp) return } +// ShowFrom drops all stack frames before the first match from the root and +// returns whether a match was found. If showFrom is nil it returns false and +// does not modify the profile. +// +// Example: consider a sample with frames [A, B, C], where A is the root. +// ShowFrom(nil) returns false and has frames [A, B, C]. +// ShowFrom(A) returns true and has frames [A, B, C]. +// ShowFrom(B) returns true and has frames [B, C]. +// ShowFrom(C) returns true and has frames [C]. +// ShowFrom(D) returns false and the sample is dropped because no frames remain. +func (p *Profile) ShowFrom(showFrom *regexp.Regexp) (matched bool) { + if showFrom == nil { + return false + } + + // showFromLocs stores location IDs that matched ShowFrom. + showFromLocs := make(map[uint64]bool) + + // Apply to locations. + for _, location := range p.Location { + if filterShowFromLocation(location, showFrom) { + showFromLocs[location.ID] = true + matched = true + } + } + + // Apply to samples and paths + filteredSamples := make([]*Sample, 0, len(p.Sample)) + for _, sample := range p.Sample { + if i := lastLocationIndex(sample.Location, showFromLocs); i >= 0 { + sample.Location = sample.Location[0:i+1] + filteredSamples = append(filteredSamples, sample) + } + } + + p.Sample = filteredSamples + return +} + +// filterShowFromLocation tests a showFrom regex against a location and removes +// lines after the last match. If the mapping is matched, then all lines are +// kept. +func filterShowFromLocation(location *Location, showFrom *regexp.Regexp) (matched bool) { + if showFrom == nil { + return false + } + + if location.matchesMapping(showFrom) { + return true + } + + if i := location.lastMatchedLineIndex(showFrom); i >= 0 { + matched = true + location.Line = location.Line[0:i+1] + } + + return +} + +// lastLocationIndex returns the index of the last location who's ID is in the +// map. +func lastLocationIndex(path []*Location, matchedIDs map[uint64]bool) int { + for i := len(path) - 1; i >= 0; i-- { + if matchedIDs[path[i].ID] { + return i + } + } + return -1 +} + +// lastMatchedLineIndex returns the index of the last line that matches a regex, +// or -1 if no match is found. +func (loc *Location) lastMatchedLineIndex(re *regexp.Regexp) int { + for i := len(loc.Line) - 1; i >= 0; i-- { + if fn := loc.Line[i].Function; fn != nil { + if re.MatchString(fn.Name) || re.MatchString(fn.Filename) { + return i + } + } + } + return -1 +} + +// matchesMapping returns whether a regex matches a location's mapping. +func (loc *Location) matchesMapping(re *regexp.Regexp) bool { + if m := loc.Mapping; m != nil && re != nil && re.MatchString(m.File) { + return true + } + return false +} + // FilterTagsByName filters the tags in a profile and only keeps // tags that match show and not hide. func (p *Profile) FilterTagsByName(show, hide *regexp.Regexp) (sm, hm bool) { diff --git a/profile/filter_test.go b/profile/filter_test.go index c25887a4..46dc390e 100644 --- a/profile/filter_test.go +++ b/profile/filter_test.go @@ -101,7 +101,29 @@ var inlinesProfile = &Profile{ }, } -func TestFilter(t *testing.T) { +var emptyLinesLocs = []*Location{ + {ID: 1, Mapping: mappings[0], Address: 0x1000, Line: []Line{{Function: functions[0], Line: 1}, {Function: functions[1], Line: 1}}}, + {ID: 2, Mapping: mappings[0], Address: 0x2000, Line: []Line{}}, + {ID: 3, Mapping: mappings[1], Address: 0x2000, Line: []Line{}}, +} + +var emptyLinesProfile = &Profile{ + TimeNanos: 10000, + PeriodType: &ValueType{Type: "cpu", Unit: "milliseconds"}, + Period: 1, + DurationNanos: 10e9, + SampleType: []*ValueType{{Type: "samples", Unit: "count"}}, + Mapping: mappings, + Function: functions, + Location: emptyLinesLocs, + Sample: []*Sample{ + {Value: []int64{1}, Location: []*Location{emptyLinesLocs[0], emptyLinesLocs[1]}}, + {Value: []int64{2}, Location: []*Location{emptyLinesLocs[2]}}, + {Value: []int64{3}, Location: []*Location{}}, + }, +} + +func TestFilterSamplesByName(t *testing.T) { for _, tc := range []struct { // name is the name of the test case. name string @@ -392,6 +414,113 @@ func TestFilter(t *testing.T) { } } +func TestShowFrom(t *testing.T) { + for _, tc := range []struct { + // name is the name of the test case. + name string + // profile is the profile to filter. + profile *Profile + // showFrom is the showFrom filter. + showFrom *regexp.Regexp + // wantMatch is the expected return value. + wantMatch bool + // wantSampleFuncs contains expected stack functions and sample value after + // filtering, in the same order as in the profile. The format is as + // returned by sampleFuncs function below, which is "callee caller: ". + wantSampleFuncs []string + }{ + { + name: "nil showFrom keeps all frames", + profile: noInlinesProfile, + wantMatch: false, + wantSampleFuncs: allNoInlinesSampleFuncs, + }, + { + name: "showFrom with no matches drops all samples", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("unknown"), + wantMatch: false, + }, + { + name: "showFrom matches function names", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("fun1"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun0 fun1: 1", + "fun4 fun5 fun1: 2", + "fun9 fun4 fun10: 4", + }, + }, + { + name: "showFrom matches file names", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("file1"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun0 fun1: 1", + "fun4 fun5 fun1: 2", + "fun9 fun4 fun10: 4", + }, + }, + { + name: "showFrom matches mapping names", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("map1"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun9 fun4 fun10: 4", + }, + }, + { + name: "showFrom matches inline functions", + profile: inlinesProfile, + showFrom: regexp.MustCompile("fun0|fun5"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun0: 1", + "fun4 fun5: 2", + }, + }, + { + name: "showFrom keeps all lines when matching mapping and function", + profile: inlinesProfile, + showFrom: regexp.MustCompile("map0|fun5"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun0 fun1 fun2 fun3: 1", + "fun4 fun5 fun6: 2", + }, + }, + { + name: "showFrom matches location with empty lines", + profile: emptyLinesProfile, + showFrom: regexp.MustCompile("map1"), + wantMatch: true, + wantSampleFuncs: []string{ + ": 2", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + p := tc.profile.Copy() + gotMatch := p.ShowFrom(tc.showFrom) + + if gotMatch != tc.wantMatch { + t.Errorf("match got %+v want %+v", gotMatch, tc.wantMatch) + } + + if got, want := strings.Join(sampleFuncs(p), "\n")+"\n", strings.Join(tc.wantSampleFuncs, "\n")+"\n"; got != want { + diff, err := proftest.Diff([]byte(want), []byte(got)) + if err != nil { + t.Fatalf("failed to get diff: %v", err) + } + t.Errorf("profile samples got diff(want->got):\n%s", diff) + } + }) + } +} + // sampleFuncs returns a slice of strings where each string represents one // profile sample in the format " : ". This allows // the expected values for test cases to be specifed in human-readable strings. From 331aae209c7095297444058d392f0f65c7f76135 Mon Sep 17 00:00:00 2001 From: orie Date: Thu, 3 May 2018 10:17:56 -0700 Subject: [PATCH 2/7] added show_from command line flag and driver test. --- internal/driver/commands.go | 4 ++++ internal/driver/driver.go | 2 +- internal/driver/driver_focus.go | 4 ++++ internal/driver/driver_test.go | 5 +++++ .../testdata/pprof.cpu.cum.lines.tree.show_from | 16 ++++++++++++++++ 5 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from diff --git a/internal/driver/commands.go b/internal/driver/commands.go index 80658314..bc0b6950 100644 --- a/internal/driver/commands.go +++ b/internal/driver/commands.go @@ -185,6 +185,10 @@ var pprofVariables = variables{ "Only show nodes matching regexp", "If set, only show nodes that match this location.", "Matching includes the function name, filename or object name.")}, + "show_from": &variable{stringKind, "", "", helpText( + "Drops nodes above the highest matched node of every sample.", + "If set, any frames above the highest match are dropped from every sample.", + "Matching includes the function name, filename or object name.")}, "tagfocus": &variable{stringKind, "", "", helpText( "Restricts to samples with tags in range or matched by regexp", "Use name=value syntax to limit the matching to a specific tag.", diff --git a/internal/driver/driver.go b/internal/driver/driver.go index 0a14c67a..da951597 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -242,7 +242,7 @@ func reportOptions(p *profile.Profile, numLabelUnits map[string]string, vars var } var filters []string - for _, k := range []string{"focus", "ignore", "hide", "show", "tagfocus", "tagignore", "tagshow", "taghide"} { + for _, k := range []string{"focus", "ignore", "hide", "show", "show_from", "tagfocus", "tagignore", "tagshow", "taghide"} { v := vars[k].value if v != "" { filters = append(filters, k+"="+v) diff --git a/internal/driver/driver_focus.go b/internal/driver/driver_focus.go index ba5b502a..b23ee810 100644 --- a/internal/driver/driver_focus.go +++ b/internal/driver/driver_focus.go @@ -33,6 +33,7 @@ func applyFocus(prof *profile.Profile, numLabelUnits map[string]string, v variab ignore, err := compileRegexOption("ignore", v["ignore"].value, err) hide, err := compileRegexOption("hide", v["hide"].value, err) show, err := compileRegexOption("show", v["show"].value, err) + showfrom, err := compileRegexOption("show_from", v["show_from"].value, err) tagfocus, err := compileTagFilter("tagfocus", v["tagfocus"].value, numLabelUnits, ui, err) tagignore, err := compileTagFilter("tagignore", v["tagignore"].value, numLabelUnits, ui, err) prunefrom, err := compileRegexOption("prune_from", v["prune_from"].value, err) @@ -46,6 +47,9 @@ func applyFocus(prof *profile.Profile, numLabelUnits map[string]string, v variab warnNoMatches(hide == nil || hm, "Hide", ui) warnNoMatches(show == nil || hnm, "Show", ui) + sfm := prof.ShowFrom(showfrom) + warnNoMatches(showfrom == nil || sfm, "ShowFrom", ui) + tfm, tim := prof.FilterSamplesByTag(tagfocus, tagignore) warnNoMatches(tagfocus == nil || tfm, "TagFocus", ui) warnNoMatches(tagignore == nil || tim, "TagIgnore", ui) diff --git a/internal/driver/driver_test.go b/internal/driver/driver_test.go index 06219eae..795b8f96 100644 --- a/internal/driver/driver_test.go +++ b/internal/driver/driver_test.go @@ -65,6 +65,7 @@ func TestParse(t *testing.T) { {"topproto,lines,cum,hide=mangled[X3]0", "cpu"}, {"tree,lines,cum,focus=[24]00", "heap"}, {"tree,relative_percentages,cum,focus=[24]00", "heap"}, + {"tree,lines,cum,show_from=[2]00", "cpu"}, {"callgrind", "cpu"}, {"callgrind,call_tree", "cpu"}, {"callgrind", "heap"}, @@ -247,6 +248,7 @@ func testSourceURL(port int) string { // solutionFilename returns the name of the solution file for the test func solutionFilename(source string, f *testFlags) string { + fmt.Printf("solution flags: %v\n", f) name := []string{"pprof", strings.TrimPrefix(source, testSourceURL(8000))} name = addString(name, f, []string{"flat", "cum"}) name = addString(name, f, []string{"functions", "files", "lines", "addresses"}) @@ -261,6 +263,9 @@ func solutionFilename(source string, f *testFlags) string { if f.strings["ignore"] != "" || f.strings["tagignore"] != "" { name = append(name, "ignore") } + if f.strings["show_from"] != "" { + name = append(name, "show_from") + } name = addString(name, f, []string{"hide", "show"}) if f.strings["unit"] != "minimum" { name = addString(name, f, []string{"unit"}) diff --git a/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from b/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from new file mode 100644 index 00000000..fb3297e0 --- /dev/null +++ b/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from @@ -0,0 +1,16 @@ +Active filters: + show_from=[2]00 +Showing nodes accounting for 1.01s, 100% of 1.01s total +----------------------------------------------------------+------------- + flat flat% sum% cum cum% calls calls% + context +----------------------------------------------------------+------------- + 0 0% 0% 1.01s 100% | line2000 testdata/file2000.src:4 + 1.01s 100% | line2001 testdata/file2000.src:9 (inline) +----------------------------------------------------------+------------- + 1.01s 100% | line2000 testdata/file2000.src:4 (inline) + 0.01s 0.99% 0.99% 1.01s 100% | line2001 testdata/file2000.src:9 + 1s 99.01% | line1000 testdata/file1000.src:1 +----------------------------------------------------------+------------- + 1s 100% | line2001 testdata/file2000.src:9 + 1s 99.01% 100% 1s 99.01% | line1000 testdata/file1000.src:1 +----------------------------------------------------------+------------- From 75fee66429c8c9c878cdfe9d19c072471aedc3e8 Mon Sep 17 00:00:00 2001 From: orie Date: Thu, 3 May 2018 14:33:01 -0700 Subject: [PATCH 3/7] added show_from handling to command overrides to avoid applying it for raw and proto commands. updated the driver test case to match demangled function name. fixed code formatting. --- internal/driver/driver.go | 5 ++ internal/driver/driver_test.go | 2 +- .../pprof.cpu.cum.lines.tree.show_from | 10 ++-- profile/filter.go | 4 +- profile/filter_test.go | 58 +++++++++---------- 5 files changed, 42 insertions(+), 37 deletions(-) diff --git a/internal/driver/driver.go b/internal/driver/driver.go index da951597..9d4b95da 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -151,10 +151,12 @@ func generateReport(p *profile.Profile, cmd []string, vars variables, o *plugin. func applyCommandOverrides(cmd []string, v variables) variables { trim, focus, tagfocus, hide := v["trim"].boolValue(), true, true, true + showFrom := true switch cmd[0] { case "proto", "raw": trim, focus, tagfocus, hide = false, false, false, false + showFrom = false v.set("addresses", "t") case "callgrind", "kcachegrind": trim = false @@ -193,6 +195,9 @@ func applyCommandOverrides(cmd []string, v variables) variables { v.set("hide", "") v.set("show", "") } + if !showFrom { + v.set("show_from", "") + } return v } diff --git a/internal/driver/driver_test.go b/internal/driver/driver_test.go index 795b8f96..c7b6d5f7 100644 --- a/internal/driver/driver_test.go +++ b/internal/driver/driver_test.go @@ -65,7 +65,7 @@ func TestParse(t *testing.T) { {"topproto,lines,cum,hide=mangled[X3]0", "cpu"}, {"tree,lines,cum,focus=[24]00", "heap"}, {"tree,relative_percentages,cum,focus=[24]00", "heap"}, - {"tree,lines,cum,show_from=[2]00", "cpu"}, + {"tree,lines,cum,show_from=line2", "cpu"}, {"callgrind", "cpu"}, {"callgrind,call_tree", "cpu"}, {"callgrind", "heap"}, diff --git a/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from b/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from index fb3297e0..112b49b3 100644 --- a/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from +++ b/internal/driver/testdata/pprof.cpu.cum.lines.tree.show_from @@ -1,16 +1,16 @@ Active filters: - show_from=[2]00 -Showing nodes accounting for 1.01s, 100% of 1.01s total + show_from=line2 +Showing nodes accounting for 1.01s, 90.18% of 1.12s total ----------------------------------------------------------+------------- flat flat% sum% cum cum% calls calls% + context ----------------------------------------------------------+------------- - 0 0% 0% 1.01s 100% | line2000 testdata/file2000.src:4 + 0 0% 0% 1.01s 90.18% | line2000 testdata/file2000.src:4 1.01s 100% | line2001 testdata/file2000.src:9 (inline) ----------------------------------------------------------+------------- 1.01s 100% | line2000 testdata/file2000.src:4 (inline) - 0.01s 0.99% 0.99% 1.01s 100% | line2001 testdata/file2000.src:9 + 0.01s 0.89% 0.89% 1.01s 90.18% | line2001 testdata/file2000.src:9 1s 99.01% | line1000 testdata/file1000.src:1 ----------------------------------------------------------+------------- 1s 100% | line2001 testdata/file2000.src:9 - 1s 99.01% 100% 1s 99.01% | line1000 testdata/file1000.src:1 + 1s 89.29% 90.18% 1s 89.29% | line1000 testdata/file1000.src:1 ----------------------------------------------------------+------------- diff --git a/profile/filter.go b/profile/filter.go index a9d83d70..e2ed4bab 100644 --- a/profile/filter.go +++ b/profile/filter.go @@ -104,7 +104,7 @@ func (p *Profile) ShowFrom(showFrom *regexp.Regexp) (matched bool) { filteredSamples := make([]*Sample, 0, len(p.Sample)) for _, sample := range p.Sample { if i := lastLocationIndex(sample.Location, showFromLocs); i >= 0 { - sample.Location = sample.Location[0:i+1] + sample.Location = sample.Location[0 : i+1] filteredSamples = append(filteredSamples, sample) } } @@ -127,7 +127,7 @@ func filterShowFromLocation(location *Location, showFrom *regexp.Regexp) (matche if i := location.lastMatchedLineIndex(showFrom); i >= 0 { matched = true - location.Line = location.Line[0:i+1] + location.Line = location.Line[0 : i+1] } return diff --git a/profile/filter_test.go b/profile/filter_test.go index 46dc390e..e637472c 100644 --- a/profile/filter_test.go +++ b/profile/filter_test.go @@ -432,20 +432,20 @@ func TestShowFrom(t *testing.T) { { name: "nil showFrom keeps all frames", profile: noInlinesProfile, - wantMatch: false, + wantMatch: false, wantSampleFuncs: allNoInlinesSampleFuncs, }, { - name: "showFrom with no matches drops all samples", - profile: noInlinesProfile, - showFrom: regexp.MustCompile("unknown"), - wantMatch: false, + name: "showFrom with no matches drops all samples", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("unknown"), + wantMatch: false, }, { - name: "showFrom matches function names", - profile: noInlinesProfile, - showFrom: regexp.MustCompile("fun1"), - wantMatch: true, + name: "showFrom matches function names", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("fun1"), + wantMatch: true, wantSampleFuncs: []string{ "fun0 fun1: 1", "fun4 fun5 fun1: 2", @@ -453,10 +453,10 @@ func TestShowFrom(t *testing.T) { }, }, { - name: "showFrom matches file names", - profile: noInlinesProfile, - showFrom: regexp.MustCompile("file1"), - wantMatch: true, + name: "showFrom matches file names", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("file1"), + wantMatch: true, wantSampleFuncs: []string{ "fun0 fun1: 1", "fun4 fun5 fun1: 2", @@ -464,39 +464,39 @@ func TestShowFrom(t *testing.T) { }, }, { - name: "showFrom matches mapping names", - profile: noInlinesProfile, - showFrom: regexp.MustCompile("map1"), - wantMatch: true, + name: "showFrom matches mapping names", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("map1"), + wantMatch: true, wantSampleFuncs: []string{ "fun9 fun4 fun10: 4", }, }, { - name: "showFrom matches inline functions", - profile: inlinesProfile, - showFrom: regexp.MustCompile("fun0|fun5"), - wantMatch: true, + name: "showFrom matches inline functions", + profile: inlinesProfile, + showFrom: regexp.MustCompile("fun0|fun5"), + wantMatch: true, wantSampleFuncs: []string{ "fun0: 1", "fun4 fun5: 2", }, }, { - name: "showFrom keeps all lines when matching mapping and function", - profile: inlinesProfile, - showFrom: regexp.MustCompile("map0|fun5"), - wantMatch: true, + name: "showFrom keeps all lines when matching mapping and function", + profile: inlinesProfile, + showFrom: regexp.MustCompile("map0|fun5"), + wantMatch: true, wantSampleFuncs: []string{ "fun0 fun1 fun2 fun3: 1", "fun4 fun5 fun6: 2", }, }, { - name: "showFrom matches location with empty lines", - profile: emptyLinesProfile, - showFrom: regexp.MustCompile("map1"), - wantMatch: true, + name: "showFrom matches location with empty lines", + profile: emptyLinesProfile, + showFrom: regexp.MustCompile("map1"), + wantMatch: true, wantSampleFuncs: []string{ ": 2", }, From 30bb8e26fcfddcfd89cfd161ab686b1b637be5db Mon Sep 17 00:00:00 2001 From: orie Date: Fri, 4 May 2018 09:42:17 -0700 Subject: [PATCH 4/7] review feedback round 1. --- internal/driver/commands.go | 4 +- internal/driver/driver.go | 18 +++---- internal/driver/driver_test.go | 1 - profile/filter.go | 86 ++++++++++++---------------------- profile/filter_test.go | 33 +++++++++---- 5 files changed, 63 insertions(+), 79 deletions(-) diff --git a/internal/driver/commands.go b/internal/driver/commands.go index bc0b6950..e36ce7ca 100644 --- a/internal/driver/commands.go +++ b/internal/driver/commands.go @@ -186,8 +186,8 @@ var pprofVariables = variables{ "If set, only show nodes that match this location.", "Matching includes the function name, filename or object name.")}, "show_from": &variable{stringKind, "", "", helpText( - "Drops nodes above the highest matched node of every sample.", - "If set, any frames above the highest match are dropped from every sample.", + "Drops functions above the highest matched frame.", + "If set, all frames above the highest match are dropped from every sample.", "Matching includes the function name, filename or object name.")}, "tagfocus": &variable{stringKind, "", "", helpText( "Restricts to samples with tags in range or matched by regexp", diff --git a/internal/driver/driver.go b/internal/driver/driver.go index 9d4b95da..51deef29 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -150,13 +150,11 @@ func generateReport(p *profile.Profile, cmd []string, vars variables, o *plugin. } func applyCommandOverrides(cmd []string, v variables) variables { - trim, focus, tagfocus, hide := v["trim"].boolValue(), true, true, true - showFrom := true + trim, tagfocus, filter := v["trim"].boolValue(), true, true switch cmd[0] { case "proto", "raw": - trim, focus, tagfocus, hide = false, false, false, false - showFrom = false + trim, tagfocus, filter = false, false, false v.set("addresses", "t") case "callgrind", "kcachegrind": trim = false @@ -165,7 +163,7 @@ func applyCommandOverrides(cmd []string, v variables) variables { trim = false v.set("addressnoinlines", "t") case "peek": - trim, focus, hide = false, false, false + trim, filter = false, false case "list": v.set("nodecount", "0") v.set("lines", "t") @@ -183,19 +181,15 @@ func applyCommandOverrides(cmd []string, v variables) variables { v.set("nodefraction", "0") v.set("edgefraction", "0") } - if !focus { - v.set("focus", "") - v.set("ignore", "") - } if !tagfocus { v.set("tagfocus", "") v.set("tagignore", "") } - if !hide { + if !filter { + v.set("focus", "") + v.set("ignore", "") v.set("hide", "") v.set("show", "") - } - if !showFrom { v.set("show_from", "") } return v diff --git a/internal/driver/driver_test.go b/internal/driver/driver_test.go index c7b6d5f7..309e9950 100644 --- a/internal/driver/driver_test.go +++ b/internal/driver/driver_test.go @@ -248,7 +248,6 @@ func testSourceURL(port int) string { // solutionFilename returns the name of the solution file for the test func solutionFilename(source string, f *testFlags) string { - fmt.Printf("solution flags: %v\n", f) name := []string{"pprof", strings.TrimPrefix(source, testSourceURL(8000))} name = addString(name, f, []string{"flat", "cum"}) name = addString(name, f, []string{"functions", "files", "lines", "addresses"}) diff --git a/profile/filter.go b/profile/filter.go index e2ed4bab..da5d24d3 100644 --- a/profile/filter.go +++ b/profile/filter.go @@ -74,74 +74,56 @@ func (p *Profile) FilterSamplesByName(focus, ignore, hide, show *regexp.Regexp) return } -// ShowFrom drops all stack frames before the first match from the root and -// returns whether a match was found. If showFrom is nil it returns false and -// does not modify the profile. +// ShowFrom drops all stack frames above the highest matching frame and returns +// whether a match was found. If showFrom is nil it returns false and does not +// modify the profile. // -// Example: consider a sample with frames [A, B, C], where A is the root. -// ShowFrom(nil) returns false and has frames [A, B, C]. -// ShowFrom(A) returns true and has frames [A, B, C]. -// ShowFrom(B) returns true and has frames [B, C]. -// ShowFrom(C) returns true and has frames [C]. -// ShowFrom(D) returns false and the sample is dropped because no frames remain. +// Example: consider a sample with frames [A, B, C, B], where A is the root. +// ShowFrom(nil) returns false and has frames [A, B, C, B]. +// ShowFrom(A) returns true and has frames [A, B, C, B]. +// ShowFrom(B) returns true and has frames [B, C, B]. +// ShowFrom(C) returns true and has frames [C, B]. +// ShowFrom(D) returns false and drops the sample because no frames remain. func (p *Profile) ShowFrom(showFrom *regexp.Regexp) (matched bool) { if showFrom == nil { return false } - // showFromLocs stores location IDs that matched ShowFrom. showFromLocs := make(map[uint64]bool) - // Apply to locations. - for _, location := range p.Location { - if filterShowFromLocation(location, showFrom) { - showFromLocs[location.ID] = true + for _, loc := range p.Location { + if filterShowFromLocation(loc, showFrom) { + showFromLocs[loc.ID] = true matched = true } } - - // Apply to samples and paths - filteredSamples := make([]*Sample, 0, len(p.Sample)) + // For all samples, strip locations after the highest matching one. + s := make([]*Sample, 0, len(p.Sample)) for _, sample := range p.Sample { - if i := lastLocationIndex(sample.Location, showFromLocs); i >= 0 { - sample.Location = sample.Location[0 : i+1] - filteredSamples = append(filteredSamples, sample) + for i := len(sample.Location) - 1; i >= 0; i-- { + if showFromLocs[sample.Location[i].ID] { + sample.Location = sample.Location[:i+1] + s = append(s, sample) + break + } } } - - p.Sample = filteredSamples - return + p.Sample = s + return matched } -// filterShowFromLocation tests a showFrom regex against a location and removes -// lines after the last match. If the mapping is matched, then all lines are -// kept. -func filterShowFromLocation(location *Location, showFrom *regexp.Regexp) (matched bool) { - if showFrom == nil { - return false - } - - if location.matchesMapping(showFrom) { +// filterShowFromLocation tests a showFrom regex against a location, removes +// lines after the last match and returns whether a match was found. If the +// mapping is matched, then all lines are kept. +func filterShowFromLocation(loc *Location, showFrom *regexp.Regexp) (matched bool) { + if m := loc.Mapping; m != nil && showFrom.MatchString(m.File) { return true } - - if i := location.lastMatchedLineIndex(showFrom); i >= 0 { + if i := loc.lastMatchedLineIndex(showFrom); i >= 0 { matched = true - location.Line = location.Line[0 : i+1] + loc.Line = loc.Line[0 : i+1] } - - return -} - -// lastLocationIndex returns the index of the last location who's ID is in the -// map. -func lastLocationIndex(path []*Location, matchedIDs map[uint64]bool) int { - for i := len(path) - 1; i >= 0; i-- { - if matchedIDs[path[i].ID] { - return i - } - } - return -1 + return matched } // lastMatchedLineIndex returns the index of the last line that matches a regex, @@ -157,14 +139,6 @@ func (loc *Location) lastMatchedLineIndex(re *regexp.Regexp) int { return -1 } -// matchesMapping returns whether a regex matches a location's mapping. -func (loc *Location) matchesMapping(re *regexp.Regexp) bool { - if m := loc.Mapping; m != nil && re != nil && re.MatchString(m.File) { - return true - } - return false -} - // FilterTagsByName filters the tags in a profile and only keeps // tags that match show and not hide. func (p *Profile) FilterTagsByName(show, hide *regexp.Regexp) (sm, hm bool) { diff --git a/profile/filter_test.go b/profile/filter_test.go index e637472c..3fd1787e 100644 --- a/profile/filter_test.go +++ b/profile/filter_test.go @@ -416,11 +416,8 @@ func TestFilterSamplesByName(t *testing.T) { func TestShowFrom(t *testing.T) { for _, tc := range []struct { - // name is the name of the test case. - name string - // profile is the profile to filter. - profile *Profile - // showFrom is the showFrom filter. + name string + profile *Profile showFrom *regexp.Regexp // wantMatch is the expected return value. wantMatch bool @@ -472,6 +469,17 @@ func TestShowFrom(t *testing.T) { "fun9 fun4 fun10: 4", }, }, + { + name: "showFrom drops frames above highest of multiple matches", + profile: noInlinesProfile, + showFrom: regexp.MustCompile("fun[12]"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun0 fun1 fun2: 1", + "fun4 fun5 fun1: 2", + "fun9 fun4 fun10: 4", + }, + }, { name: "showFrom matches inline functions", profile: inlinesProfile, @@ -482,6 +490,16 @@ func TestShowFrom(t *testing.T) { "fun4 fun5: 2", }, }, + { + name: "showFrom drops frames above highest of multiple inline matches", + profile: inlinesProfile, + showFrom: regexp.MustCompile("fun[1245]"), + wantMatch: true, + wantSampleFuncs: []string{ + "fun0 fun1 fun2: 1", + "fun4 fun5: 2", + }, + }, { name: "showFrom keeps all lines when matching mapping and function", profile: inlinesProfile, @@ -504,10 +522,9 @@ func TestShowFrom(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { p := tc.profile.Copy() - gotMatch := p.ShowFrom(tc.showFrom) - if gotMatch != tc.wantMatch { - t.Errorf("match got %+v want %+v", gotMatch, tc.wantMatch) + if gotMatch := p.ShowFrom(tc.showFrom); gotMatch != tc.wantMatch { + t.Errorf("match got %+v, want %+v", gotMatch, tc.wantMatch) } if got, want := strings.Join(sampleFuncs(p), "\n")+"\n", strings.Join(tc.wantSampleFuncs, "\n")+"\n"; got != want { From 5e56ca2dcfab5c89dcf0d4b07c204f93a961c43a Mon Sep 17 00:00:00 2001 From: orie Date: Fri, 4 May 2018 15:32:09 -0700 Subject: [PATCH 5/7] review feedback round 2. --- internal/driver/driver.go | 6 +++--- profile/filter.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/driver/driver.go b/internal/driver/driver.go index 51deef29..85a31e29 100644 --- a/internal/driver/driver.go +++ b/internal/driver/driver.go @@ -150,11 +150,11 @@ func generateReport(p *profile.Profile, cmd []string, vars variables, o *plugin. } func applyCommandOverrides(cmd []string, v variables) variables { - trim, tagfocus, filter := v["trim"].boolValue(), true, true + trim, tagfilter, filter := v["trim"].boolValue(), true, true switch cmd[0] { case "proto", "raw": - trim, tagfocus, filter = false, false, false + trim, tagfilter, filter = false, false, false v.set("addresses", "t") case "callgrind", "kcachegrind": trim = false @@ -181,7 +181,7 @@ func applyCommandOverrides(cmd []string, v variables) variables { v.set("nodefraction", "0") v.set("edgefraction", "0") } - if !tagfocus { + if !tagfilter { v.set("tagfocus", "") v.set("tagignore", "") } diff --git a/profile/filter.go b/profile/filter.go index da5d24d3..ea8e66c6 100644 --- a/profile/filter.go +++ b/profile/filter.go @@ -115,15 +115,15 @@ func (p *Profile) ShowFrom(showFrom *regexp.Regexp) (matched bool) { // filterShowFromLocation tests a showFrom regex against a location, removes // lines after the last match and returns whether a match was found. If the // mapping is matched, then all lines are kept. -func filterShowFromLocation(loc *Location, showFrom *regexp.Regexp) (matched bool) { +func filterShowFromLocation(loc *Location, showFrom *regexp.Regexp) bool { if m := loc.Mapping; m != nil && showFrom.MatchString(m.File) { return true } if i := loc.lastMatchedLineIndex(showFrom); i >= 0 { - matched = true - loc.Line = loc.Line[0 : i+1] + loc.Line = loc.Line[:i+1] + return true } - return matched + return false } // lastMatchedLineIndex returns the index of the last line that matches a regex, From 7af823d3995313c612580de94d7c49e23e408f13 Mon Sep 17 00:00:00 2001 From: orie Date: Mon, 7 May 2018 14:42:28 -0700 Subject: [PATCH 6/7] Add show_from to pprof doc. --- doc/pprof.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/pprof.md b/doc/pprof.md index 57613fad..96a1d6c4 100644 --- a/doc/pprof.md +++ b/doc/pprof.md @@ -94,6 +94,8 @@ Some common pprof options are: *regex*. * **-ignore= _regex_:** Do not include samples that include a report entry matching *regex*. +* **-show\_from= _regex_:** Do not show entries above the first frame that + matches *regex*. * **-show= _regex_:** Only show entries that match *regex*. * **-hide= _regex_:** Do not show entries that match *regex*. From 2e80e4ee1c9d097c45c8a8d3edc3ff9fc38892a9 Mon Sep 17 00:00:00 2001 From: orie Date: Mon, 7 May 2018 15:10:18 -0700 Subject: [PATCH 7/7] change language of show_from doc. --- doc/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/README.md b/doc/README.md index 96a1d6c4..de7c393e 100644 --- a/doc/README.md +++ b/doc/README.md @@ -94,7 +94,7 @@ Some common pprof options are: *regex*. * **-ignore= _regex_:** Do not include samples that include a report entry matching *regex*. -* **-show\_from= _regex_:** Do not show entries above the first frame that +* **-show\_from= _regex_:** Do not show entries above the first one that matches *regex*. * **-show= _regex_:** Only show entries that match *regex*. * **-hide= _regex_:** Do not show entries that match *regex*.