From 30a4f39ee010c45d649d50951785f31c0e7e688a Mon Sep 17 00:00:00 2001 From: Priti Desai Date: Tue, 17 Nov 2020 23:11:34 -0800 Subject: [PATCH] fix cycle detection logic visited map holds name of the nodes been visited as keys and "true" as value. This map is being updated with currentName.HashKey on every iteration which results in keys such as a.d, b.d where a, b, and d are nodes of a dag. But such keys are not utilized anywhere in the visit function. Visit function checks existence of the node just by the name without any string concatenation. This extra addition in the map is causing severe delay for a graph with more than >60 nodes. --- pkg/reconciler/pipeline/dag/dag.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pkg/reconciler/pipeline/dag/dag.go b/pkg/reconciler/pipeline/dag/dag.go index 16f39fc4fce..d632aec0d98 100644 --- a/pkg/reconciler/pipeline/dag/dag.go +++ b/pkg/reconciler/pipeline/dag/dag.go @@ -128,7 +128,7 @@ func linkPipelineTasks(prev *Node, next *Node) error { // Check if we are adding cycles. visited := map[string]bool{prev.Task.HashKey(): true, next.Task.HashKey(): true} path := []string{next.Task.HashKey(), prev.Task.HashKey()} - if err := visit(next.Task.HashKey(), prev.Prev, path, visited); err != nil { + if err := visit(prev.Prev, path, visited); err != nil { return fmt.Errorf("cycle detected: %w", err) } next.Prev = append(next.Prev, prev) @@ -136,18 +136,13 @@ func linkPipelineTasks(prev *Node, next *Node) error { return nil } -func visit(currentName string, nodes []*Node, path []string, visited map[string]bool) error { - var sb strings.Builder +func visit(nodes []*Node, path []string, visited map[string]bool) error { for _, n := range nodes { path = append(path, n.Task.HashKey()) if _, ok := visited[n.Task.HashKey()]; ok { return errors.New(getVisitedPath(path)) } - sb.WriteString(currentName) - sb.WriteByte('.') - sb.WriteString(n.Task.HashKey()) - visited[sb.String()] = true - if err := visit(n.Task.HashKey(), n.Prev, path, visited); err != nil { + if err := visit(n.Prev, path, visited); err != nil { return err } }