From ed2075e3847552bf825dde988a0140a5efc0a2ab Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Feb 2015 19:12:19 -0800 Subject: [PATCH 1/4] dag: TransitiveReduction --- dag/dag.go | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ dag/dag_test.go | 57 +++++++++++++++++++++++++++++++++++++ dag/set.go | 14 ++++++++++ 3 files changed, 145 insertions(+) diff --git a/dag/dag.go b/dag/dag.go index 9490cf98a..d319e84c5 100644 --- a/dag/dag.go +++ b/dag/dag.go @@ -40,6 +40,46 @@ func (g *AcyclicGraph) Root() (Vertex, error) { return roots[0], nil } +// TransitiveReduction performs the transitive reduction of graph g in place. +// The transitive reduction of a graph is a graph with as few edges as +// possible with the same reachability as the original graph. This means +// that if there are three nodes A => B => C, and A connects to both +// B and C, and B connects to C, then the transitive reduction is the +// same graph with only a single edge between A and B, and a single edge +// between B and C. +// +// The graph must be valid for this operation to behave properly. If +// Validate() returns an error, the behavior is undefined and the results +// will likely be unexpected. +// +// Complexity: O(V(V+E)), or asymptotically O(VE) +func (g *AcyclicGraph) TransitiveReduction() { + // Find the root-like objects to start a depthFirstWalk from. + frontier := make([]Vertex, 0, 5) + for _, v := range g.Vertices() { + if g.UpEdges(v).Len() == 0 { + frontier = append(frontier, v) + } + } + + // Do a DFS + g.depthFirstWalk(frontier, func(v Vertex) error { + parents := g.UpEdges(v).List() + targets := g.DownEdges(v) + + for _, rawParent := range parents { + parent := rawParent.(Vertex) + shared := g.DownEdges(parent).Intersection(targets) + for _, rawTarget := range shared.List() { + target := rawTarget.(Vertex) + g.RemoveEdge(BasicEdge(parent, target)) + } + } + + return nil + }) +} + // Validate validates the DAG. A DAG is valid if it has a single root // with no cycles. func (g *AcyclicGraph) Validate() error { @@ -161,3 +201,37 @@ func (g *AcyclicGraph) Walk(cb WalkFunc) error { <-doneCh return errs } + +// 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. +func (g *AcyclicGraph) depthFirstWalk(start []Vertex, cb WalkFunc) error { + seen := make(map[Vertex]struct{}) + frontier := make([]Vertex, len(start)) + copy(frontier, start) + 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]; ok { + continue + } + seen[current] = struct{}{} + + // Visit the current node + if err := cb(current); err != nil { + return err + } + + // Visit targets of this in reverse order. + targets := g.DownEdges(current).List() + for i := len(targets) - 1; i >= 0; i-- { + frontier = append(frontier, targets[i].(Vertex)) + } + } + + return nil +} diff --git a/dag/dag_test.go b/dag/dag_test.go index cc9442be5..feead7968 100644 --- a/dag/dag_test.go +++ b/dag/dag_test.go @@ -3,6 +3,7 @@ package dag import ( "fmt" "reflect" + "strings" "sync" "testing" ) @@ -48,6 +49,44 @@ func TestAcyclicGraphRoot_multiple(t *testing.T) { } } +func TestAyclicGraphTransReduction(t *testing.T) { + var g AcyclicGraph + g.Add(1) + g.Add(2) + g.Add(3) + g.Connect(BasicEdge(1, 2)) + g.Connect(BasicEdge(1, 3)) + g.Connect(BasicEdge(2, 3)) + g.TransitiveReduction() + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testGraphTransReductionStr) + if actual != expected { + t.Fatalf("bad: %s", actual) + } +} + +func TestAyclicGraphTransReduction_more(t *testing.T) { + var g AcyclicGraph + g.Add(1) + g.Add(2) + g.Add(3) + g.Add(4) + g.Connect(BasicEdge(1, 2)) + g.Connect(BasicEdge(1, 3)) + g.Connect(BasicEdge(1, 4)) + g.Connect(BasicEdge(2, 3)) + g.Connect(BasicEdge(2, 4)) + g.Connect(BasicEdge(3, 4)) + g.TransitiveReduction() + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testGraphTransReductionMoreStr) + if actual != expected { + t.Fatalf("bad: %s", actual) + } +} + func TestAcyclicGraphValidate(t *testing.T) { var g AcyclicGraph g.Add(1) @@ -156,3 +195,21 @@ func TestAcyclicGraphWalk_error(t *testing.T) { t.Fatalf("bad: %#v", visits) } + +const testGraphTransReductionStr = ` +1 + 2 +2 + 3 +3 +` + +const testGraphTransReductionMoreStr = ` +1 + 2 +2 + 3 +3 + 4 +4 +` diff --git a/dag/set.go b/dag/set.go index 2b1d7f21b..9cc0d98e2 100644 --- a/dag/set.go +++ b/dag/set.go @@ -36,6 +36,20 @@ func (s *Set) Include(v interface{}) bool { return ok } +// Intersection computes the set intersection with other. +func (s *Set) Intersection(other *Set) *Set { + result := new(Set) + if other != nil { + for _, v := range s.m { + if other.Include(v) { + result.Add(v) + } + } + } + + return result +} + // Len is the number of items in the set. func (s *Set) Len() int { if s == nil { From 903e49162daffe573909fb8e230860aa86eac44c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Feb 2015 19:18:04 -0800 Subject: [PATCH 2/4] terraform: add TransitiveReductionTransformer --- .../transform-trans-reduce-basic/main.tf | 10 +++++ terraform/transform_transitive_reduction.go | 20 ++++++++++ .../transform_transitive_reduction_test.go | 39 +++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 terraform/test-fixtures/transform-trans-reduce-basic/main.tf create mode 100644 terraform/transform_transitive_reduction.go create mode 100644 terraform/transform_transitive_reduction_test.go diff --git a/terraform/test-fixtures/transform-trans-reduce-basic/main.tf b/terraform/test-fixtures/transform-trans-reduce-basic/main.tf new file mode 100644 index 000000000..4fb97c7a7 --- /dev/null +++ b/terraform/test-fixtures/transform-trans-reduce-basic/main.tf @@ -0,0 +1,10 @@ +resource "aws_instance" "A" {} + +resource "aws_instance" "B" { + A = "${aws_instance.A.id}" +} + +resource "aws_instance" "C" { + A = "${aws_instance.A.id}" + B = "${aws_instance.B.id}" +} diff --git a/terraform/transform_transitive_reduction.go b/terraform/transform_transitive_reduction.go new file mode 100644 index 000000000..21842789c --- /dev/null +++ b/terraform/transform_transitive_reduction.go @@ -0,0 +1,20 @@ +package terraform + +// TransitiveReductionTransformer is a GraphTransformer that performs +// finds the transitive reduction of the graph. For a definition of +// transitive reduction, see Wikipedia. +type TransitiveReductionTransformer struct{} + +func (t *TransitiveReductionTransformer) Transform(g *Graph) error { + // If the graph isn't valid, skip the transitive reduction. + // We don't error here because Terraform itself handles graph + // validation in a better way, or we assume it does. + if err := g.Validate(); err != nil { + return nil + } + + // Do it + g.TransitiveReduction() + + return nil +} diff --git a/terraform/transform_transitive_reduction_test.go b/terraform/transform_transitive_reduction_test.go new file mode 100644 index 000000000..bcd3b7d23 --- /dev/null +++ b/terraform/transform_transitive_reduction_test.go @@ -0,0 +1,39 @@ +package terraform + +import ( + "strings" + "testing" +) + +func TestTransitiveReductionTransformer(t *testing.T) { + mod := testModule(t, "transform-trans-reduce-basic") + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + transform := &TransitiveReductionTransformer{} + if err := transform.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformTransReduceBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + +const testTransformTransReduceBasicStr = ` +aws_instance.A +aws_instance.B + aws_instance.A +aws_instance.C + aws_instance.B +` From 9eb7ebbddddcfe346e1104fe82c62cfbe8af18f6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Feb 2015 19:23:20 -0800 Subject: [PATCH 3/4] terraform: do the transitive reduction as part of the graph builder --- terraform/graph_builder.go | 4 ++++ terraform/graph_builder_test.go | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index e70d58752..6368e54dc 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -109,5 +109,9 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer { // Make sure we create one root &RootTransformer{}, + + // Perform the transitive reduction to make our graph a bit + // more sane if possible (it usually is possible). + &TransitiveReductionTransformer{}, } } diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index ee1dbc1f5..882d054b9 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -125,14 +125,11 @@ const testBasicGraphBuilderStr = ` const testBuiltinGraphBuilderBasicStr = ` aws_instance.db aws_instance.db (destroy tainted) - provider.aws aws_instance.db (destroy tainted) aws_instance.web (destroy tainted) - provider.aws aws_instance.web aws_instance.db aws_instance.web (destroy tainted) - provider.aws aws_instance.web (destroy tainted) provider.aws provider.aws From 865de51816ed7bafe0f93b5de0f12a7e8e5d46e1 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 27 Feb 2015 19:37:59 -0800 Subject: [PATCH 4/4] dag: do a DFS for each vertex --- dag/dag.go | 38 ++++++++++++++++----------------- terraform/graph_builder_test.go | 1 - 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/dag/dag.go b/dag/dag.go index d319e84c5..b81cb2874 100644 --- a/dag/dag.go +++ b/dag/dag.go @@ -54,30 +54,28 @@ func (g *AcyclicGraph) Root() (Vertex, error) { // // Complexity: O(V(V+E)), or asymptotically O(VE) func (g *AcyclicGraph) TransitiveReduction() { - // Find the root-like objects to start a depthFirstWalk from. - frontier := make([]Vertex, 0, 5) - for _, v := range g.Vertices() { - if g.UpEdges(v).Len() == 0 { - frontier = append(frontier, v) + // For each vertex u in graph g, do a DFS starting from each vertex + // v such that the edge (u,v) exists (v is a direct descendant of u). + // + // For each v-prime reachable from v, remove the edge (u, v-prime). + + for _, u := range g.Vertices() { + uTargets := g.DownEdges(u) + vs := make([]Vertex, uTargets.Len()) + for i, vRaw := range uTargets.List() { + vs[i] = vRaw.(Vertex) } - } - // Do a DFS - g.depthFirstWalk(frontier, func(v Vertex) error { - parents := g.UpEdges(v).List() - targets := g.DownEdges(v) - - for _, rawParent := range parents { - parent := rawParent.(Vertex) - shared := g.DownEdges(parent).Intersection(targets) - for _, rawTarget := range shared.List() { - target := rawTarget.(Vertex) - g.RemoveEdge(BasicEdge(parent, target)) + g.depthFirstWalk(vs, func(v Vertex) error { + shared := uTargets.Intersection(g.DownEdges(v)) + for _, raw := range shared.List() { + vPrime := raw.(Vertex) + g.RemoveEdge(BasicEdge(u, vPrime)) } - } - return nil - }) + return nil + }) + } } // Validate validates the DAG. A DAG is valid if it has a single root diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index 882d054b9..2f072abab 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -129,7 +129,6 @@ aws_instance.db (destroy tainted) aws_instance.web (destroy tainted) aws_instance.web aws_instance.db - aws_instance.web (destroy tainted) aws_instance.web (destroy tainted) provider.aws provider.aws