Copilot commented on code in PR #1938:
URL: https://github.com/apache/maven-resolver/pull/1938#discussion_r3487910914
##########
maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java:
##########
@@ -258,22 +264,58 @@ public ConflictResolver(
}
@Override
+ @SuppressWarnings("unchecked")
public DependencyNode transformGraph(DependencyNode node,
DependencyGraphTransformationContext context)
throws RepositoryException {
String cf = ConfigUtils.getString(
context.getSession(), DEFAULT_CONFLICT_RESOLVER_IMPL,
CONFIG_PROP_CONFLICT_RESOLVER_IMPL);
ConflictResolver delegate;
- if (PATH_CONFLICT_RESOLVER.equals(cf)) {
+ if (AUTO_CONFLICT_RESOLVER.equals(cf)) {
+ delegate = selectConflictResolver(node, context);
+ } else if (PATH_CONFLICT_RESOLVER.equals(cf)) {
delegate = new PathConflictResolver(versionSelector,
scopeSelector, optionalitySelector, scopeDeriver);
} else if (CLASSIC_CONFLICT_RESOLVER.equals(cf)) {
delegate = new ClassicConflictResolver(versionSelector,
scopeSelector, optionalitySelector, scopeDeriver);
} else {
throw new IllegalArgumentException("Unknown conflict resolver: " +
cf + "; known are "
- + Arrays.asList(PATH_CONFLICT_RESOLVER,
CLASSIC_CONFLICT_RESOLVER));
+ + Arrays.asList(AUTO_CONFLICT_RESOLVER,
PATH_CONFLICT_RESOLVER, CLASSIC_CONFLICT_RESOLVER));
}
return delegate.transformGraph(node, context);
}
+ /**
+ * 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.
+ * In such cases, ClassicConflictResolver is used instead — it works
in-place with no parallel
+ * structure, trading O(N²) worst-case time for O(1) extra space.
+ */
+ @SuppressWarnings("unchecked")
+ private ConflictResolver selectConflictResolver(DependencyNode node,
DependencyGraphTransformationContext context)
+ throws RepositoryException {
+ // Ensure conflict IDs are computed — both implementations need this
anyway
+ if (context.get(TransformationContextKeys.SORTED_CONFLICT_IDS) ==
null) {
+ new ConflictIdSorter().transformGraph(node, context);
+ }
+
+ 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;
Review Comment:
The auto-selection heuristic uses `conflictIds.size()` as `nodeCount`, but
`conflictIds` is built from an `IdentityHashMap` of *unique* `DependencyNode`
instances (see `ConflictMarker`), while `PathConflictResolver` allocates a
`Path` per *occurrence/path* (a node instance can appear under multiple
parents, e.g. shared subgraphs/cycles). This can make `pathTreeEstimate` a
lower bound and may still select `PathConflictResolver` in cases where the
actual Path tree is much larger, undermining the OOM-avoidance goal. Consider
either using a more conservative safety factor or improving the estimate to
account for node re-use/graph sharing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
