Cleans up comments and names regarding Graph

internal/graph/graph.go

@@ -144,14 +144,14 @@ type NodeMap map[NodeInfo]*Node
 // NodeSet is a collection of node info structs.
 type NodeSet map[NodeInfo]bool
 
-// NodePtrSet is a collection of Nodes. Trimming a graph or tree requires a set
-// of objects that uniquely identify which nodes to keep. In a graph, NodeInfo
-// works as a unique identifier; however, in a tree, multiple nodes may share
-// identical NodeInfos. *Node do uniquely identify nodes so we can use those
-// instead.Though *Node also uniquely identify nodes in a Graph, we currently
-// rebuild Graphs from scratch using only the NodeSet when trimming, so there
-// would not be the required context of the initial graph to allow for the use
-// of *Node to identify the nodes.
+// NodePtrSet is a collection of nodes. Trimming a graph or tree requires a set
+// of objects which uniquely identify the nodes to keep. In a graph, NodeInfo
+// works as a unique identifier; however, in a tree multiple nodes may share
+// identical NodeInfos. A *Node does uniquely identify a node so we can use that
+// instead. Though a *Node also uniquely identifies a node in a graph,
+// currently, during trimming, graphs are rebult from scratch using only the
+// NodeSet, so there would not be the required context of the initial graph to
+// allow for the use of *Node.
 type NodePtrSet map[*Node]bool
 
 // FindOrInsertNode takes the info for a node and either returns a matching node
@@ -341,8 +341,8 @@ func newTree(prof *profile.Profile, o *Options) (g *Graph) {
 	return selectNodesForGraph(nodes, o.DropNegative)
 }
 
