ConflictResolver improvements#1558
Merged
cstamas merged 24 commits intoapache:masterfrom Aug 28, 2025
Merged
Conversation
Member
Author
|
Current verbosity difference can be seen using toolbox (verbosity=standard): |
Member
Author
|
Verbose mode fixed as well, undone UT changes related to Verbose mode. |
Member
Author
|
The rewrite should not replace the old one, but coexist with it, so caller (ie Maven) should be able to choose. |
cstamas
commented
Aug 28, 2025
...er-util/src/main/java/org/eclipse/aether/util/graph/transformer/ClassicConflictResolver.java
Outdated
Show resolved
Hide resolved
cstamas
commented
Aug 28, 2025
...-resolver-util/src/main/java/org/eclipse/aether/util/graph/transformer/ConflictResolver.java
Outdated
Show resolved
Hide resolved
cstamas
commented
Aug 28, 2025
...olver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
Outdated
Show resolved
Hide resolved
cstamas
commented
Aug 28, 2025
...olver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
Outdated
Show resolved
Hide resolved
Member
Author
|
This part is common (literally is same) for both conflict resolver implementation:
|
cstamas
commented
Aug 28, 2025
...olver-util/src/main/java/org/eclipse/aether/util/graph/transformer/PathConflictResolver.java
Outdated
Show resolved
Hide resolved
Remove redundancies, document verbosity only on one single place as it is conflict resolver contract, all implementations should behave the same.
gnodet
approved these changes
Aug 28, 2025
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.
The ConflictResolver class was last bit that was identical between Resolver 1.x and 2.x and it had a nasty property: in "worst case" it has performance of O(N^2) where N=partition count, that in some projects (see projects generated with https://github.com/maven-turbo-reactor/maven-multiproject-generator) is same as module count and can be huge.
Moreover, Maven 4 does "one extra" collection on
installto build consumer POM. This impacted Maven performance a lot, see apache/maven#2481This PR introduces following changes related to ConflictResolver:
ClassicConflictResolverandPathConflictResolver.There is one UT added that comes from BF/DF resolver UTs too.
The "path" conflict resolver idea:
Input is "dirty rooted graph" (as it may contain cycles) coming from Collector. Moreover, due memory savings, same instance of
DependencyNodemay be present on different paths in the graph (even without cycles involved). Finally, theDependencyNode.childrenlist may be shared across unrelated instances (again, memory saving). I did not want to touch collectors at all, but I "lived" with these things.Approach was to introduce
Pathtype, that "builds a parallel" graph to input graph but only to up to cycle beginnings (verbosity decides what happens with node, is removed, is left with children/cycle removed, etc). Hence,Pathgraph from root is always a tree. Then, the parent-child "inheritance" is applied (scope + optionality) and alsoPathrecords conflictId it belongs to (partition). Basically, each instance ofPathrepresents a path to givenPath'sDependencyNode, as if you'd walkPath.parentuntilparent == null, you'd get the path. MultiplePathinstances may point to sameDependencyNode(see input).Then, based on topologically sorted conflictIds, I am selecting "winner", and applying changes to related
Paths, recursively applying to children (but only one level!) or unlinking them (eliminating them fully from graph).Finally, changes are "pushed" to parallel existing
DependencyNodegraph applying required changes (scope, optional, version, unlinking).At the end, DN graph is: