Skip to content

Store DependencyInsight's dependency graph as a DAG to prevent OOM on ubiquitous transitive deps#8193

Merged
jkschneider merged 1 commit into
mainfrom
dependency-insight-dag-dedup
Jul 8, 2026
Merged

Store DependencyInsight's dependency graph as a DAG to prevent OOM on ubiquitous transitive deps#8193
jkschneider merged 1 commit into
mainfrom
dependency-insight-dag-dedup

Conversation

@jkschneider

@jkschneider jkschneider commented Jul 8, 2026

Copy link
Copy Markdown
Member

What's the problem?

DependencyInsight (Gradle/Maven) can OOM on a single repository when the target is a ubiquitous, deeply-transitive dependency such as com.fasterxml.jackson*. DependencyTreeWalker.walkRecursive enumerates every root→match path — it prunes cycles within the current path but has no DAG-level dedup — so a diamond-heavy graph expands combinatorially, and the per-match callback appends each path into a non-deduplicated DependencyGraph. One repo materialized ~37M DependencyGraph$DependencyNode and blew a ~16 GB heap.

The fix

Give the walk a single visited-set spanning the whole traversal (not just the current path). It both prevents cycles and dedups subtrees reachable through multiple paths, so a match reachable via k diamonds fires the callback once with a representative path instead of k times. Traversal goes from O(paths) (exponential) to O(nodes) (linear).

This also makes good on the walker's existing Javadoc — "Uses caching to avoid re-traversing duplicate dependencies" — which the implementation never actually honored.

The visited-set is per direct-dependency root (each walk(root, …) gets its own), so attribution is preserved: the SearchResult markers on the POM/build.gradle are unchanged (DFS reachability is identical). Only the DependenciesInUse / ExplainDependenciesInUse data tables change — count becomes the number of direct dependencies a match is pulled in through, and the dependency tree shows one representative path per root instead of the combinatorial expansion.

Benchmark

Reproducing the exact OOM structure — one matched dependency reachable from the root via 2^depth diamond paths — and running the pre-fix algorithm against the post-fix one on the identical graph:

Total traversal work — O(2ᵈ)O(d):

depth graph nodes OLD node-visits NEW node-visits
10 31 4,093 31
18 55 1,048,573 55
22 67 16,777,213 67
24 73 67,108,861 73

DependencyGraph built the way DependencyInsight does (depth 18 = 262,144 paths to one match):

OLD: size=262,144 appends, time=676 ms, retained ≈ 224 MB
NEW: size=1 append,        time=0 ms,   retained ≈ 0 MB

Extrapolated to the issue's ~depth-25 scale (~33M paths): OLD ≈ 28 GB, NEW stays flat — matching the reported 37M nodes / 16 GB heap in order of magnitude. For non-pathological graphs the walk visits each node once, so there's no slowdown.

Tests

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Jul 8, 2026
@jkschneider jkschneider marked this pull request as draft July 8, 2026 00:59
@jkschneider jkschneider force-pushed the dependency-insight-dag-dedup branch from fcf75dd to 6300851 Compare July 8, 2026 01:18
@jkschneider jkschneider changed the title Dedup shared subtrees in DependencyTreeWalker to prevent DependencyInsight OOM Store DependencyInsight's dependency graph as a DAG to prevent OOM on ubiquitous transitive deps Jul 8, 2026
@jkschneider jkschneider force-pushed the dependency-insight-dag-dedup branch from 6300851 to addaba7 Compare July 8, 2026 01:42
Comment thread rewrite-maven/src/main/java/org/openrewrite/maven/graph/DependencyGraph.java Outdated
… ubiquitous transitive deps

`DependencyInsight` could OOM on a single repository when the target is a
ubiquitous, deeply-transitive dependency such as `com.fasterxml.jackson*`.
`DependencyTreeWalker.walkRecursive` enumerated every root->match path (cycles
were pruned only within the current path), and the per-match `DependencyGraph`
stored each path as its own tree branch. On a diamond-heavy graph both grow
combinatorially; one repo materialized ~37M `DependencyGraph$DependencyNode`
and blew a 16 GB heap.

Store the graph as a coordinate-interned DAG instead of a per-path tree: a
dependency reachable through many paths (a diamond) is stored once.
`DependencyGraph.build` constructs it in O(nodes+edges), and `getSize()` reports
the true distinct-path count via dynamic programming over the DAG rather than by
enumerating exponentially many paths. `DependencyTreeWalker` also gets a global
visited-set so the `Matches` computation behind the `SearchResult` markers is
O(nodes) as well.

Observable behavior is unchanged: the markers, the `ExplainDependenciesInUse`
dependency tree (`print()` already collapses shared nodes with `(*)`), and the
`DependenciesInUse` count are all identical to before -- the existing tests pass
untouched. Only the internal representation and its complexity change.

Fixes #8181
@jkschneider jkschneider force-pushed the dependency-insight-dag-dedup branch from addaba7 to 99513ad Compare July 8, 2026 01:46
@jkschneider jkschneider marked this pull request as ready for review July 8, 2026 01:56
@jkschneider jkschneider merged commit 6d773b6 into main Jul 8, 2026
1 check passed
@jkschneider jkschneider deleted the dependency-insight-dag-dedup branch July 8, 2026 01:56
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Dependency Insight OOMs on ubiquitous transitive deps (jackson): DependencyTreeWalker enumerates all root→match paths with no DAG dedup

1 participant