Disambiguate names for kcachegrind under the call_tree option

When using the call_tree option and generating a graph for kcachegrind,
it will merge back nodes that are distinct on the tree, producing some
confusing results. Add a suffix so that these entries are kept separate.

This addresses the problem described in
http://yosefk.com/blog/how-profilers-lie-the-cases-of-gprof-and-kcachegrind.html ,
particularly the summary "Choosing a profiler is hard" section.

internal/graph/graph.go

@@ -50,20 +50,28 @@ type Nodes []*Node
 // Node is an entry on a profiling report. It represents a unique
 // program location.
 type Node struct {
-	// Information associated to this entry.
+	// Info describes the source location associated to this node.
 	Info NodeInfo
 
-	// values associated to this node.
-	// Flat is exclusive to this node, cum includes all descendents.
+	// Function represents the function that this node belongs to.  On
+	// graphs with sub-function resolution (eg line number or
+	// addresses), two nodes in a NodeMap that are part of the same
+	// function have the same value of Node.Function. If the Node
+	// represents the whole function, it points back to itself.
+	Function *Node
+
+	// Values associated to this node. Flat is exclusive to this node,
+	// Cum includes all descendents.
 	Flat, Cum int64
 
-	// in and out contains the nodes immediately reaching or reached by this nodes.
+	// In and out Contains the nodes immediately reaching or reached by
+	// this node.
 	In, Out EdgeMap
 
-	// tags provide additional information about subsets of a sample.
+	// LabelTags provide additional information about subsets of a sample.
 	LabelTags TagMap
 
-	// Numeric tags provide additional values for subsets of a sample.
+	// NumericTags provide additional values for subsets of a sample.
 	// Numeric tags are optionally associated to a label tag. The key
 	// for NumericTags is the name of the LabelTag they are associated
 	// to, or "" for numeric tags not associated to a label tag.
@@ -176,6 +184,16 @@ func (nm NodeMap) FindOrInsertNode(info NodeInfo, kept NodeSet) *Node {
 		NumericTags: make(map[string]TagMap),
 	}
 	nm[info] = n
+	if info.Address == 0 && info.Lineno == 0 {
+		// This node represents the whole function, so point Function
+		// back to itself.
+		n.Function = n
+		return n
+	}
+	// Find a node that represents the whole function.
+	info.Address = 0
+	info.Lineno = 0
+	n.Function = nm.FindOrInsertNode(info, nil)
 	return n
 }
 

internal/report/report.go

