Merge pull request #16252 from hashicorp/jbardin/fast-transitive

faster DAG transitive reduction

dag/dag.go

@@ -106,7 +106,7 @@ func (g *AcyclicGraph) TransitiveReduction() {
 		uTargets := g.DownEdges(u)
 		vs := AsVertexList(g.DownEdges(u))
 
-		g.DepthFirstWalk(vs, func(v Vertex, d int) error {
+		g.depthFirstWalk(vs, false, func(v Vertex, d int) error {
 			shared := uTargets.Intersection(g.DownEdges(v))
 			for _, vPrime := range AsVertexList(shared) {
 				g.RemoveEdge(BasicEdge(u, vPrime))
@@ -187,9 +187,18 @@ type vertexAtDepth struct {
 }
 
 // depthFirstWalk does a depth-first walk of the graph starting from
-// the vertices in start. This is not exported now but it would make sense
-// to export this publicly at some point.
+// the vertices in start.
 func (g *AcyclicGraph) DepthFirstWalk(start []Vertex, f DepthWalkFunc) error {
+	return g.depthFirstWalk(start, true, f)
+}
+
+// This internal method provides the option of not sorting the vertices during
+// the walk, which we use for the Transitive reduction.
+// Some configurations can lead to fully-connected subgraphs, which makes our
+// transitive reduction algorithm O(n^3). This is still passable for the size
+// of our graphs, but the additional n^2 sort operations would make this
+// uncomputable in a reasonable amount of time.
+func (g *AcyclicGraph) depthFirstWalk(start []Vertex, sorted bool, f DepthWalkFunc) error {
 	defer g.debug.BeginOperation(typeDepthFirstWalk, "").End("")
 
 	seen := make(map[Vertex]struct{})
@@ -219,7 +228,11 @@ func (g *AcyclicGraph) DepthFirstWalk(start []Vertex, f DepthWalkFunc) error {
 
 		// Visit targets of this in a consistent order.
 		targets := AsVertexList(g.DownEdges(current.Vertex))
+
+		if sorted {
 			sort.Sort(byVertexName(targets))
+		}
+
 		for _, t := range targets {
 			frontier = append(frontier, &vertexAtDepth{
 				Vertex: t,

dag/dag_test.go

@@ -7,6 +7,7 @@ import (
 	"log"
 	"os"
 	"reflect"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -106,6 +107,61 @@ func TestAyclicGraphTransReduction_more(t *testing.T) {
 	}
 }
 
+// use this to simulate slow sort operations
+type counter struct {
+	Name  string
+	Calls int64
+}
+
+func (s *counter) String() string {
+	s.Calls++
+	return s.Name
+}
+
+// Make sure we can reduce a sizable, fully-connected graph.
+func TestAyclicGraphTransReduction_fullyConnected(t *testing.T) {
+	var g AcyclicGraph
+
+	const nodeCount = 200
+	nodes := make([]*counter, nodeCount)
+	for i := 0; i < nodeCount; i++ {
+		nodes[i] = &counter{Name: strconv.Itoa(i)}
+	}
+
+	// Add them all to the graph
+	for _, n := range nodes {
+		g.Add(n)
+	}
+
+	// connect them all
+	for i := range nodes {
+		for j := range nodes {
+			if i == j {
+				continue
+			}
+			g.Connect(BasicEdge(nodes[i], nodes[j]))
+		}
+	}
+
+	g.TransitiveReduction()
+
+	vertexNameCalls := int64(0)
+	for _, n := range nodes {
+		vertexNameCalls += n.Calls
+	}
+
+	switch {
+	case vertexNameCalls > 2*nodeCount:
+		// Make calling it more the 2x per node fatal.
+		// If we were sorting this would give us roughly ln(n)(n^3) calls, or
+		// >59000000 calls for 200 vertices.
+		t.Fatalf("VertexName called %d times", vertexNameCalls)
+	case vertexNameCalls > 0:
+		// we don't expect any calls, but a change here isn't necessarily fatal
+		t.Logf("WARNING: VertexName called %d times", vertexNameCalls)
+	}
+}
+
 func TestAcyclicGraphValidate(t *testing.T) {
 	var g AcyclicGraph
 	g.Add(1)

dag/marshal.go

@@ -273,6 +273,9 @@ func (e *encoder) Encode(i interface{}) {
 }
 
 func (e *encoder) Add(v Vertex) {
+	if e == nil {
+		return
+	}
 	e.Encode(marshalTransform{
 		Type:      typeTransform,
 		AddVertex: newMarshalVertex(v),
@@ -281,6 +284,9 @@ func (e *encoder) Add(v Vertex) {
 
 // Remove records the removal of Vertex v.
 func (e *encoder) Remove(v Vertex) {
+	if e == nil {
+		return
+	}
 	e.Encode(marshalTransform{
 		Type:         typeTransform,
 		RemoveVertex: newMarshalVertex(v),
@@ -288,6 +294,9 @@ func (e *encoder) Remove(v Vertex) {
 }
 
 func (e *encoder) Connect(edge Edge) {
+	if e == nil {
+		return
+	}
 	e.Encode(marshalTransform{
 		Type:    typeTransform,
 		AddEdge: newMarshalEdge(edge),
@@ -295,6 +304,9 @@ func (e *encoder) Connect(edge Edge) {
 }
 
 func (e *encoder) RemoveEdge(edge Edge) {
+	if e == nil {
+		return
+	}
 	e.Encode(marshalTransform{
 		Type:       typeTransform,
 		RemoveEdge: newMarshalEdge(edge),