-// Trims a Graph that is in forest form to contain only the nodes in kept. This
-// will not work correctly in the case that a node has multiple parents.
+// TrimTree trims a Graph in forest form, keeping only the nodes in kept. This
+// will not work correctly if even a single node has multiple parents.
 func (g *Graph) TrimTree(kept NodePtrSet) {
 	// Creates a new list of nodes
 	oldNodes := g.Nodes
@@ -354,14 +354,14 @@ func (g *Graph) TrimTree(kept NodePtrSet) {
 			panic("TrimTree only works on trees")
 		}
 
-		// If a node should be kept, add it to the next list of nodes
+		// If a node should be kept, add it to the new list of nodes
 		if _, ok := kept[cur]; ok {
 			g.Nodes = append(g.Nodes, cur)
 			continue
 		}
 
-		// If a node has no parents, delete all the in edges of the children to make them
-		// all roots of their own trees.
+		// If a node has no parents, then delete all of the in edges of its
+		// children to make them each roots of their own trees.
 		if len(cur.In) == 0 {
 			for _, outEdge := range cur.Out {
 				delete(outEdge.Dest.In, cur)
@@ -369,8 +369,11 @@ func (g *Graph) TrimTree(kept NodePtrSet) {
 			continue
 		}
 
-		// Get the parent. Since cur.In may only be of size 0 or 1, parent will be
-		// equal to either nil or the only node in cur.In
+		// Get the parent. This works since at this point cur.In must contain only
+		// one element.
+		if len(cur.In) != 1 {
+			panic("Get parent assertion failed. cur.In expected to be of length 1.")
+		}
 		var parent *Node
 		for _, edge := range cur.In {
 			parent = edge.Src
@@ -606,7 +609,7 @@ func (g *Graph) DiscardLowFrequencyNodes(nodeCutoff int64) NodeSet {
 	return makeNodeSet(g.Nodes, nodeCutoff)
 }
 
-// discardLowFrequencyNodePtrs returns a NodePtrSet of nodes at or over a
+// DiscardLowFrequencyNodePtrs returns a NodePtrSet of nodes at or over a
 // specific cum value cutoff.
 func (g *Graph) DiscardLowFrequencyNodePtrs(nodeCutoff int64) NodePtrSet {
 	cutNodes := getNodesAboveCumCutoff(g.Nodes, nodeCutoff)
@@ -626,8 +629,8 @@ func makeNodeSet(nodes Nodes, nodeCutoff int64) NodeSet {
 	return kept
 }
 
-// getNodesAboveCumCutoff returns all the nodes who have a Cum value greater
-// than or equal to cutoff
+// getNodesAboveCumCutoff returns all the nodes which have a Cum value greater
+// than or equal to cutoff.
 func getNodesAboveCumCutoff(nodes Nodes, nodeCutoff int64) Nodes {
 	cutoffNodes := make(Nodes, 0, len(nodes))
 	for _, n := range nodes {
@@ -705,7 +708,7 @@ func (g *Graph) SelectTopNodes(maxNodes int, visualMode bool) NodeSet {
 	return makeNodeSet(g.selectTopNodes(maxNodes, visualMode), 0)
 }
 
-// selectTopNodes returns a slice of the top maxNodes nodes in a graph
+// selectTopNodes returns a slice of the top maxNodes nodes in a graph.
 func (g *Graph) selectTopNodes(maxNodes int, visualMode bool) Nodes {
 	if maxNodes > 0 {
 		if visualMode {

internal/graph/graph_test.go

@@ -44,13 +44,13 @@ func graphDebugString(graph *Graph) string {
 	return debug
 }
 
-func expectedNodesDebugString(Expected []ExpectedNode) string {
+func expectedNodesDebugString(expected []ExpectedNode) string {
 	debug := ""
-	for i, node := range Expected {
+	for i, node := range expected {
 		debug += fmt.Sprintf("Node %d: %p\n", i, node.Node)
 	}
 
-	for i, node := range Expected {
+	for i, node := range expected {
 		debug += "\n"
 		debug += fmt.Sprintf("===  Node %d: %p  ===\n", i, node.Node)
 		debug += edgeMapsDebugString(node.In, node.Out)
@@ -58,7 +58,7 @@ func expectedNodesDebugString(Expected []ExpectedNode) string {
 	return debug
 }
 
-// Checks if all the edges in this equal all the edges in that.
+// edgeMapsEqual checks if all the edges in this equal all the edges in that.
 func edgeMapsEqual(this, that EdgeMap) bool {
 	if len(this) != len(that) {
 		return false
@@ -71,25 +71,25 @@ func edgeMapsEqual(this, that EdgeMap) bool {
 	return true
 }
 
-// Check if node is equal to Expected
-func nodesEqual(node *Node, Expected ExpectedNode) bool {
-	return node == Expected.Node && edgeMapsEqual(node.In, Expected.In) &&
-		edgeMapsEqual(node.Out, Expected.Out)
+// nodesEqual checks if node is equal to expected.
+func nodesEqual(node *Node, expected ExpectedNode) bool {
+	return node == expected.Node && edgeMapsEqual(node.In, expected.In) &&
+		edgeMapsEqual(node.Out, expected.Out)
 }
 
-// Check if the graph equals the one templated by Expected.
-func graphsEqual(graph *Graph, Expected []ExpectedNode) bool {
-	if len(graph.Nodes) != len(Expected) {
+// graphsEqual checks if graph is equivalent to the graph templated by expected.
+func graphsEqual(graph *Graph, expected []ExpectedNode) bool {
+	if len(graph.Nodes) != len(expected) {
 		return false
 	}
-	ExpectedSet := make(map[*Node]ExpectedNode)
-	for i := range Expected {
-		ExpectedSet[Expected[i].Node] = Expected[i]
+	expectedSet := make(map[*Node]ExpectedNode)
+	for i := range expected {
+		expectedSet[expected[i].Node] = expected[i]
 	}
 
 	for _, node := range graph.Nodes {
-		ExpectedNode, found := ExpectedSet[node]
-		if !found || !nodesEqual(node, ExpectedNode) {
+		expectedNode, found := expectedSet[node]
+		if !found || !nodesEqual(node, expectedNode) {
 			return false
 		}
 	}
@@ -107,7 +107,7 @@ type TrimTreeTestCase struct {
 	Keep     NodePtrSet
 }
 
-// Makes the edge from parent to child residual
+// makeExpectedEdgeResidual makes the edge from parent to child residual.
 func makeExpectedEdgeResidual(parent, child ExpectedNode) {
 	parent.Out[child.Node].Residual = true
 	child.In[parent.Node].Residual = true
@@ -121,7 +121,7 @@ func setEdgeWeight(edgeMap EdgeMap, node *Node, weight int64) {
 	edgeMap[node].Weight = weight
 }
 
-// Creates a directed edges from the parent to each of the children
+// createEdges creates directed edges from the parent to each of the children.
 func createEdges(parent *Node, children ...*Node) {
 	for _, child := range children {
 		edge := &Edge{
@@ -133,7 +133,7 @@ func createEdges(parent *Node, children ...*Node) {
 	}
 }
 
-// Creates a node without any edges
+// createEmptyNode creates a node without any edges.
 func createEmptyNode() *Node {
 	return &Node{
 		In:  make(EdgeMap),
@@ -141,24 +141,25 @@ func createEmptyNode() *Node {
 	}
 }
 
-// Creates an array of ExpectedNodes from nodes.
+// createExpectedNodes creates a slice of ExpectedNodes from nodes.
 func createExpectedNodes(nodes ...*Node) ([]ExpectedNode, NodePtrSet) {
-	Expected := make([]ExpectedNode, len(nodes))
-	Keep := make(NodePtrSet, len(nodes))
+	expected := make([]ExpectedNode, len(nodes))
+	keep := make(NodePtrSet, len(nodes))
 
 	for i, node := range nodes {
-		Expected[i] = ExpectedNode{
+		expected[i] = ExpectedNode{
 			Node: node,
 			In:   make(EdgeMap),
 			Out:  make(EdgeMap),
 		}
-		Keep[node] = true
+		keep[node] = true
 	}
 
-	return Expected, Keep
+	return expected, keep
 }
 
-// Creates a directed edges from the parent to each of the children
+// createExpectedEdges creates directed edges from the parent to each of the
+// children.
 func createExpectedEdges(parent ExpectedNode, children ...ExpectedNode) {
 	for _, child := range children {
 		edge := &Edge{
@@ -170,17 +171,17 @@ func createExpectedEdges(parent ExpectedNode, children ...ExpectedNode) {
 	}
 }
 
-// The first test case looks like:
+// createTestCase1 creates a test case that initally looks like:
 //     0
 //     |(5)
 //     1
 // (3)/ \(4)
-//   2   3
+//   2   3.
 //
-// After Keeping 0, 2, 3. We should see:
+// After keeping 0, 2, and 3, it expects the graph:
 //     0
 // (3)/ \(4)
-//   2   3
+//   2   3.
 func createTestCase1() TrimTreeTestCase {
 	// Create Initial graph
 	graph := &Graph{make(Nodes, 4)}
@@ -197,21 +198,21 @@ func createTestCase1() TrimTreeTestCase {
 	setEdgeWeight(nodes[1].Out, nodes[3], 4)
 
 	// Create Expected graph
-	Expected, Keep := createExpectedNodes(nodes[0], nodes[2], nodes[3])
-	createExpectedEdges(Expected[0], Expected[1], Expected[2])
-	makeEdgeInline(Expected[0].Out, Expected[1].Node)
-	makeExpectedEdgeResidual(Expected[0], Expected[1])
-	makeExpectedEdgeResidual(Expected[0], Expected[2])
-	setEdgeWeight(Expected[0].Out, Expected[1].Node, 3)
-	setEdgeWeight(Expected[0].Out, Expected[2].Node, 4)
+	expected, keep := createExpectedNodes(nodes[0], nodes[2], nodes[3])
+	createExpectedEdges(expected[0], expected[1], expected[2])
+	makeEdgeInline(expected[0].Out, expected[1].Node)
+	makeExpectedEdgeResidual(expected[0], expected[1])
+	makeExpectedEdgeResidual(expected[0], expected[2])
+	setEdgeWeight(expected[0].Out, expected[1].Node, 3)
+	setEdgeWeight(expected[0].Out, expected[2].Node, 4)
 	return TrimTreeTestCase{
 		Initial:  graph,
-		Expected: Expected,
-		Keep:     Keep,
+		Expected: expected,
+		Keep:     keep,
 	}
 }
 
-// This test case looks like:
+// createTestCase2 creates a test case that initially looks like:
 //   3
 //   | (12)
 //   1
@@ -220,12 +221,12 @@ func createTestCase1() TrimTreeTestCase {
 //   | (15)
 //   0
 //   | (10)
-//   4
+//   4.
 //
-// After Keeping 3 and 4. We should see:
+// After keeping 3 and 4, it expects the graph:
 //   3
 //   | (10)
-//   4
+//   4.
 func createTestCase2() TrimTreeTestCase {
 	// Create Initial graph
 	graph := &Graph{make(Nodes, 5)}
@@ -243,44 +244,45 @@ func createTestCase2() TrimTreeTestCase {
 	setEdgeWeight(nodes[0].Out, nodes[4], 10)
 
 	// Create Expected graph
-	Expected, Keep := createExpectedNodes(nodes[3], nodes[4])
-	createExpectedEdges(Expected[0], Expected[1])
-	makeExpectedEdgeResidual(Expected[0], Expected[1])
-	setEdgeWeight(Expected[0].Out, Expected[1].Node, 10)
+	expected, keep := createExpectedNodes(nodes[3], nodes[4])
+	createExpectedEdges(expected[0], expected[1])
+	makeExpectedEdgeResidual(expected[0], expected[1])
+	setEdgeWeight(expected[0].Out, expected[1].Node, 10)
 	return TrimTreeTestCase{
 		Initial:  graph,
-		Expected: Expected,
-		Keep:     Keep,
+		Expected: expected,
+		Keep:     keep,
 	}
 }
 
-// If we trim an empty graph it should still be empty afterwards
+// createTestCase3 creates an initally empty graph and expects an empty graph
+// after trimming.
 func createTestCase3() TrimTreeTestCase {
 	graph := &Graph{make(Nodes, 0)}
-	Expected, Keep := createExpectedNodes()
+	expected, keep := createExpectedNodes()
 	return TrimTreeTestCase{
 		Initial:  graph,
-		Expected: Expected,
-		Keep:     Keep,
+		Expected: expected,
+		Keep:     keep,
 	}
 }
 
-// This test case looks like:
-//   0
+// createTestCase4 creates a test case that initially looks like:
+//   0.
 //
-// After Keeping 0. We should see:
-//   0
+// After keeping 0, it expects the graph:
+//   0.
 func createTestCase4() TrimTreeTestCase {
 	graph := &Graph{make(Nodes, 1)}
 	nodes := graph.Nodes
 	for i := range nodes {
 		nodes[i] = createEmptyNode()
 	}
-	Expected, Keep := createExpectedNodes(nodes[0])
+	expected, keep := createExpectedNodes(nodes[0])
 	return TrimTreeTestCase{
 		Initial:  graph,
-		Expected: Expected,
-		Keep:     Keep,
+		Expected: expected,
+		Keep:     keep,
 	}
 }