Eliminate per-node HashSet allocation in PathConflictResolver by gnodet · Pull Request #2075 · apache/maven-resolver

gnodet · 2026-08-28T06:46:44Z

Summary

Replace per-node HashSet<String> copy with parent-chain walk for cycle detection in PathConflictResolver.Path
Each Path previously copied its parent's entire conflictIdsOnPath set (new HashSet<>(parent.conflictIdsOnPath) + .add()), which JFR profiling showed consumed ~45% CPU on a 4,383-module reactor build (HashSet.<init> 16.4%, AbstractCollection.addAll 16.9%, HashMap.put 13.7%)
Since dependency tree depth is bounded in practice (< 30), the O(depth) walk per hasConflictIdOnPathToRoot() call is trivially fast while eliminating all HashSet allocation overhead

Benchmark Results

Tested on the 4,383-module generated reactor project, clean install -DskipTests -q -B, Apple M4 Pro, JDK 21:

Configuration	Wall time	vs RC6
Maven 4.0.0-rc-6 (unpatched)	2:12	baseline
Patched maven-4.0.x (all #12667 PRs, without this fix)	2:25	—
+ this HashSet elimination	1:21	-39%
+ install/deploy plugin fixes	1:14	-44%
Maven 3.9.16	1:20	—

With this fix applied alongside the other optimizations from #12667, Maven 4 is now faster than Maven 3.9.16 on this benchmark.

See: apache/maven#12667

Test plan

Existing PathConflictResolver unit tests pass
Full maven-resolver-util test suite passes
Benchmarked on 4,383-module reactor — correct build output, significant performance improvement

🤖 Generated with Claude Code

Replace the per-node HashSet<String> copy in Path constructor with a parent-chain walk for cycle detection. Each Path previously copied its parent's entire conflictIdsOnPath set (O(depth) per node), which JFR profiling showed consumed ~45% CPU on a 4,383-module reactor build. Since dependency tree depth is bounded in practice (< 30), the O(depth) walk per hasConflictIdOnPathToRoot() call is trivially fast while eliminating all HashSet allocation, HashMap.put, and AbstractCollection.addAll overhead that dominated the profile. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

gnodet

LGTM — Well-justified performance optimization. JFR profiling showed HashSet ops consuming ~45% CPU on a 4,383-module reactor build; replacing per-node HashSet allocation with a parent-chain walk yields a 39% wall-time improvement, bringing Maven 4 below Maven 3.9.16 on this benchmark.

One minor observation below (non-blocking).

📋 PR Metadata

Aspect	Current	Suggested
Category	(unlabeled)	`performance`
Labels	(none)	+ `enhancement`
Milestone	(none)	`3.0.0`

🔀 Backport Status: Not needed — PathConflictResolver.java does not exist on maintenance branches (maven-resolver-1.9.x, maven-resolver-1.6.x).

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

gnodet · 2026-08-28T09:20:15Z

+         * Walks the parent chain comparing conflict IDs. Since dependency tree depth is bounded
+         * in practice (&lt; 30), each check is fast while avoiding per-node HashSet allocation
+         * that was a major JFR hotspot (~45% CPU) in large multi-module builds.
         */


Minor (non-blocking): targetConflictId.equals(current.conflictId) would NPE if targetConflictId is null. The pre-HashSet version (before commit 51f1af6) used Objects.equals(current.conflictId, targetConflictId) which was null-safe.

In practice, null conflict IDs would indicate a separate upstream bug (ConflictMarker assigns IDs to all nodes with dependencies), and the codebase already uses the same non-null-safe pattern elsewhere (line 967), so this is consistent. Just noting it for completeness.

Suggested change

*/

for (Path current = this; current != null; current = current.parent) {

if (Objects.equals(targetConflictId, current.conflictId)) {

return true;

}

github-actions · 2026-08-30T13:20:07Z

@cstamas Please assign appropriate label to PR according to the type of change.

In highly connected graphs (e.g. a 813-module reactor where each module depends on ~9 others), PathConflictResolver created an exponential number of Path objects: each DependencyNode reachable via N different parent paths was expanded N times, causing its entire subtree to be traversed N times. This resulted in OutOfMemoryError in gatherCRNodes/addChildren. Fix: track which DependencyNode instances have already been expanded in an IdentityHashMap<DependencyNode, Integer> (mapping to expansion depth). When the same DependencyNode is encountered again via a different parent path, a Path entry is still created (so all occurrences appear in the conflict partition for winner selection), but its subtree is not re- traversed. If the same node is later reached at a shallower depth, it is re-expanded at that depth, updating the recorded minimum. Also eliminates the per-Path HashSet<String> copy used for O(1) cycle ## Problem `PathConflictResolver` runs out of memory (`OutOfMemoryError: Java heap space`) on highly connected dependency graphs, such as a 813-module reactor where each module depends on ~9 others. Reproduced with `-Xmx512m`: ``` java.lang.OutOfMemoryError: Java heap space at PathConflictResolver$Path.addChildren(PathConflictResolver.java:699) at PathConflictResolver$State.gatherCRNodes(PathConflictResolver.java:363) ``` Workaround: `-Daether.conflictResolver.impl=classic` ## Root Cause `gatherCRNodes` performs a BFS/DFS traversal that creates a new `Path` object for **every edge** in the expanded dependency tree — not just one per `DependencyNode`. When the same node is reachable via N different parent paths, its entire subtree is traversed N times, leading to exponential `Path` allocation. For 813 modules × ~9 avg deps, this produces millions of `Path` objects instead of the ~7,500 that actually represent unique graph edges. ## Fix Two complementary changes: **1. Expansion deduplication** (the OOM fix): Track which `DependencyNode` instances have already been expanded in an `IdentityHashMap<DependencyNode, Integer>` (mapped to expansion depth). When the same `DependencyNode` is encountered again via a different parent path, a `Path` entry is still created for it (so all occurrences appear in the conflict partition for winner selection), but its subtree is **not re-traversed**. If the same node is later reached at a shallower depth, it is re-expanded and the recorded minimum is updated. **2. Remove per-Path `HashSet` copy** (CPU/memory fix): The `HashSet<String> conflictIdsOnPath` that was copied on every `Path` construction is dropped. Since expansion deduplication already bounds total path count to O(graph edges), the O(depth) parent-chain walk for `hasConflictIdOnPathToRoot` is sufficient and cheaper. This also eliminates the per-node allocation that was identified as a JFR hotspot in the prior PR #2075. ## Testing - All 463 `maven-resolver-util` tests pass - Regression test `denseGraphDoesNotOom` added: 3-level shared graph (100×100×100), would create ~1M `Path` objects and OOM without the fix, completes in <100ms with it - Both `path` and `classic` resolvers produce identical results on the reproducer ## Performance — 813-module reproducer (813 modules × 9 deps = 7,317 edges) Benchmarked on the exact reproducer topology (circular dependency graph, all nodes shared): **Minimum heap to complete without OOM:** | Resolver | Min heap | |---|---| | `PathConflictResolver` (fixed) | **28 MB** | | `ClassicConflictResolver` | **48 MB** | **First-run latency (cold JIT, as in a real build):** | Resolver | Time | |---|---| | `PathConflictResolver` (fixed) | **~260 ms** | | `ClassicConflictResolver` | **~5,400 ms** | **Warmed latency (JIT steady state):** | Resolver | Time | |---|---| | `PathConflictResolver` (fixed) | **~2–3 ms** | | `ClassicConflictResolver` | **~50–90 ms** | The fixed `PathConflictResolver` is **×21 faster cold** and **×30 faster warmed** than `ClassicConflictResolver`, while also requiring 40% less heap. The classic resolver's 5 s cold time is dominated by GC pressure when the heap is near its minimum: it allocates heavily during traversal and spends most of the first run collecting garbage. _Hermes Agent (Claude Sonnet 4.6) on behalf of Guillaume Nodet_ Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

gnodet commented Aug 28, 2026

View reviewed changes

gnodet mentioned this pull request Aug 28, 2026

Performance: Maven 4 at parity with Maven 3 on large reactors (2:45 → 1:18) apache/maven#12667

Closed

elharo approved these changes Aug 29, 2026

View reviewed changes

cstamas approved these changes Aug 30, 2026

View reviewed changes

cstamas merged commit 7436f8f into master Aug 30, 2026
44 of 45 checks passed

cstamas deleted the perf/path-conflict-resolver-hashset-elimination branch August 30, 2026 13:20

github-actions Bot assigned cstamas Aug 30, 2026

github-actions Bot added this to the 2.0.23 milestone Aug 30, 2026

cstamas added the enhancement New feature or request label Aug 30, 2026

gnodet mentioned this pull request Sep 22, 2026

Fix PathConflictResolver OOM on dense dependency graphs #2153

Merged

Navigation Menu

Eliminate per-node HashSet allocation in PathConflictResolver - #2075

Eliminate per-node HashSet allocation in PathConflictResolver#2075
cstamas merged 1 commit into
masterfrom
perf/path-conflict-resolver-hashset-elimination

gnodet commented Aug 28, 2026

Uh oh!

gnodet left a comment

Uh oh!

gnodet Aug 28, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-         */
+            for (Path current = this; current != null; current = current.parent) {
+                if (Objects.equals(targetConflictId, current.conflictId)) {
+                    return true;
+                }

Conversation

gnodet commented Aug 28, 2026

Summary

Benchmark Results

Test plan

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

📋 PR Metadata

Uh oh!

gnodet Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants