Merge pull request #36 from jriecken/fix-perf

Rewrite the topological sort DFS to be more efficient

CHANGELOG.md

@@ -46,8 +46,8 @@
 ## 0.4.0 (Aug 1, 2015)
 
 - Better error messages
-    - When a cycle is detected, the error message will now include the cycle in it. E.g `Dependency Cycle Found: a -> b -> c -> a` (Fixes #7)
-    - When calling `addDependency` if one of the nodes does not exist, the error will say which one it was (instead of saying that "one" of the two nodes did not exist and making you manually determine which one)
+  - When a cycle is detected, the error message will now include the cycle in it. E.g `Dependency Cycle Found: a -> b -> c -> a` (Fixes #7)
+  - When calling `addDependency` if one of the nodes does not exist, the error will say which one it was (instead of saying that "one" of the two nodes did not exist and making you manually determine which one)
 - Calling `overallOrder` on an empty graph will no longer throw an error about a dependency cycle. It will return an empty array.
 
 ## 0.3.0 (July 24, 2015)

LICENSE

@@ -1,4 +1,4 @@
-Copyright (C) 2013-2015 by Jim Riecken
+Copyright (C) 2013-2020 by Jim Riecken
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

lib/dep_graph.js

@@ -3,59 +3,88 @@
  */
 
 /**
- * Helper for creating a Depth-First-Search on
- * a set of edges.
+ * Helper for creating a Topological Sort using Depth-First-Search on a set of edges.
  *
- * Detects cycles and throws an Error if one is detected.
+ * Detects cycles and throws an Error if one is detected (unless the "circular"
+ * parameter is "true" in which case it ignores them).
  *
  * @param edges The set of edges to DFS through
  * @param leavesOnly Whether to only return "leaf" nodes (ones who have no edges)
  * @param result An array in which the results will be populated
  * @param circular A boolean to allow circular dependencies
  */
 function createDFS(edges, leavesOnly, result, circular) {
-  var currentPath = [];
   var visited = {};
-  return function DFS(currentNode) {
-    visited[currentNode] = true;
-    currentPath.push(currentNode);
-    edges[currentNode].forEach(function (node) {
-      if (!visited[node]) {
-        DFS(node);
-      } else if (currentPath.indexOf(node) >= 0) {
-        currentPath.push(node);
-        if (!circular) {
+  return function(start) {
+    if (visited[start]) {
+      return;
+    }
+    var inCurrentPath = {};
+    var currentPath = [];
+    var todo = []; // used as a stack
+    todo.push({ node: start, processed: false });
+    while (todo.length > 0) {
+      var current = todo[todo.length - 1]; // peek at the todo stack
+      var processed = current.processed;
+      var node = current.node;
+      if (!processed) {
+        // Haven't visited edges yet (visiting phase)
+        if (visited[node]) {
+          todo.pop();
+          continue;
+        } else if (inCurrentPath[node]) {
+          // It's not a DAG
+          if (circular) {
+            todo.pop();
+            // If we're tolerating cycles, don't revisit the node
+            continue;
+          }
+          currentPath.push(node);
           throw new DepGraphCycleError(currentPath);
         }
+
+        inCurrentPath[node] = true;
+        currentPath.push(node);
+        var nodeEdges = edges[node];
+        // (push edges onto the todo stack in reverse order to be order-compatible with the old DFS implementation)
+        for (var i = nodeEdges.length - 1; i >= 0; i--) {
+          todo.push({ node: nodeEdges[i], processed: false });
+        }
+        current.processed = true;
+      } else {
+        // Have visited edges (stack unrolling phase)
+        todo.pop();
+        currentPath.pop();
+        inCurrentPath[node] = false;
+        visited[node] = true;
+        if (!leavesOnly || edges[node].length === 0) {
+          result.push(node);
+        }
       }
-    });
-    currentPath.pop();
-    if ((!leavesOnly || edges[currentNode].length === 0) && result.indexOf(currentNode) === -1) {
-      result.push(currentNode);
     }
   };
 }
 
 /**
  * Simple Dependency Graph
  */
-var DepGraph = exports.DepGraph = function DepGraph(opts) {
+var DepGraph = (exports.DepGraph = function DepGraph(opts) {
   this.nodes = {}; // Node -> Node/Data (treated like a Set)
   this.outgoingEdges = {}; // Node -> [Dependency Node]
   this.incomingEdges = {}; // Node -> [Dependant Node]
   this.circular = opts && !!opts.circular; // Allows circular deps
-};
+});
 DepGraph.prototype = {
   /**
    * The number of nodes in the graph.
    */
-  size: function () {
+  size: function() {
     return Object.keys(this.nodes).length;
   },
   /**
    * Add a node to the dependency graph. If a node already exists, this method will do nothing.
    */
-  addNode: function (node, data) {
+  addNode: function(node, data) {
     if (!this.hasNode(node)) {
       // Checking the arguments length allows the user to add a node with undefined data
       if (arguments.length === 2) {
@@ -70,13 +99,13 @@ DepGraph.prototype = {
   /**
    * Remove a node from the dependency graph. If a node does not exist, this method will do nothing.
    */
-  removeNode: function (node) {
+  removeNode: function(node) {
     if (this.hasNode(node)) {
       delete this.nodes[node];
       delete this.outgoingEdges[node];
       delete this.incomingEdges[node];
-      [this.incomingEdges, this.outgoingEdges].forEach(function (edgeList) {
-        Object.keys(edgeList).forEach(function (key) {
+      [this.incomingEdges, this.outgoingEdges].forEach(function(edgeList) {
+        Object.keys(edgeList).forEach(function(key) {
           var idx = edgeList[key].indexOf(node);
           if (idx >= 0) {
             edgeList[key].splice(idx, 1);
@@ -88,39 +117,39 @@ DepGraph.prototype = {
   /**
    * Check if a node exists in the graph
    */
-  hasNode: function (node) {
+  hasNode: function(node) {
     return this.nodes.hasOwnProperty(node);
   },
   /**
    * Get the data associated with a node name
    */
-  getNodeData: function (node) {
+  getNodeData: function(node) {
     if (this.hasNode(node)) {
       return this.nodes[node];
     } else {
-      throw new Error('Node does not exist: ' + node);
+      throw new Error("Node does not exist: " + node);
     }
   },
   /**
    * Set the associated data for a given node name. If the node does not exist, this method will throw an error
    */
-  setNodeData: function (node, data) {
+  setNodeData: function(node, data) {
     if (this.hasNode(node)) {
       this.nodes[node] = data;
     } else {
-      throw new Error('Node does not exist: ' + node);
+      throw new Error("Node does not exist: " + node);
     }
   },
   /**
    * Add a dependency between two nodes. If either of the nodes does not exist,
    * an Error will be thrown.
    */
-  addDependency: function (from, to) {
+  addDependency: function(from, to) {
     if (!this.hasNode(from)) {
-      throw new Error('Node does not exist: ' + from);
+      throw new Error("Node does not exist: " + from);
     }
     if (!this.hasNode(to)) {
-      throw new Error('Node does not exist: ' + to);
+      throw new Error("Node does not exist: " + to);
     }
     if (this.outgoingEdges[from].indexOf(to) === -1) {
       this.outgoingEdges[from].push(to);
@@ -133,7 +162,7 @@ DepGraph.prototype = {
   /**
    * Remove a dependency between two nodes.
    */
-  removeDependency: function (from, to) {
+  removeDependency: function(from, to) {
     var idx;
     if (this.hasNode(from)) {
       idx = this.outgoingEdges[from].indexOf(to);
@@ -153,11 +182,11 @@ DepGraph.prototype = {
    * Return a clone of the dependency graph. If any custom data is attached
    * to the nodes, it will only be shallow copied.
    */
-  clone: function () {
+  clone: function() {
     var source = this;
     var result = new DepGraph();
     var keys = Object.keys(source.nodes);
-    keys.forEach(function (n) {
+    keys.forEach(function(n) {
       result.nodes[n] = source.nodes[n];
       result.outgoingEdges[n] = source.outgoingEdges[n].slice(0);
       result.incomingEdges[n] = source.incomingEdges[n].slice(0);
@@ -172,19 +201,23 @@ DepGraph.prototype = {
    * If `leavesOnly` is true, only nodes that do not depend on any other nodes will be returned
    * in the array.
    */
-  dependenciesOf: function (node, leavesOnly) {
+  dependenciesOf: function(node, leavesOnly) {
     if (this.hasNode(node)) {
       var result = [];
-      var DFS = createDFS(this.outgoingEdges, leavesOnly, result, this.circular);
+      var DFS = createDFS(
+        this.outgoingEdges,
+        leavesOnly,
+        result,
+        this.circular
+      );
       DFS(node);
       var idx = result.indexOf(node);
       if (idx >= 0) {
         result.splice(idx, 1);
       }
       return result;
-    }
-    else {
-      throw new Error('Node does not exist: ' + node);
+    } else {
+      throw new Error("Node does not exist: " + node);
     }
   },
   /**
@@ -194,18 +227,23 @@ DepGraph.prototype = {
    *
    * If `leavesOnly` is true, only nodes that do not have any dependants will be returned in the array.
    */
-  dependantsOf: function (node, leavesOnly) {
+  dependantsOf: function(node, leavesOnly) {
     if (this.hasNode(node)) {
       var result = [];
-      var DFS = createDFS(this.incomingEdges, leavesOnly, result, this.circular);
+      var DFS = createDFS(
+        this.incomingEdges,
+        leavesOnly,
+        result,
+        this.circular
+      );
       DFS(node);
       var idx = result.indexOf(node);
       if (idx >= 0) {
         result.splice(idx, 1);
       }
       return result;
     } else {
-      throw new Error('Node does not exist: ' + node);
+      throw new Error("Node does not exist: " + node);
     }
   },
   /**
@@ -215,7 +253,7 @@ DepGraph.prototype = {
    *
    * If `leavesOnly` is true, only nodes that do not depend on any other nodes will be returned.
    */
-  overallOrder: function (leavesOnly) {
+  overallOrder: function(leavesOnly) {
     var self = this;
     var result = [];
     var keys = Object.keys(this.nodes);
@@ -226,29 +264,38 @@ DepGraph.prototype = {
         // Look for cycles - we run the DFS starting at all the nodes in case there
         // are several disconnected subgraphs inside this dependency graph.
         var CycleDFS = createDFS(this.outgoingEdges, false, [], this.circular);
-        keys.forEach(function (n) {
+        keys.forEach(function(n) {
           CycleDFS(n);
         });
       }
 
-      var DFS = createDFS(this.outgoingEdges, leavesOnly, result, this.circular);
+      var DFS = createDFS(
+        this.outgoingEdges,
+        leavesOnly,
+        result,
+        this.circular
+      );
       // Find all potential starting points (nodes with nothing depending on them) an
       // run a DFS starting at these points to get the order
-      keys.filter(function (node) {
-        return self.incomingEdges[node].length === 0;
-      }).forEach(function (n) {
-        DFS(n);
-      });
+      keys
+        .filter(function(node) {
+          return self.incomingEdges[node].length === 0;
+        })
+        .forEach(function(n) {
+          DFS(n);
+        });
 
       // If we're allowing cycles - we need to run the DFS against any remaining
       // nodes that did not end up in the initial result (as they are part of a
       // subgraph that does not have a clear starting point)
       if (this.circular) {
-        keys.filter(function (node) {
-          return result.indexOf(node) === -1;
-        }).forEach(function (n) {
-          DFS(n);
-        });
+        keys
+          .filter(function(node) {
+            return result.indexOf(node) === -1;
+          })
+          .forEach(function(n) {
+            DFS(n);
+          });
       }
 
       return result;
@@ -259,16 +306,16 @@ DepGraph.prototype = {
 /**
  * Cycle error, including the path of the cycle.
  */
-var DepGraphCycleError = exports.DepGraphCycleError = function (cyclePath) {
-  var message = 'Dependency Cycle Found: ' + cyclePath.join(' -> ')
+var DepGraphCycleError = (exports.DepGraphCycleError = function(cyclePath) {
+  var message = "Dependency Cycle Found: " + cyclePath.join(" -> ");
   var instance = new Error(message);
   instance.cyclePath = cyclePath;
   Object.setPrototypeOf(instance, Object.getPrototypeOf(this));
   if (Error.captureStackTrace) {
     Error.captureStackTrace(instance, DepGraphCycleError);
   }
   return instance;
-}
+});
 DepGraphCycleError.prototype = Object.create(Error.prototype, {
   constructor: {
     value: Error,