Ensure pprof call_tree output is a tree

When generating a call tree, pprof was using a map to keep track of all the
inline nodes for a location. That is incorrect as it may cause inline functions
at different nesting levels to reuse the same node, causing the resulting graph
to not be a tree.

When creating a tree nodes with the same info may appear on multiple places
in the tree. Keeping one of them preserves them all, which may cause disconnected
nodes to remain. To ensure the resulting graph is a connected tree, do not include
children on any removed node, which is suitable for the normal tree refinement
(nodecount and nodefraction) but does not allow visual refinement, which may eliminate
intermediate nodes. Disable visual mode refinement for call_tree to avoid this issue.

internal/graph/graph.go

@@ -288,39 +288,39 @@ func newTree(prof *profile.Profile, o *Options) (g *Graph) {
 	kept := o.KeptNodes
 	keepBinary := o.ObjNames
 	parentNodeMap := make(map[*Node]NodeMap, len(prof.Sample))
+nextSample:
 	for _, sample := range prof.Sample {
 		weight := o.SampleValue(sample.Value)
 		if weight == 0 {
 			continue
 		}
 		var parent *Node
-		// A residual edge goes over one or more nodes that were not kept.
-		residual := false
 		labels := joinLabels(sample)
 		// Group the sample frames, based on a per-node map.
 		for i := len(sample.Location) - 1; i >= 0; i-- {
 			l := sample.Location[i]
-			nodeMap := parentNodeMap[parent]
-			if nodeMap == nil {
-				nodeMap = make(NodeMap)
-				parentNodeMap[parent] = nodeMap
+			lines := l.Line
+			if len(lines) == 0 {
+				lines = []profile.Line{{}} // Create empty line to include location info.
 			}
-			locNodes := nodeMap.findOrInsertLocation(l, keepBinary, kept)
-			for ni := len(locNodes) - 1; ni >= 0; ni-- {
-				n := locNodes[ni]
+			for lidx := len(lines) - 1; lidx >= 0; lidx-- {
+				nodeMap := parentNodeMap[parent]
+				if nodeMap == nil {
+					nodeMap = make(NodeMap)
+					parentNodeMap[parent] = nodeMap
+				}
+				n := nodeMap.findOrInsertLine(l, lines[lidx], keepBinary, kept)
 				if n == nil {
-					residual = true
-					continue
+					continue nextSample
 				}
 				n.addSample(weight, labels, sample.NumLabel, o.FormatTag, false)
 				if parent != nil {
-					parent.AddToEdge(n, weight, residual, ni != len(locNodes)-1)
+					parent.AddToEdge(n, weight, false, lidx != len(lines)-1)
 				}
 				parent = n
-				residual = false
 			}
 		}
-		if parent != nil && !residual {
+		if parent != nil {
 			parent.addSample(weight, labels, sample.NumLabel, o.FormatTag, true)
 		}
 	}
@@ -395,9 +395,15 @@ func CreateNodes(prof *profile.Profile, keepBinary bool, kept NodeSet) (Nodes, m
 
 	nm := make(NodeMap, len(prof.Location))
 	for _, l := range prof.Location {
-		if nodes := nm.findOrInsertLocation(l, keepBinary, kept); nodes != nil {
-			locations[l.ID] = nodes
+		lines := l.Line
+		if len(lines) == 0 {
+			lines = []profile.Line{{}} // Create empty line to include location info.
 		}
+		nodes := make(Nodes, len(lines))
+		for ln := range lines {
+			nodes[ln] = nm.findOrInsertLine(l, lines[ln], keepBinary, kept)
+		}
+		locations[l.ID] = nodes
 	}
 	return nm.nodes(), locations
 }
@@ -410,51 +416,33 @@ func (nm NodeMap) nodes() Nodes {
 	return nodes
 }
 
-func (nm NodeMap) findOrInsertLocation(l *profile.Location, keepBinary bool, kept NodeSet) Nodes {
+func (nm NodeMap) findOrInsertLine(l *profile.Location, li profile.Line, keepBinary bool, kept NodeSet) *Node {
 	var objfile string
 	if m := l.Mapping; m != nil && m.File != "" {
 		objfile = filepath.Base(m.File)
 	}
 
-	if len(l.Line) == 0 {
-		ni := NodeInfo{
-			Address: l.Address,
-			Objfile: objfile,
-		}
-		return Nodes{nm.FindOrInsertNode(ni, kept)}
+	if ni := nodeInfo(l, li, objfile, keepBinary); ni != nil {
+		return nm.FindOrInsertNode(*ni, kept)
 	}
-	locNodes := make(Nodes, len(l.Line))
-	for li := range l.Line {
-		if ni := nodeInfo(l, li, objfile, keepBinary); ni != nil {
-			locNodes[li] = nm.FindOrInsertNode(*ni, kept)
-		}
-	}
-	return locNodes
+	return nil
 }
 
-func nodeInfo(l *profile.Location, li int, objfile string, keepBinary bool) *NodeInfo {
-	if !keepBinary {
-		objfile = ""
-	}
-	line := l.Line[li]
+func nodeInfo(l *profile.Location, line profile.Line, objfile string, keepBinary bool) *NodeInfo {
 	if line.Function == nil {
-		if l.Address == 0 {
-			return nil
-		}
 		return &NodeInfo{Address: l.Address, Objfile: objfile}
 	}
-
 	ni := &NodeInfo{
 		Address:  l.Address,
 		Lineno:   int(line.Line),
 		Name:     line.Function.Name,
 		OrigName: line.Function.SystemName,
-		Objfile:  objfile,
 	}
 	if fname := line.Function.Filename; fname != "" {
 		ni.File = filepath.Clean(fname)
 	}
 	if keepBinary {
+		ni.Objfile = objfile
 		ni.StartLine = int(line.Function.StartLine)
 	}
 	return ni
@@ -479,7 +467,7 @@ func (t tags) Less(i, j int) bool {
 	return t.t[i].Name < t.t[j].Name
 }
 
-// Sum adds the Flat and sum values on a report.
+// Sum adds the flat and cum values of a set of nodes.
 func (ns Nodes) Sum() (flat int64, cum int64) {
 	for _, n := range ns {
 		flat += n.Flat

internal/report/report.go

@@ -77,6 +77,8 @@ func (rpt *Report) newTrimmedGraph() (g *graph.Graph, origCount, droppedNodes, d
 
 	// Build a graph and refine it. On each refinement step we must rebuild the graph from the samples,
 	// as the graph itself doesn't contain enough information to preserve full precision.
+	visualMode := o.OutputFormat == Dot
+	cumSort := o.CumSort
 
 	// First step: Build complete graph to identify low frequency nodes, based on their cum weight.
 	g = rpt.newGraph(nil)
@@ -84,6 +86,16 @@ func (rpt *Report) newTrimmedGraph() (g *graph.Graph, origCount, droppedNodes, d
 	nodeCutoff := abs64(int64(float64(totalValue) * o.NodeFraction))
 	edgeCutoff := abs64(int64(float64(totalValue) * o.EdgeFraction))
 
+	// Visual mode optimization only supports graph output, not tree.
+	// Do not apply edge cutoff to preserve tree structure.
+	if o.CallTree {
+		visualMode = false
+		if o.OutputFormat == Dot {
+			cumSort = true
+		}
+		edgeCutoff = 0
+	}
+
 	// Filter out nodes with cum value below nodeCutoff.
 	if nodeCutoff > 0 {
 		if nodesKept := g.DiscardLowFrequencyNodes(nodeCutoff); len(g.Nodes) != len(nodesKept) {
@@ -95,15 +107,14 @@ func (rpt *Report) newTrimmedGraph() (g *graph.Graph, origCount, droppedNodes, d
 
 	// Second step: Limit the total number of nodes. Apply specialized heuristics to improve
 	// visualization when generating dot output.
-	visualMode := o.OutputFormat == Dot
-	g.SortNodes(o.CumSort, visualMode)
+	g.SortNodes(cumSort, visualMode)
 	if nodeCount := o.NodeCount; nodeCount > 0 {
 		// Remove low frequency tags and edges as they affect selection.
 		g.TrimLowFrequencyTags(nodeCutoff)
 		g.TrimLowFrequencyEdges(edgeCutoff)
 		if nodesKept := g.SelectTopNodes(nodeCount, visualMode); len(nodesKept) != len(g.Nodes) {
 			g = rpt.newGraph(nodesKept)
-			g.SortNodes(o.CumSort, visualMode)
+			g.SortNodes(cumSort, visualMode)
 		}
 	}
 

internal/report/testdata/source.dot

@@ -1,17 +1,17 @@
 digraph "unnamed" {
 node [style=filled fillcolor="#f8f8f8"]
 subgraph cluster_L { "Duration: 10s, Total samples = 11111 " [shape=box fontsize=16 label="Duration: 10s, Total samples = 11111 \lShowing nodes accounting for 11111, 100% of 11111 total\l"] }
-N1 [label="tee\nsource2:8\n10000 (90.00%)" fontsize=24 shape=box tooltip="tee testdata/source2:8 (10000)" color="#b20500" fillcolor="#edd6d5"]
-N2 [label="main\nsource1:2\n1 (0.009%)\nof 11111 (100%)" fontsize=9 shape=box tooltip="main testdata/source1:2 (11111)" color="#b20000" fillcolor="#edd5d5"]
-N3 [label="tee\nsource2:2\n1000 (9.00%)\nof 11000 (99.00%)" fontsize=14 shape=box tooltip="tee testdata/source2:2 (11000)" color="#b20000" fillcolor="#edd5d5"]
-N4 [label="tee\nsource2:8\n100 (0.9%)" fontsize=10 shape=box tooltip="tee testdata/source2:8 (100)" color="#b2b0aa" fillcolor="#edecec"]
-N5 [label="bar\nsource1:10\n10 (0.09%)" fontsize=9 shape=box tooltip="bar testdata/source1:10 (10)" color="#b2b2b1" fillcolor="#ededed"]
-N6 [label="bar\nsource1:10\n0 of 100 (0.9%)" fontsize=8 shape=box tooltip="bar testdata/source1:10 (100)" color="#b2b0aa" fillcolor="#edecec"]
+N1 [label="main\nsource1:2\n1 (0.009%)\nof 11111 (100%)" fontsize=9 shape=box tooltip="main testdata/source1:2 (11111)" color="#b20000" fillcolor="#edd5d5"]
+N2 [label="tee\nsource2:2\n1000 (9.00%)\nof 11000 (99.00%)" fontsize=14 shape=box tooltip="tee testdata/source2:2 (11000)" color="#b20000" fillcolor="#edd5d5"]
+N3 [label="tee\nsource2:8\n10000 (90.00%)" fontsize=24 shape=box tooltip="tee testdata/source2:8 (10000)" color="#b20500" fillcolor="#edd6d5"]
+N4 [label="bar\nsource1:10\n0 of 100 (0.9%)" fontsize=8 shape=box tooltip="bar testdata/source1:10 (100)" color="#b2b0aa" fillcolor="#edecec"]
+N5 [label="tee\nsource2:8\n100 (0.9%)" fontsize=10 shape=box tooltip="tee testdata/source2:8 (100)" color="#b2b0aa" fillcolor="#edecec"]
+N6 [label="bar\nsource1:10\n10 (0.09%)" fontsize=9 shape=box tooltip="bar testdata/source1:10 (10)" color="#b2b2b1" fillcolor="#ededed"]
 N7 [label="foo\nsource1:4\n0 of 10 (0.09%)" fontsize=8 shape=box tooltip="foo testdata/source1:4 (10)" color="#b2b2b1" fillcolor="#ededed"]
-N2 -> N3 [label=" 11000" weight=100 penwidth=5 color="#b20000" tooltip="main testdata/source1:2 -> tee testdata/source2:2 (11000)" labeltooltip="main testdata/source1:2 -> tee testdata/source2:2 (11000)"]
-N3 -> N1 [label=" 10000" weight=91 penwidth=5 color="#b20500" tooltip="tee testdata/source2:2 -> tee testdata/source2:8 (10000)" labeltooltip="tee testdata/source2:2 -> tee testdata/source2:8 (10000)"]
-N6 -> N4 [label=" 100" color="#b2b0aa" tooltip="bar testdata/source1:10 -> tee testdata/source2:8 (100)" labeltooltip="bar testdata/source1:10 -> tee testdata/source2:8 (100)"]
-N2 -> N6 [label=" 100" color="#b2b0aa" tooltip="main testdata/source1:2 -> bar testdata/source1:10 (100)" labeltooltip="main testdata/source1:2 -> bar testdata/source1:10 (100)"]
-N7 -> N5 [label=" 10" color="#b2b2b1" tooltip="foo testdata/source1:4 -> bar testdata/source1:10 (10)" labeltooltip="foo testdata/source1:4 -> bar testdata/source1:10 (10)"]
-N2 -> N7 [label=" 10" color="#b2b2b1" tooltip="main testdata/source1:2 -> foo testdata/source1:4 (10)" labeltooltip="main testdata/source1:2 -> foo testdata/source1:4 (10)"]
+N1 -> N2 [label=" 11000" weight=100 penwidth=5 color="#b20000" tooltip="main testdata/source1:2 -> tee testdata/source2:2 (11000)" labeltooltip="main testdata/source1:2 -> tee testdata/source2:2 (11000)"]
+N2 -> N3 [label=" 10000" weight=91 penwidth=5 color="#b20500" tooltip="tee testdata/source2:2 -> tee testdata/source2:8 (10000)" labeltooltip="tee testdata/source2:2 -> tee testdata/source2:8 (10000)"]
+N4 -> N5 [label=" 100" color="#b2b0aa" tooltip="bar testdata/source1:10 -> tee testdata/source2:8 (100)" labeltooltip="bar testdata/source1:10 -> tee testdata/source2:8 (100)"]
+N1 -> N4 [label=" 100" color="#b2b0aa" tooltip="main testdata/source1:2 -> bar testdata/source1:10 (100)" labeltooltip="main testdata/source1:2 -> bar testdata/source1:10 (100)"]
+N7 -> N6 [label=" 10" color="#b2b2b1" tooltip="foo testdata/source1:4 -> bar testdata/source1:10 (10)" labeltooltip="foo testdata/source1:4 -> bar testdata/source1:10 (10)"]
+N1 -> N7 [label=" 10" color="#b2b2b1" tooltip="main testdata/source1:2 -> foo testdata/source1:4 (10)" labeltooltip="main testdata/source1:2 -> foo testdata/source1:4 (10)"]
 }