faster DAG transitive reduction

In the case of highly-connected graphs, the TransitiveReduction process
was far too computationally intensive. Since no operations are applied
to the nodes, and the walk order is not even user visible, we don't need
to sort them n^2 times.

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))
-		sort.Sort(byVertexName(targets))
+
+		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),