Browse Source

Keep more residual edges

Do not remove a residual edge between A and B if there isn't another path
going between A and B. We previously would remove it if A and B had a common
ancestor, but that caused some value duplication as A could become a leaf
but its value would still be accounted for in B.
Raul Silvera 8 years ago
parent
commit
83add08ef1

+ 1
- 0
internal/driver/testdata/pprof.heap.flat.inuse_space.dot.focus.ignore View File

@@ -12,4 +12,5 @@ N4 -> NN4_0 [label=" 3.91MB" weight=100 tooltip="3.91MB" labeltooltip="3.91MB"]
12 12
 N2 -> N3 [label=" 36.13MB\n (inline)" weight=37 penwidth=2 color="#b22e00" tooltip="line3000 testdata/file3000.src -> line3001 testdata/file3000.src (36.13MB)" labeltooltip="line3000 testdata/file3000.src -> line3001 testdata/file3000.src (36.13MB)"]
13 13
 N3 -> N1 [label=" 32.23MB\n (inline)" weight=33 penwidth=2 color="#b23200" tooltip="line3001 testdata/file3000.src -> line3002 testdata/file3000.src (32.23MB)" labeltooltip="line3001 testdata/file3000.src -> line3002 testdata/file3000.src (32.23MB)"]
14 14
 N3 -> N4 [label=" 3.91MB" weight=4 color="#b2a58f" tooltip="line3001 testdata/file3000.src -> line1000 testdata/file1000.src (3.91MB)" labeltooltip="line3001 testdata/file3000.src -> line1000 testdata/file1000.src (3.91MB)"]
15
+N1 -> N4 [label=" 0.98MB" color="#b2b0a9" tooltip="line3002 testdata/file3000.src ... line1000 testdata/file1000.src (0.98MB)" labeltooltip="line3002 testdata/file3000.src ... line1000 testdata/file1000.src (0.98MB)" style="dotted" minlen=2]
15 16
 }

+ 9
- 22
internal/graph/graph.go View File

@@ -859,7 +859,7 @@ func (g *Graph) RemoveRedundantEdges() {
859 859
 				// avoid potential confusion.
860 860
 				break
861 861
 			}
862
-			if isRedundant(e) {
862
+			if isRedundantEdge(e) {
863 863
 				delete(e.Src.Out, e.Dest)
864 864
 				delete(e.Dest.In, e.Src)
865 865
 			}
@@ -867,26 +867,10 @@ func (g *Graph) RemoveRedundantEdges() {
867 867
 	}
868 868
 }
869 869
 
870
-// isRedundant determines if an edge can be removed without impacting
871
-// connectivity of the whole graph. This is implemented by checking if the
872
-// nodes have a common ancestor after removing the edge.
873
-func isRedundant(e *Edge) bool {
874
-	destPred := predecessors(e, e.Dest)
875
-	if len(destPred) == 1 {
876
-		return false
877
-	}
878
-	srcPred := predecessors(e, e.Src)
879
-
880
-	for n := range srcPred {
881
-		if destPred[n] && n != e.Dest {
882
-			return true
883
-		}
884
-	}
885
-	return false
886
-}
887
-
888
-// predecessors collects all the predecessors to node n, excluding edge e.
889
-func predecessors(e *Edge, n *Node) map[*Node]bool {
870
+// isRedundantEdge determines if there is a path that allows e.Src
871
+// to reach e.Dest after removing e.
872
+func isRedundantEdge(e *Edge) bool {
873
+	src, n := e.Src, e.Dest
890 874
 	seen := map[*Node]bool{n: true}
891 875
 	queue := Nodes{n}
892 876
 	for len(queue) > 0 {
@@ -896,11 +880,14 @@ func predecessors(e *Edge, n *Node) map[*Node]bool {
896 880
 			if e == ie || seen[ie.Src] {
897 881
 				continue
898 882
 			}
883
+			if ie.Src == src {
884
+				return true
885
+			}
899 886
 			seen[ie.Src] = true
900 887
 			queue = append(queue, ie.Src)
901 888
 		}
902 889
 	}
903
-	return seen
890
+	return false
904 891
 }
905 892
 
906 893
 // nodeSorter is a mechanism used to allow a report to be sorted