Merge pull request #149 from snyk/feature/count-paths-to-root-perf-improvements

fix(ts): improve runtime performance of countPathsToRoot

Author: mcombuechen

src/core/dep-graph.ts

@@ -153,16 +153,7 @@ class DepGraphImpl implements types.DepGraphInternal {
     let count = 0;
     const limit = opts?.limit;
     for (const nodeId of this.getPkgNodeIds(pkg)) {
-      if (this._countNodePathsToRootCache.has(nodeId)) {
-        count += this._countNodePathsToRootCache.get(nodeId)!;
-      } else {
-        const c = this.countNodePathsToRoot(nodeId, limit);
-        // don't cache if a limit was supplied
-        if (!limit) {
-          this._countNodePathsToRootCache.set(nodeId, c);
-        }
-        count += c;
-      }
+      count += this.countNodePathsToRoot(nodeId, limit);
       if (limit && count >= limit) {
         return limit;
       }
@@ -379,7 +370,50 @@ class DepGraphImpl implements types.DepGraphInternal {
     return allPaths;
   }
 
-  private countNodePathsToRoot(
+  private countNodePathsToRoot(nodeId: string, limit?: number): number {
+    if (this._countNodePathsToRootCache.has(nodeId)) {
+      return this._countNodePathsToRootCache.get(nodeId)!;
+    }
+
+    let count: number;
+    if (nodeId === this._rootNodeId) {
+      count = 1;
+    } else if (this.hasCycles()) {
+      // When cycles exist, we must walk every path individually to avoid
+      // counting paths that loop back on themselves.
+      count = this.countNodePathsToRootBacktracking(nodeId, limit);
+    } else {
+      // Without cycles, we can compute path counts by summing each parent's
+      // count. This avoids the exponential cost of enumerating every path.
+      count = this.countNodePathsToRootMemoized(nodeId, limit);
+    }
+
+    if (!limit) {
+      this._countNodePathsToRootCache.set(nodeId, count);
+    }
+    return count;
+  }
+
+  /**
+   * Memoized path counting, the count is the sum of the counts of the parents.
+   * Only valid for acyclic graphs.
+   */
+  private countNodePathsToRootMemoized(nodeId: string, limit = 0): number {
+    let count = 0;
+    for (const parentNodeId of this.getNodeParentsNodeIds(nodeId)) {
+      count += this.countNodePathsToRoot(parentNodeId, limit);
+      if (limit && count >= limit) {
+        return limit;
+      }
+    }
+    return count;
+  }
+
+  /**
+   * Backtracking approach that enumerates simple paths.
+   * Required for cyclic graphs where memoization is not applicable.
+   */
+  private countNodePathsToRootBacktracking(
     nodeId: string,
     limit = 0,
     count = 0,
@@ -390,12 +424,18 @@ class DepGraphImpl implements types.DepGraphInternal {
     }
     visited.add(nodeId);
     for (const parentNodeId of this.getNodeParentsNodeIds(nodeId)) {
-      if (!visited.has(parentNodeId)) {
-        count = this.countNodePathsToRoot(parentNodeId, limit, count, visited);
-        if (limit && count >= limit) {
-          visited.delete(nodeId);
-          return limit;
-        }
+      if (visited.has(parentNodeId)) {
+        continue;
+      }
+      count = this.countNodePathsToRootBacktracking(
+        parentNodeId,
+        limit,
+        count,
+        visited,
+      );
+      if (limit && count >= limit) {
+        visited.delete(nodeId);
+        return limit;
       }
     }
     visited.delete(nodeId);

src/graphlib/graph.ts

@@ -137,7 +137,7 @@ export class Graph {
   }
 
   nodeCount(): number {
-    return this.nodes.length;
+    return this.nodes().length;
   }
 
   nodes(): string[] {
@@ -378,7 +378,7 @@ export class Graph {
   }
 
   edgeCount(): number {
-    return this.edges.length;
+    return this.edges().length;
   }
 
   edges(): Edge[] {

test/core/count-paths-to-root.test.ts

@@ -1,6 +1,35 @@
+import { performance } from 'perf_hooks';
 import * as depGraphLib from '../../src';
 import * as helpers from '../helpers';
 
+function buildDiamondChain(layers: number): depGraphLib.DepGraph {
+  const builder = new depGraphLib.DepGraphBuilder(
+    { name: 'npm' },
+    { name: 'root', version: '1.0.0' },
+  );
+  const rootNodeId = 'root-node';
+
+  let prevNodeId = rootNodeId;
+  for (let i = 0; i < layers; i++) {
+    const leftId = `left-${i}`;
+    const rightId = `right-${i}`;
+    const joinId = `join-${i}`;
+
+    builder.addPkgNode({ name: leftId, version: '1.0.0' }, leftId);
+    builder.addPkgNode({ name: rightId, version: '1.0.0' }, rightId);
+    builder.addPkgNode({ name: joinId, version: '1.0.0' }, joinId);
+
+    builder.connectDep(prevNodeId, leftId);
+    builder.connectDep(prevNodeId, rightId);
+    builder.connectDep(leftId, joinId);
+    builder.connectDep(rightId, joinId);
+
+    prevNodeId = joinId;
+  }
+
+  return builder.build();
+}
+
 describe('countPathsToRoot', () => {
   describe('basic', () => {
     const depGraphData = helpers.loadFixture('plain-dep-graph.json');
@@ -122,4 +151,49 @@ describe('countPathsToRoot', () => {
       );
     });
   });
+
+  describe('diamond chain (exponential paths)', () => {
+    it('computes correct count for a small diamond chain', () => {
+      const graph = buildDiamondChain(5);
+      const leafPkg = { name: 'join-4', version: '1.0.0' };
+      expect(graph.countPathsToRoot(leafPkg)).toBe(Math.pow(2, 5));
+    });
+
+    it('handles 50-layer diamond chain (2^50 paths) efficiently', () => {
+      const layers = 50;
+      const graph = buildDiamondChain(layers);
+      const leafPkg = { name: `join-${layers - 1}`, version: '1.0.0' };
+
+      const start = performance.now();
+      const count = graph.countPathsToRoot(leafPkg);
+      const elapsed = performance.now() - start;
+
+      expect(count).toBe(Math.pow(2, layers));
+      expect(elapsed).toBeLessThan(5000);
+    });
+
+    it('respects limit on exponential-path graph', () => {
+      const graph = buildDiamondChain(50);
+      const leafPkg = { name: 'join-49', version: '1.0.0' };
+      expect(graph.countPathsToRoot(leafPkg, { limit: 100 })).toBe(100);
+    });
+
+    it('counts scale linearly with layers, not exponentially', () => {
+      const small = buildDiamondChain(100);
+      const large = buildDiamondChain(400);
+
+      const startSmall = performance.now();
+      small.countPathsToRoot({ name: 'join-99', version: '1.0.0' });
+      const timeSmall = performance.now() - startSmall || 1;
+
+      const startLarge = performance.now();
+      large.countPathsToRoot({ name: 'join-399', version: '1.0.0' });
+      const timeLarge = performance.now() - startLarge || 1;
+
+      // With DP, 4x the layers should take ~4x the time (linear).
+      // With backtracking, it would be 2^300 times slower (never finish).
+      // Use a generous bound to avoid flakiness.
+      expect(timeLarge).toBeLessThan(timeSmall * 50);
+    });
+  });
 });