@@ -203,7 +203,7 @@ func (rpt *Report) newGraph(nodes graph.NodeSet) *graph.Graph {
 	gopt := &graph.Options{
 		SampleValue:  o.SampleValue,
 		FormatTag:    formatTag,
-		CallTree:     o.CallTree && o.OutputFormat == Dot,
+		CallTree:     o.CallTree && (o.OutputFormat == Dot || o.OutputFormat == Callgrind),
 		DropNegative: o.DropNegative,
 		KeptNodes:    nodes,
 	}
@@ -627,13 +627,14 @@ func printCallgrind(w io.Writer, rpt *Report) error {
 	g, _, _, _ := rpt.newTrimmedGraph()
 	rpt.selectOutputUnit(g)
 
+	nodeNames := getDisambiguatedNames(g)
 	fmt.Fprintln(w, "events:", o.SampleType+"("+o.OutputUnit+")")
 
 	files := make(map[string]int)
 	names := make(map[string]int)
 	for _, n := range g.Nodes {
 		fmt.Fprintln(w, "fl="+callgrindName(files, n.Info.File))
-		fmt.Fprintln(w, "fn="+callgrindName(names, n.Info.Name))
+		fmt.Fprintln(w, "fn="+callgrindName(names, nodeNames[n]))
 		sv, _ := measurement.Scale(n.Flat, o.SampleUnit, o.OutputUnit)
 		fmt.Fprintf(w, "%d %d\n", n.Info.Lineno, int64(sv))
 
@@ -642,7 +643,7 @@ func printCallgrind(w io.Writer, rpt *Report) error {
 			c, _ := measurement.Scale(out.Weight, o.SampleUnit, o.OutputUnit)
 			callee := out.Dest
 			fmt.Fprintln(w, "cfl="+callgrindName(files, callee.Info.File))
-			fmt.Fprintln(w, "cfn="+callgrindName(names, callee.Info.Name))
+			fmt.Fprintln(w, "cfn="+callgrindName(names, nodeNames[callee]))
 			// pprof doesn't have a flat weight for a call, leave as 0.
 			fmt.Fprintln(w, "calls=0", callee.Info.Lineno)
 			fmt.Fprintln(w, n.Info.Lineno, int64(c))
@@ -653,6 +654,48 @@ func printCallgrind(w io.Writer, rpt *Report) error {
 	return nil
 }
 
+// getDisambiguatedNames returns a map from each node in the graph to
+// the name to use in the callgrind output. Callgrind merges all
+// functions with the same [file name, function name]. Add a [%d/n]
+// suffix to disambiguate nodes with different values of
+// node.Function, which we want to keep separate. In particular, this
+// affects graphs created with --call_tree, where nodes from different
+// contexts are associated to different Functions.
+func getDisambiguatedNames(g *graph.Graph) map[*graph.Node]string {
+	nodeName := make(map[*graph.Node]string, len(g.Nodes))
+
+	type names struct {
+		file, function string
+	}
+
+	// nameFunctionIndex maps the callgrind names (filename, function)
+	// to the node.Function values found for that name, and each
+	// node.Function value to a sequential index to be used on the
+	// disambiguated name.
+	nameFunctionIndex := make(map[names]map[*graph.Node]int)
+	for _, n := range g.Nodes {
+		nm := names{n.Info.File, n.Info.Name}
+		p, ok := nameFunctionIndex[nm]
+		if !ok {
+			p = make(map[*graph.Node]int)
+			nameFunctionIndex[nm] = p
+		}
+		if _, ok := p[n.Function]; !ok {
+			p[n.Function] = len(p)
+		}
+	}
+
+	for _, n := range g.Nodes {
+		nm := names{n.Info.File, n.Info.Name}
+		nodeName[n] = n.Info.Name
+		if p := nameFunctionIndex[nm]; len(p) > 1 {
+			// If there is more than one function, add suffix to disambiguate.
+			nodeName[n] += fmt.Sprintf(" [%d/%d]", p[n.Function]+1, len(p))
+		}
+	}
+	return nodeName
+}
+
 // callgrindName implements the callgrind naming compression scheme.
 // For names not previously seen returns "(N) name", where N is a
 // unique index.  For names previously seen returns "(N)" where N is

internal/report/report_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/google/pprof/internal/binutils"
+	"github.com/google/pprof/internal/graph"
 	"github.com/google/pprof/internal/proftest"
 	"github.com/google/pprof/profile"
 )
@@ -205,3 +206,33 @@ var testProfile = &profile.Profile{
 	Function: testF,
 	Mapping:  testM,
 }
+
+func TestDisambiguation(t *testing.T) {
+	parent1 := &graph.Node{Info: graph.NodeInfo{Name: "parent1"}}
+	parent2 := &graph.Node{Info: graph.NodeInfo{Name: "parent2"}}
+	child1 := &graph.Node{Info: graph.NodeInfo{Name: "child"}, Function: parent1}
+	child2 := &graph.Node{Info: graph.NodeInfo{Name: "child"}, Function: parent2}
+	child3 := &graph.Node{Info: graph.NodeInfo{Name: "child"}, Function: parent1}
+	sibling := &graph.Node{Info: graph.NodeInfo{Name: "sibling"}, Function: parent1}
+
+	n := []*graph.Node{parent1, parent2, child1, child2, child3, sibling}
+
+	wanted := map[*graph.Node]string{
+		parent1: "parent1",
+		parent2: "parent2",
+		child1:  "child [1/2]",
+		child2:  "child [2/2]",
+		child3:  "child [1/2]",
+		sibling: "sibling",
+	}
+
+	g := &graph.Graph{n}
+
+	names := getDisambiguatedNames(g)
+
+	for node, want := range wanted {
+		if got := names[node]; got != want {
+			t.Errorf("name %s, got %s, want %s", node.Info.Name, got, want)
+		}
+	}
+}