diff --git a/dag/dag.go b/dag/dag.go index f8776bc51..b7eb10c33 100644 --- a/dag/dag.go +++ b/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, diff --git a/dag/dag_test.go b/dag/dag_test.go index 4f8aa3d0d..ae9d87244 100644 --- a/dag/dag_test.go +++ b/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) diff --git a/dag/marshal.go b/dag/marshal.go index 16d5dd6dd..c567d2719 100644 --- a/dag/marshal.go +++ b/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),