From 6096371068cef7709c88f2f1558c6a24cc70e5c0 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 7 Jan 2020 17:28:56 -0500 Subject: [PATCH] remove all unneeded list-based iteration Make the default walks always use the sets directly, which avoids repeated copying of every set into a new slice. --- dag/dag.go | 108 ++++++++++++++++++++++++++++++++++---------- dag/dag_test.go | 2 +- dag/marshal_test.go | 43 ------------------ 3 files changed, 86 insertions(+), 67 deletions(-) diff --git a/dag/dag.go b/dag/dag.go index ea78d2c3c..c77e775fd 100644 --- a/dag/dag.go +++ b/dag/dag.go @@ -31,13 +31,12 @@ func (g *AcyclicGraph) DirectedGraph() Grapher { // provided starting Vertex v. func (g *AcyclicGraph) Ancestors(v Vertex) (Set, error) { s := make(Set) - start := AsVertexList(g.DownEdges(v)) memoFunc := func(v Vertex, d int) error { s.Add(v) return nil } - if err := g.DepthFirstWalk(start, memoFunc); err != nil { + if err := g.DepthFirstWalk(g.DownEdges(v), memoFunc); err != nil { return nil, err } @@ -48,13 +47,12 @@ func (g *AcyclicGraph) Ancestors(v Vertex) (Set, error) { // provided starting Vertex v. func (g *AcyclicGraph) Descendents(v Vertex) (Set, error) { s := make(Set) - start := AsVertexList(g.UpEdges(v)) memoFunc := func(v Vertex, d int) error { s.Add(v) return nil } - if err := g.ReverseDepthFirstWalk(start, memoFunc); err != nil { + if err := g.ReverseDepthFirstWalk(g.UpEdges(v), memoFunc); err != nil { return nil, err } @@ -106,11 +104,10 @@ func (g *AcyclicGraph) TransitiveReduction() { for _, u := range g.Vertices() { uTargets := g.DownEdges(u) - vs := AsVertexList(g.DownEdges(u)) - g.depthFirstWalk(vs, false, func(v Vertex, d int) error { + g.DepthFirstWalk(g.DownEdges(u), func(v Vertex, d int) error { shared := uTargets.Intersection(g.DownEdges(v)) - for _, vPrime := range AsVertexList(shared) { + for _, vPrime := range shared { g.RemoveEdge(BasicEdge(u, vPrime)) } @@ -187,19 +184,48 @@ type vertexAtDepth struct { Depth int } -// depthFirstWalk does a depth-first walk of the graph starting from +// DepthFirstWalk does a depth-first walk of the graph starting from // the vertices in start. -func (g *AcyclicGraph) DepthFirstWalk(start []Vertex, f DepthWalkFunc) error { - return g.depthFirstWalk(start, true, f) +func (g *AcyclicGraph) DepthFirstWalk(start Set, f DepthWalkFunc) error { + seen := make(map[Vertex]struct{}) + frontier := make([]*vertexAtDepth, 0, len(start)) + for _, v := range start { + frontier = append(frontier, &vertexAtDepth{ + Vertex: v, + Depth: 0, + }) + } + for len(frontier) > 0 { + // Pop the current vertex + n := len(frontier) + current := frontier[n-1] + frontier = frontier[:n-1] + + // Check if we've seen this already and return... + if _, ok := seen[current.Vertex]; ok { + continue + } + seen[current.Vertex] = struct{}{} + + // Visit the current node + if err := f(current.Vertex, current.Depth); err != nil { + return err + } + + for _, v := range g.DownEdges(current.Vertex) { + frontier = append(frontier, &vertexAtDepth{ + Vertex: v, + Depth: current.Depth + 1, + }) + } + } + + return nil } -// 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 { +// SortedDepthFirstWalk does a depth-first walk of the graph starting from +// the vertices in start, always iterating the nodes in a consistent order. +func (g *AcyclicGraph) SortedDepthFirstWalk(start []Vertex, f DepthWalkFunc) error { defer g.debug.BeginOperation(typeDepthFirstWalk, "").End("") seen := make(map[Vertex]struct{}) @@ -229,10 +255,7 @@ func (g *AcyclicGraph) depthFirstWalk(start []Vertex, sorted bool, f DepthWalkFu // Visit targets of this in a consistent order. targets := AsVertexList(g.DownEdges(current.Vertex)) - - if sorted { - sort.Sort(byVertexName(targets)) - } + sort.Sort(byVertexName(targets)) for _, t := range targets { frontier = append(frontier, &vertexAtDepth{ @@ -245,9 +268,48 @@ func (g *AcyclicGraph) depthFirstWalk(start []Vertex, sorted bool, f DepthWalkFu return nil } -// reverseDepthFirstWalk does a depth-first walk _up_ the graph starting from +// ReverseDepthFirstWalk does a depth-first walk _up_ the graph starting from // the vertices in start. -func (g *AcyclicGraph) ReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) error { +func (g *AcyclicGraph) ReverseDepthFirstWalk(start Set, f DepthWalkFunc) error { + seen := make(map[Vertex]struct{}) + frontier := make([]*vertexAtDepth, 0, len(start)) + for _, v := range start { + frontier = append(frontier, &vertexAtDepth{ + Vertex: v, + Depth: 0, + }) + } + for len(frontier) > 0 { + // Pop the current vertex + n := len(frontier) + current := frontier[n-1] + frontier = frontier[:n-1] + + // Check if we've seen this already and return... + if _, ok := seen[current.Vertex]; ok { + continue + } + seen[current.Vertex] = struct{}{} + + for _, t := range g.UpEdges(current.Vertex) { + frontier = append(frontier, &vertexAtDepth{ + Vertex: t, + Depth: current.Depth + 1, + }) + } + + // Visit the current node + if err := f(current.Vertex, current.Depth); err != nil { + return err + } + } + + return nil +} + +// SortedReverseDepthFirstWalk does a depth-first walk _up_ the graph starting from +// the vertices in start, always iterating the nodes in a consistent order. +func (g *AcyclicGraph) SortedReverseDepthFirstWalk(start []Vertex, f DepthWalkFunc) error { defer g.debug.BeginOperation(typeReverseDepthFirstWalk, "").End("") seen := make(map[Vertex]struct{}) diff --git a/dag/dag_test.go b/dag/dag_test.go index 222df257e..d3da8a743 100644 --- a/dag/dag_test.go +++ b/dag/dag_test.go @@ -345,7 +345,7 @@ func TestAcyclicGraph_ReverseDepthFirstWalk_WithRemoval(t *testing.T) { var visits []Vertex var lock sync.Mutex - err := g.ReverseDepthFirstWalk([]Vertex{1}, func(v Vertex, d int) error { + err := g.SortedReverseDepthFirstWalk([]Vertex{1}, func(v Vertex, d int) error { lock.Lock() defer lock.Unlock() visits = append(visits, v) diff --git a/dag/marshal_test.go b/dag/marshal_test.go index c2f52a936..e5839c887 100644 --- a/dag/marshal_test.go +++ b/dag/marshal_test.go @@ -117,49 +117,6 @@ func TestGraphJSON_basic(t *testing.T) { } } -// record some graph transformations, and make sure we get the same graph when -// they're replayed -func TestGraphJSON_basicRecord(t *testing.T) { - var g Graph - var buf bytes.Buffer - g.SetDebugWriter(&buf) - - g.Add(1) - g.Add(2) - g.Add(3) - g.Connect(BasicEdge(1, 2)) - g.Connect(BasicEdge(1, 3)) - g.Connect(BasicEdge(2, 3)) - (&AcyclicGraph{g}).TransitiveReduction() - - recorded := buf.Bytes() - // the Walk doesn't happen in a determined order, so just count operations - // for now to make sure we wrote stuff out. - if len(bytes.Split(recorded, []byte{'\n'})) != 17 { - t.Fatalf("bad: %s", recorded) - } - - original, err := g.MarshalJSON() - if err != nil { - t.Fatal(err) - } - - // replay the logs, and marshal the graph back out again - m, err := decodeGraph(bytes.NewReader(buf.Bytes())) - if err != nil { - t.Fatal(err) - } - - replayed, err := json.MarshalIndent(m, "", " ") - if err != nil { - t.Fatal(err) - } - - if !bytes.Equal(original, replayed) { - t.Fatalf("\noriginal: %s\nreplayed: %s", original, replayed) - } -} - // Verify that Vertex and Edge annotations appear in the debug output func TestGraphJSON_debugInfo(t *testing.T) { var g Graph