perf: Graph cycle algorithm (#5)

* perf: Improve perf of graphHasCycles function

The `graphHasCycles` does not require the list of nodes without dependencies
to determine whether the graph has cycles or not, hence the
`nodesWithNoDependencies` parameter is removed from the function
signature.

* Change files

* change DFS algorithm to be iterative

* perf(graphHasCycles): Skip redundant searches

* Update src/graphHasCycles.ts

Co-authored-by: Kenneth Chau <34725+kenotron@users.noreply.github.com>

change/p-graph-2020-10-02-20-40-20-graph-cycle-algorithm.json

@@ -0,0 +1,8 @@
+{
+  "type": "patch",
+  "comment": "perf: Improve perf of graphHasCycles function",
+  "packageName": "p-graph",
+  "email": "olwheele@microsoft.com",
+  "dependentChangeType": "patch",
+  "date": "2020-10-02T18:40:20.038Z"
+}

src/PGraph.ts

@@ -37,7 +37,7 @@ export class PGraph {
       throw new Error("We could not find a node in the graph with no dependencies, this likely means there is a cycle including all nodes");
     }
 
-    if (graphHasCycles(this.pGraphDependencyMap, this.nodesWithNoDependencies)) {
+    if (graphHasCycles(this.pGraphDependencyMap)) {
       throw new Error("The dependency graph has a cycle in it");
     }
   }

src/graphHasCycles.ts

@@ -3,24 +3,95 @@ import { PGraphNodeWithDependencies } from "./types";
 /**
  * Checks for any cycles in the dependency graph, returning false if no cycles were detected.
  */
-export function graphHasCycles(pGraphDependencyMap: Map<string, PGraphNodeWithDependencies>, nodesWithNoDependencies: string[]): boolean {
-  const stack: { nodeId: string; visitedNodes: Set<string> }[] = [];
-  nodesWithNoDependencies.forEach((root) => stack.push({ nodeId: root, visitedNodes: new Set() }));
+export function graphHasCycles(pGraphDependencyMap: Map<string, PGraphNodeWithDependencies>): boolean {
+  /**
+   *  A map to keep track of the visited and visiting nodes.
+   * <node, true> entry means it is currently being visited.
+   * <node, false> entry means it's sub graph has been visited and is a DAG.
+   * No entry means the node has not been visited yet.
+   */
+  const visitMap = new Map<string, boolean>();
 
-  while (stack.length > 0) {
-    const { nodeId, visitedNodes } = stack.pop()!;
-
-    // If we have already seen this node, we've found a cycle
-    if (visitedNodes.has(nodeId)) {
-      return true;
+  for (const [nodeId] of pGraphDependencyMap.entries()) {
+    /**
+     * Test whether this node has already been visited or not.
+     */
+    if (!visitMap.has(nodeId)) {
+      /**
+       * Test whether the sub-graph of this node has cycles.
+       */
+      if (hasCycleDFS(pGraphDependencyMap, visitMap, nodeId)) {
+        return true;
+      }
     }
-
-    visitedNodes.add(nodeId);
-
-    const node = pGraphDependencyMap.get(nodeId)!;
-
-    [...node.dependedOnBy.keys()].forEach((childId) => stack.push({ nodeId: childId, visitedNodes: new Set(visitedNodes) }));
   }
 
   return false;
 }
+
+/**
+ * Stack element represents an item on the
+ * stack used for depth-first search
+ */
+interface StackElement {
+  /**
+   * The node name
+   */
+  node: string;
+
+  /**
+   * This represents if this instance of the
+   * node on the stack is being traversed or not
+   */
+  traversing: boolean;
+}
+
+const hasCycleDFS = (graph: Map<string, PGraphNodeWithDependencies>, visitMap: Map<string, boolean>, nodeId: string): boolean => {
+  const stack: StackElement[] = [{ node: nodeId, traversing: false }];
+  while (stack.length > 0) {
+    const current = stack[stack.length - 1];
+    if (!current.traversing) {
+      if (visitMap.has(current.node)) {
+        if (visitMap.get(current.node)) {
+          /**
+           * The current node has already been visited,
+           * hence there is a cycle.
+           */
+          return true;
+        } else {
+          /**
+           * The current node has already been fully traversed
+           */
+          stack.pop();
+          continue;
+        }
+      }
+
+      /**
+       * The current node is starting its traversal
+       */
+      visitMap.set(current.node, true);
+      stack[stack.length - 1] = { ...current, traversing: true };
+
+      /**
+       * Get the current node in the graph
+       */
+      const node = graph.get(current.node);
+      if (!node) {
+        throw new Error(`Could not find node "${current.node}" in the graph`);
+      }
+
+      /**
+       * Add the current node's dependencies to the stack
+       */
+      stack.push(...[...node.dependsOn].map((n) => ({ node: n, traversing: false })));
+    } else {
+      /**
+       * The current node has now been fully traversed.
+       */
+      visitMap.set(current.node, false);
+      stack.pop();
+    }
+  }
+  return false;
+};