Skip to content

ConflictResolver improvements#1558

Merged
cstamas merged 24 commits intoapache:masterfrom
cstamas:cr-alt
Aug 28, 2025
Merged

ConflictResolver improvements#1558
cstamas merged 24 commits intoapache:masterfrom
cstamas:cr-alt

Conversation

@cstamas
Copy link
Member

@cstamas cstamas commented Aug 21, 2025

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 install to build consumer POM. This impacted Maven performance a lot, see apache/maven#2481

This PR introduces following changes related to ConflictResolver:

  • changes to make possible for more ConflictResolver implementations to coexist
  • The old one is renamed to "classic" conflict resolver
  • implemented "path" conflict resolver
  • UTs run against both
  • this is a breaking change for integrators (mostly Maven only, as others use suppliers and are protected!), but luckily conflict resolver was never part of public API aside of instantiating one... now client code must choose between ClassicConflictResolver and PathConflictResolver.

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 DependencyNode may be present on different paths in the graph (even without cycles involved). Finally, the DependencyNode.children list 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 Path type, 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, Path graph from root is always a tree. Then, the parent-child "inheritance" is applied (scope + optionality) and also Path records conflictId it belongs to (partition). Basically, each instance of Path represents a path to given Path's DependencyNode, as if you'd walk Path.parent until parent == null, you'd get the path. Multiple Path instances may point to same DependencyNode (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 DependencyNode graph applying required changes (scope, optional, version, unlinking).

At the end, DN graph is:

  • verbosity=none, a tree pruned from all of redundant/loser nodes
  • verbosity=standard a tree with redundant/loser nodes left in place containing recorded information and children/cycle trimmed
  • verbosirt=full a graph (if it had cycles, it still have them), but the tree subset of it is "decorated" with information as in case of standard verbosity.

@cstamas cstamas self-assigned this Aug 21, 2025
@cstamas cstamas linked an issue Aug 21, 2025 that may be closed by this pull request
@cstamas cstamas added this to the 2.0.11 milestone Aug 21, 2025
@cstamas cstamas added the enhancement New feature or request label Aug 21, 2025
@cstamas
Copy link
Member Author

cstamas commented Aug 21, 2025

Current verbosity difference can be seen using toolbox (verbosity=standard):

@cstamas
Copy link
Member Author

cstamas commented Aug 22, 2025

Verbose mode fixed as well, undone UT changes related to Verbose mode.

@cstamas
Copy link
Member Author

cstamas commented Aug 22, 2025

The rewrite should not replace the old one, but coexist with it, so caller (ie Maven) should be able to choose.

@cstamas cstamas changed the title Rewritten ConflictResolver ConflictResolver improvements Aug 22, 2025
@cstamas cstamas marked this pull request as ready for review August 22, 2025 16:58
@cstamas
Copy link
Member Author

cstamas commented Aug 28, 2025

This part is common (literally is same) for both conflict resolver implementation:

  • checks context for presence of (topo) sorted conflict IDs, if not present invoke ConflictIdSorter
    • ConflictIdSorter checks context for presence of conflict IDs, if not present invoke ConflictMarker
      • ConflictMarker partitions the DependencyNode dirty graph by GACE (partitions graph by assigning same "conflict ID" to nodes having same GACE)
      • ConflictMarker stores result of mapping (DependencyNode to conflictId) to context
    • ConflictIdSorter then topo sorts the conflict IDs, and looks then for cycles (both are stored in context)
  • finally, conflict resolver makes use of conflict mapping, topo sorted conflict IDs and cycles

Remove redundancies, document verbosity only on one single place
as it is conflict resolver contract, all implementations should
behave the same.
@cstamas cstamas merged commit ffa2177 into apache:master Aug 28, 2025
8 checks passed
@cstamas cstamas deleted the cr-alt branch August 28, 2025 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conflict Resolver improvements

2 participants