Reduce PathConflictResolver memory and auto-select resolver - #1938
Merged
Conversation
|
The build gets stuck. Some stack traces: While all bfcollector threads are sleeping: Worth to mention:
|
|
After rerunning the build I have OOM problems. The implementation consumes memory quite actively (for 3.9.16 it's enough to have only 9g): 
At some points the process gets stuck, the stack trace is: Regarding the stuck state - I've already seen this, it happens once in a while. But this actually means there is a wrong multi-thread implementation somewhere and at scale these problems will happen quite often. |
Four improvements to prevent OOM on large dependency graphs: 1. Auto-selection heuristic (new default): estimates Path tree memory cost (~200 bytes × nodeCount) from the conflict-ID map and falls back to ClassicConflictResolver if the parallel tree would exceed 25% of available heap. 2. Lazy right-sized children lists: Path.children starts as null (not an empty ArrayList) and is initialized to exactly the right capacity in addChildren(). Leaves (~60-70% of nodes) never allocate a list, saving ~40 bytes per leaf. 3. outOfScope flag replaces LinkedHashSet partition entries: moveOutOfScope() sets a boolean instead of removing from a LinkedHashSet, eliminating the HashMap.Node overhead (~48 bytes per Path). Active paths are filtered at query time with a simple boolean check. 4. Iterative gatherCRNodes: converts the recursive tree-building method to use an explicit stack, avoiding StackOverflowError on very deep dependency graphs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
force-pushed
the
gnodet/conflict-resolver-memory-improvements
branch
from
June 28, 2026 10:50
7b3ae16 to
32c90a7
Compare
|
Few comments:
|
…opeContext, right-sized collections - Replace ArtifactIdUtils.toId()/toVersionlessId() + String.equals() with allocation-free equalsId()/equalsVersionlessId() in push() and isDirectDependencyOnPathToRoot() - Eliminate string concatenation in relatedSiblingsCount() by comparing groupId/artifactId fields directly - Pre-size activePaths and items lists with allPaths.size() - Right-size partitions and resolvedIds HashMaps using conflict ID count - Pool ScopeContext on State with reset() method to avoid per-node allocation - Remove dead recursive push() code (always called with levels=0) - Optimize isDirectDependencyOnPathToRoot to walk directly to depth-1 ancestor instead of recursing through every level Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request improves dependency conflict resolution scalability by reducing PathConflictResolver’s memory footprint and recursion risks, and introduces a new default "auto" mode in ConflictResolver that selects between path-based and classic resolution based on estimated heap usage.
Changes:
- Add
"auto"(now default) conflict resolver selection that choosesPathConflictResolveronly when the estimated Path tree fits in available heap; otherwise falls back toClassicConflictResolver. - Reduce
PathConflictResolverallocations (lazy children lists, allocation-free artifact comparisons, right-sized collections, pooled scope context) and replace recursive graph build with an explicit stack. - Replace partition removal with an
outOfScopeflag and filter paths at query time.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java | Memory/allocation optimizations, iterative graph walk, and new out-of-scope handling for conflict partitions. |
| maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java | Introduces default "auto" resolver selection heuristic and wiring for the new resolver mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+175
to
+185
| // paths in given conflict group to consider; filter out those moved out of scope | ||
| List<Path> allPaths = state.partitions.get(conflictId); | ||
| List<Path> activePaths = new ArrayList<>(allPaths.size()); | ||
| List<ConflictItem> items = new ArrayList<>(allPaths.size()); | ||
| for (Path p : allPaths) { | ||
| if (!p.outOfScope) { | ||
| activePaths.add(p); | ||
| items.add(new ConflictItem(p)); | ||
| } | ||
| } | ||
| if (activePaths.isEmpty()) { |
Contributor
Author
There was a problem hiding this comment.
Fixed in 0b4ca58. Partition entries are now replaced with the filtered activePaths list after each conflict group, releasing references to out-of-scope paths and their detached subtrees for GC during resolution. Additionally, moveOutOfScope() now nulls children references on marked nodes to break reference chains, and conflictItemCount is tracked inline during the main loop instead of re-scanning partitions post-resolution.
Comment on lines
+667
to
+671
| private void moveOutOfScope() { | ||
| this.state.partitions.get(this.conflictId).remove(this); | ||
| for (Path child : this.children) { | ||
| child.moveOutOfScope(); | ||
| this.outOfScope = true; | ||
| if (this.children != null) { | ||
| for (Path child : this.children) { | ||
| child.moveOutOfScope(); |
Contributor
Author
There was a problem hiding this comment.
Fixed in 0b4ca58. Converted moveOutOfScope() to use an explicit ArrayList stack instead of recursion, consistent with the iterative gatherCRNodes() approach. The iterative version also nulls children references on out-of-scope nodes to accelerate GC of detached subtrees.
Comment on lines
+286
to
+290
| /** | ||
| * Selects the most appropriate conflict resolver based on graph size and available memory. | ||
| * <p> | ||
| * PathConflictResolver builds a parallel tree of Path objects that costs ~200 bytes per node. | ||
| * For very large dependency graphs (millions of nodes), this can exhaust the heap. |
Contributor
Author
There was a problem hiding this comment.
Fixed in 0b4ca58. Added 5 unit tests covering the delegating ConflictResolver dispatch: explicit "auto" config, explicit "path" config, explicit "classic" config, unknown config rejection (IllegalArgumentException), and default config (no property set, exercises the auto-selection code path). All 443 tests pass.
… auto-selection tests - Compact partition entries after filtering to release references to out-of-scope paths and their detached subtrees, allowing GC during resolution instead of retaining losers until the end - Make moveOutOfScope() iterative (explicit stack) to avoid StackOverflowError on deep dependency chains, consistent with gatherCRNodes(); also null children references on out-of-scope nodes to accelerate GC of detached subtrees - Track conflictItemCount inline during the main loop instead of re-scanning partitions in the stats block - Add 5 unit tests covering the auto-selection heuristic and delegating ConflictResolver: auto mode, explicit path/classic config, unknown config rejection, and default config behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java:236
- Losers are marked
outOfScopeduringpush(), but the partition list for the currentconflictIdwas populated before that and is never compacted again (the conflictId is processed only once). This keeps strong references to newly-losingPathinstances until the end of the transform, delaying GC of pruned losers. Consider compacting the partition after the group is resolved (e.g., keep onlywinnerPath) to drop loser references promptly.
// derive with new values from this to children only; observe winner flag
path.derive(1, path == winnerPath);
// push this node full level changes to DN graph
path.push(0);
}
Comment on lines
+302
to
+307
| Map<DependencyNode, String> conflictIds = | ||
| (Map<DependencyNode, String>) context.get(TransformationContextKeys.CONFLICT_IDS); | ||
| int nodeCount = conflictIds != null ? conflictIds.size() : 0; | ||
|
|
||
| // Estimate Path tree memory: ~200 bytes per Path (object + right-sized children list + partition entry) | ||
| long pathTreeEstimate = (long) nodeCount * 200; |
cstamas
approved these changes
Jun 28, 2026
Member
|
@seregamorph have you tried latest changes? |
|
Checked 0b4ca58 |
gnodet
force-pushed
the
gnodet/conflict-resolver-memory-improvements
branch
from
June 28, 2026 20:18
798a43d to
663438b
Compare
…ue nodes The previous auto-selection heuristic used conflictIds.size() (unique DependencyNode count) to estimate Path tree memory. For large projects with diamond dependencies, this vastly underestimates the actual tree size because each shared subtree is duplicated in the Path tree. For example, a single module in an 814-module project showed: - Unique nodes (conflictIds): 7,522 - Actual tree nodes (diamond-expanded): 1,190,821 (158x larger) The old heuristic estimated ~1MB for this module's Path tree; the actual size was ~227MB. This caused the auto-selector to always choose PathConflictResolver, leading to OOM even at 4GB heap. The fix walks the dependency tree counting total nodes (including diamond-expanded duplicates) with an early-exit optimization: counting stops as soon as the threshold is exceeded, avoiding the cost of measuring multi-million-node trees that clearly won't fit. With this fix, auto-selection correctly falls back to ClassicConflictResolver for modules with large diamond-expanded trees, allowing 814-module projects to resolve at 512MB heap (vs OOM at 4GB before). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
force-pushed
the
gnodet/conflict-resolver-memory-improvements
branch
from
June 28, 2026 20:20
663438b to
6072fc5
Compare
gnodet
marked this pull request as ready for review
June 28, 2026 21:35
Contributor
Author
|
Claude Code on behalf of Guillaume Nodet
Edit: The assertions in this comment were incorrect. The benchmark script had a bug (missing tar extraction of the Maven distribution), which caused the 3.10.0-SNAPSHOT runs to use a stale binary. Additionally, a debugging patch (removing early-exit from the tree walk heuristic) caused spurious OOMs that were mistakenly attributed to the threshold being too conservative. See the corrected benchmark results for accurate numbers. The auto heuristic correctly selects |
Member
|
So, then basically gigantic projects are doomed to use classic (if they insist on same memory amount; no speedup)? |
Contributor
Author
|
Claude Code on behalf of Guillaume Nodet Ran an independent benchmark with the reproducer project (814-module ResultsMaven 3.9.16 (baseline, classic resolver) — stable across all heap sizes (~96–99s):
3.10.0-SNAPSHOT (patched, auto mode selects path resolver):
Observations
|
|
LGTM |
Member
|
Maybe add this one too https://gist.github.com/cstamas/99f760aa13d738f02712accfd4a49c29 |
cstamas
approved these changes
Jun 29, 2026
|
@cstamas Please assign appropriate label to PR according to the type of change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improvements to
PathConflictResolverto prevent OOM and StackOverflow on large dependency graphs, plus allocation and collection-sizing optimizations:Memory & safety improvements
Auto-selection heuristic (new default
"auto"mode): walks the dependency tree counting total nodes (including diamond-expanded duplicates) with early-exit optimization. Falls back toClassicConflictResolverwhen the estimated Path tree memory exceeds 25% of available heap. This correctly handles the pathological case where diamond dependencies cause the Path tree to be 100-150× larger than the unique node count.Lazy children lists:
Path.childrenstarts asnull(leaf nodes never allocate a list). Initialized inaddChildren()with exact capacity. Saves ~40 bytes per leaf node (typically 60-70% of all nodes).Out-of-scope flag: replaces removal from
LinkedHashSetpartitions with a booleanoutOfScopeflag. Partitions useArrayListinstead ofLinkedHashSet, avoiding ~48 bytes/entry of HashMap.Node overhead.Iterative
gatherCRNodesandmoveOutOfScope: both replaced recursive DFS with explicitArrayListstacks to avoidStackOverflowErroron deep dependency chains.Partition compaction: after filtering active paths for each conflict group, the partition entry is replaced with the filtered list, releasing references to out-of-scope paths and their detached subtrees for GC during resolution instead of retaining them until the end.
Children nulling on out-of-scope:
moveOutOfScope()nulls children references on marked nodes, accelerating GC of detached subtrees.Allocation & performance optimizations
Allocation-free artifact comparisons: replaced
ArtifactIdUtils.toId()/toVersionlessId()+String.equals()withequalsId()/equalsVersionlessId()inpush()andisDirectDependencyOnPathToRoot().Eliminate string concatenation in
relatedSiblingsCount(): comparegroupId/artifactIdfields directly.Pre-sized
activePaths/itemslists: initialized withallPaths.size()capacity.Right-sized
partitions/resolvedIdsHashMaps: initialized with exact capacity fromsortedConflictIds.size().Pooled
ScopeContext: single mutable instance onState, reset and reused.Optimized
isDirectDependencyOnPathToRoot(): walks directly to the depth-1 ancestor.Removed dead recursive
push()code:push()is always called withlevels=0.Inline stats tracking:
conflictItemCounttracked during the main loop.Benchmark results
Tested with a real-world 814-module project with heavy inter-module dependencies:
pathdefault)pathdefault)pathdefault)pathdefault)pathforced)classicforced)classicforced)autodefault)autodefault)Key findings:
PathConflictResolvercannot resolve this project at any heap size (OOM even at 4GB)conflictIds.size()(unique nodes) — estimated ~1MB when the actual Path tree was ~227MBClassicConflictResolverfor large graphsTest plan