Repository: google/pprof
Pull request #881 (2024-07-16 14:37:01)

Speed-up flame graph generation by skipping unneeded work.
Previously, we used to call report.TextItems() during flame graph generation just so we could get a hand on the legend to print in the profile details box. All the work done by TextItems() to produce a trimmed graph was discarded.

This change separates out the legend generation into a separate routine so that we can avoid doing the unnecessary work.

Benchmark result:

```
name      old time/op  new time/op  delta
Flame-12   6.10s ± 3%   0.39s ± 4%  -93.59%  (p=0.000 n=10+10)
```

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/google/pprof/pull/881?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
Attention: Patch coverage is `80.00000%` with `3 lines` in your changes missing coverage. Please review.
> Project coverage is 66.50%. Comparing base [(`0ed6a68`)](https://app.codecov.io/gh/google/pprof/commit/0ed6a68c8d9e6d5df69a1f2694c7b5c3297fdf4f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) to head [(`f04b210`)](https://app.codecov.io/gh/google/pprof/commit/f04b21070a94841ee95acdf77a8858c57296388c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).
> Report is 29 commits behind head on main.

| [Files](https://app.codecov.io/gh/google/pprof/pull/881?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) | Patch % | Lines |
|---|---|---|
| [internal/report/stacks.go](https://app.codecov.io/gh/google/pprof/pull/881?src=pr&el=tree&filepath=internal%2Freport%2Fstacks.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-aW50ZXJuYWwvcmVwb3J0L3N0YWNrcy5nbw==) | 33.33% | [2 Missing :warning: ](https://app.codecov.io/gh/google/pprof/pull/881?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) |
| [internal/report/report.go](https://app.codecov.io/gh/google/pprof/pull/881?src=pr&el=tree&filepath=internal%2Freport%2Freport.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google#diff-aW50ZXJuYWwvcmVwb3J0L3JlcG9ydC5nbw==) | 90.90% | [1 Missing :warning: ](https://app.codecov.io/gh/google/pprof/pull/881?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #881      +/-   ##
==========================================
- Coverage   66.86%   66.50%   -0.37%     
==========================================
  Files          44       44              
  Lines        9824     9796      -28     
==========================================
- Hits         6569     6515      -54     
- Misses       2794     2826      +32     
+ Partials      461      455       -6     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/google/pprof/pull/881?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).


Patch:
diff --git a/internal/driver/stacks.go b/internal/driver/stacks.go
index 355b8f2..a793610 100644
--- a/internal/driver/stacks.go
+++ b/internal/driver/stacks.go
@@ -20,7 +20,6 @@ import (
 	"net/http"
 
 	"github.com/google/pprof/internal/measurement"
-	"github.com/google/pprof/internal/report"
 )
 
 // stackView generates the flamegraph view.
@@ -51,8 +50,7 @@ func (ui *webInterface) stackView(w http.ResponseWriter, req *http.Request) {
 	}
 	nodes[0] = "" // root is not a real node
 
-	_, legend := report.TextItems(rpt)
-	ui.render(w, req, "stacks", rpt, errList, legend, webArgs{
+	ui.render(w, req, "stacks", rpt, errList, stacks.Legend(), webArgs{
 		Stacks:   template.JS(b),
 		Nodes:    nodes,
 		UnitDefs: measurement.UnitTypes,
diff --git a/internal/report/report.go b/internal/report/report.go
index d72ebe9..e21ce85 100644
--- a/internal/report/report.go
+++ b/internal/report/report.go
@@ -781,7 +781,7 @@ type TextItem struct {
 func TextItems(rpt *Report) ([]TextItem, []string) {
 	g, origCount, droppedNodes, _ := rpt.newTrimmedGraph()
 	rpt.selectOutputUnit(g)
-	labels := reportLabels(rpt, g, origCount, droppedNodes, 0, false)
+	labels := reportLabels(rpt, graphTotal(g), len(g.Nodes), origCount, droppedNodes, 0, false)
 
 	var items []TextItem
 	var flatSum int64
@@ -1064,7 +1064,7 @@ func printTree(w io.Writer, rpt *Report) error {
 	g, origCount, droppedNodes, _ := rpt.newTrimmedGraph()
 	rpt.selectOutputUnit(g)
 
-	fmt.Fprintln(w, strings.Join(reportLabels(rpt, g, origCount, droppedNodes, 0, false), "\n"))
+	fmt.Fprintln(w, strings.Join(reportLabels(rpt, graphTotal(g), len(g.Nodes), origCount, droppedNodes, 0, false), "\n"))
 
 	fmt.Fprintln(w, separator)
 	fmt.Fprintln(w, legend)
@@ -1128,7 +1128,7 @@ func printTree(w io.Writer, rpt *Report) error {
 func GetDOT(rpt *Report) (*graph.Graph, *graph.DotConfig) {
 	g, origCount, droppedNodes, droppedEdges := rpt.newTrimmedGraph()
 	rpt.selectOutputUnit(g)
-	labels := reportLabels(rpt, g, origCount, droppedNodes, droppedEdges, true)
+	labels := reportLabels(rpt, graphTotal(g), len(g.Nodes), origCount, droppedNodes, droppedEdges, true)
 
 	c := &graph.DotConfig{
 		Title:       rpt.options.Title,
@@ -1184,12 +1184,19 @@ func ProfileLabels(rpt *Report) []string {
 	return label
 }
 
+func graphTotal(g *graph.Graph) int64 {
+	var total int64
+	for _, n := range g.Nodes {
+		total += n.FlatValue()
+	}
+	return total
+}
+
 // reportLabels returns printable labels for a report. Includes
 // profileLabels.
-func reportLabels(rpt *Report, g *graph.Graph, origCount, droppedNodes, droppedEdges int, fullHeaders bool) []string {
+func reportLabels(rpt *Report, shownTotal int64, nodeCount, origCount, droppedNodes, droppedEdges int, fullHeaders bool) []string {
 	nodeFraction := rpt.options.NodeFraction
 	edgeFraction := rpt.options.EdgeFraction
-	nodeCount := len(g.Nodes)
 
 	var label []string
 	if len(rpt.options.ProfileLabels) > 0 {
@@ -1198,17 +1205,12 @@ func reportLabels(rpt *Report, g *graph.Graph, origCount, droppedNodes, droppedE
 		label = ProfileLabels(rpt)
 	}
 
-	var flatSum int64
-	for _, n := range g.Nodes {
-		flatSum = flatSum + n.FlatValue()
-	}
-
 	if len(rpt.options.ActiveFilters) > 0 {
 		activeFilters := legendActiveFilters(rpt.options.ActiveFilters)
 		label = append(label, activeFilters...)
 	}
 
-	label = append(label, fmt.Sprintf("Showing nodes accounting for %s, %s of %s total", rpt.formatValue(flatSum), strings.TrimSpace(measurement.Percentage(flatSum, rpt.total)), rpt.formatValue(rpt.total)))
+	label = append(label, fmt.Sprintf("Showing nodes accounting for %s, %s of %s total", rpt.formatValue(shownTotal), strings.TrimSpace(measurement.Percentage(shownTotal, rpt.total)), rpt.formatValue(rpt.total)))
 
 	if rpt.total != 0 {
 		if droppedNodes > 0 {
diff --git a/internal/report/stacks.go b/internal/report/stacks.go
index aa3bf80..c6b07b8 100644
--- a/internal/report/stacks.go
+++ b/internal/report/stacks.go
@@ -34,6 +34,7 @@ type StackSet struct {
 	Unit    string        // One of "B", "s", "GCU", or "" (if unknown)
 	Stacks  []Stack       // List of stored stacks
 	Sources []StackSource // Mapping from source index to info
+	report  *Report
 }
 
 // Stack holds a single stack instance.
@@ -94,6 +95,7 @@ func (rpt *Report) Stacks() StackSet {
 		Unit:    unit,
 		Stacks:  []Stack{},       // Ensure non-nil
 		Sources: []StackSource{}, // Ensure non-nil
+		report:  rpt,
 	}
 	s.makeInitialStacks(rpt)
 	s.fillPlaces()
@@ -187,3 +189,8 @@ func (s *StackSet) assignColors() {
 		s.Sources[i].Color = int(index % numColors)
 	}
 }
+
+// Legend returns the list of lines to display as the legend.
+func (s *StackSet) Legend() []string {
+	return reportLabels(s.report, s.report.total, len(s.Sources), len(s.Sources), 0, 0, false)
+}


Test patch:
diff --git a/internal/report/stacks_test.go b/internal/report/stacks_test.go
index 660b76d..22d024b 100644
--- a/internal/report/stacks_test.go
+++ b/internal/report/stacks_test.go
@@ -3,6 +3,7 @@ package report
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/google/pprof/profile"
@@ -164,6 +165,22 @@ func TestStackSources(t *testing.T) {
 	}
 }
 
+func TestLegend(t *testing.T) {
+	// See report_test.go for the functions available to use in tests.
+	main, foo, bar, tee := testL[0], testL[1], testL[2], testL[3]
+	stacks := makeTestStacks(
+		testSample(100, bar, foo, main),
+		testSample(200, tee, foo, main),
+	)
+	got := strings.Join(stacks.Legend(), "\n")
+	expectStrings := []string{"Type: samples", "Showing nodes", "100% of 300 total"}
+	for _, expect := range expectStrings {
+		if !strings.Contains(got, expect) {
+			t.Errorf("missing expected string %q in legend %q", expect, got)
+		}
+	}
+}
+
 func findSource(stacks StackSet, name string) StackSource {
 	for _, src := range stacks.Sources {
 		if src.FullName == name {