This is an automated email from the ASF dual-hosted git repository. cstamas pushed a commit to branch cstamas-mresolver-228 in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
commit 27e30e0fffe85b937947843a2171906d547e54e4 Author: Tamas Cservenak <[email protected]> AuthorDate: Sat Nov 27 20:12:40 2021 +0100 Proposed changes --- .../impl/collect/ConflictWinnerFinder.java | 42 +++--- .../impl/collect/DefaultDependencyCollector.java | 150 ++++++++++++--------- .../impl/collect/DependencyResolveReconciler.java | 79 +++-------- .../collect/DefaultDependencyCollectorTest.java | 2 +- .../collect/DependencyResolveReconcilerTest.java | 2 +- 5 files changed, 130 insertions(+), 145 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java index 36c269c..f5a0cfe 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ConflictWinnerFinder.java @@ -19,8 +19,6 @@ package org.eclipse.aether.internal.impl.collect; * under the License. */ -import com.google.common.collect.LinkedHashMultimap; -import com.google.common.collect.Multimap; import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.RepositoryException; import org.eclipse.aether.RepositorySystemSession; @@ -36,7 +34,9 @@ import org.eclipse.aether.util.graph.visitor.CloningDependencyVisitor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.Objects; @@ -49,26 +49,19 @@ import static org.eclipse.aether.internal.impl.collect.DependencyResolveReconcil public class ConflictWinnerFinder { - private boolean verbose; - private static final Logger LOGGER = LoggerFactory.getLogger( ConflictWinnerFinder.class ); - public ConflictWinnerFinder( boolean verbose ) - { - this.verbose = verbose; - } - /** * Find artifacts that conflicts with the winner. Only artifacts resolved before winner are considered as * conflicts. * * @return */ - public Collection<Conflict> getVersionConflicts( RepositorySystemSession session, CollectResult res ) + Collection<Conflict> getVersionConflicts( RepositorySystemSession session, CollectResult result ) throws DependencyCollectionException { LinkedHashSet<Conflict> conflicts = new LinkedHashSet<>(); - Multimap<String, Conflict> conflictMap = resolveConflictWinners( session, res ); + HashMap<String, ArrayList<Conflict>> conflictMap = resolveConflictWinners( session, result ); if ( conflictMap == null ) { return conflicts; @@ -77,7 +70,7 @@ public class ConflictWinnerFinder for ( String key : conflictMap.keySet() ) { Collection<Conflict> col = conflictMap.get( key ); - if ( col != null && col.size() > 1 ) + if ( col != null && col.size() > 1 ) // more than ONE { Conflict[] array = col.toArray( new Conflict[0] ); @@ -100,9 +93,9 @@ public class ConflictWinnerFinder { if ( i < index ) { - if ( verbose ) + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Found dependency: {} that conflicts with: {} ", array[i], winner ); + LOGGER.debug( "Found dependency: {} that conflicts with: {} ", array[i], winner ); } conflicts.add( array[i] ); } @@ -117,11 +110,12 @@ public class ConflictWinnerFinder * Use the ConflictResolver to find out all conflict winners based on a cloned dependency graph * * @param session - * @param res + * @param result * @return * @throws DependencyCollectionException */ - private Multimap<String, Conflict> resolveConflictWinners( RepositorySystemSession session, CollectResult res ) + private HashMap<String, ArrayList<Conflict>> resolveConflictWinners( RepositorySystemSession session, + CollectResult result ) throws DependencyCollectionException { long start = System.nanoTime(); @@ -137,7 +131,7 @@ public class ConflictWinnerFinder //clone graph CloningDependencyVisitor vis = new CloningDependencyVisitor(); - DependencyNode root = res.getRoot(); + DependencyNode root = result.getRoot(); root.accept( vis ); root = vis.getRootNode(); @@ -150,20 +144,20 @@ public class ConflictWinnerFinder } catch ( RepositoryException e ) { - res.addException( e ); + result.addException( e ); } - if ( !res.getExceptions().isEmpty() ) + if ( !result.getExceptions().isEmpty() ) { - throw new DependencyCollectionException( res ); + throw new DependencyCollectionException( result ); } DependencyConflictWinnersVisitor conflicts = new DependencyConflictWinnersVisitor(); root.accept( conflicts ); - if ( verbose ) + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); + LOGGER.debug( "Finished to resolve conflict winners in : {} ", ( System.nanoTime() - start ) ); } return conflicts.conflictMap; } @@ -177,13 +171,13 @@ public class ConflictWinnerFinder * The version conflicts of dependencies, conflicts were put to the map following the resolve sequence. ga -> * conflict */ - Multimap<String, Conflict> conflictMap = LinkedHashMultimap.create(); + final HashMap<String, ArrayList<Conflict>> conflictMap = new HashMap<>(); public boolean visitEnter( DependencyNode node ) { Artifact a = node.getArtifact(); Conflict conflict = new Conflict( a ); - conflictMap.put( ga( a ), conflict ); + conflictMap.computeIfAbsent( ga( a ), k -> new ArrayList<>() ).add( conflict ); DependencyNode winner = (DependencyNode) node.getData().get( ConflictResolver.NODE_DATA_WINNER ); if ( winner != null ) { diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index e8d4b0b..d960a4c 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -83,14 +83,17 @@ public class DefaultDependencyCollector /** * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() * configuration properties} used to store a {@link Boolean} flag controlling the resolver's skip & reconcile mode. + * + * @since 1.7.3 */ - public static final String CONFIG_PROP_RESOLVER_MODE = "resolver.mode"; + public static final String CONFIG_PROP_USE_SKIP_RECONCILE = "aether.dependencyCollector.useSkipReconcile"; /** - * The key in the repository session's {@link org.eclipse.aether.RepositorySystemSession#getConfigProperties() - * configuration properties} used to store a {@link Boolean} flag controlling the resolver's verbose mode. + * The default value for {@link #CONFIG_PROP_USE_SKIP_RECONCILE}, {@code true}. + * + * @since 1.7.3 */ - public static final String CONFIG_PROP_RESOLVER_VERBOSE_MODE = "resolver.verbose"; + public static final boolean CONFIG_PROP_USE_SKIP_RECONCILE_DEFAULT = true; private static final String CONFIG_PROP_MAX_EXCEPTIONS = "aether.dependencyCollector.maxExceptions"; @@ -108,17 +111,6 @@ public class DefaultDependencyCollector private VersionRangeResolver versionRangeResolver; - /** - * control the log related with DependencyResolveSkipper - */ - private boolean verbose; - - /** - * control the resolve mode that whether enable skip & reconcile - */ - private boolean skipMode; - - public DefaultDependencyCollector() { // enables default constructor @@ -169,8 +161,13 @@ public class DefaultDependencyCollector requireNonNull( request, "request cannot be null" ); session = optimizeSession( session ); - verbose = ConfigUtils.getBoolean( session, false, CONFIG_PROP_RESOLVER_VERBOSE_MODE ); - skipMode = ConfigUtils.getBoolean( session, true, CONFIG_PROP_RESOLVER_MODE ); + boolean useSkipReconcile = ConfigUtils.getBoolean( + session, CONFIG_PROP_USE_SKIP_RECONCILE_DEFAULT, CONFIG_PROP_USE_SKIP_RECONCILE + ); + if ( useSkipReconcile ) + { + LOGGER.debug( "Collector skip & reconcile enabled." ); + } RequestTrace trace = RequestTrace.newChild( request.getTrace(), request ); @@ -277,7 +274,9 @@ public class DefaultDependencyCollector DefaultVersionFilterContext versionContext = new DefaultVersionFilterContext( session ); - Args args = new Args( session, trace, pool, nodes, new DependencyResolveReconciler( skipMode, verbose ), + DependencyResolveReconciler dependencyResolveReconciler = + useSkipReconcile ? new DependencyResolveReconciler() : null; + Args args = new Args( session, trace, pool, nodes, dependencyResolveReconciler, context, versionContext, request ); Results results = new Results( result, session ); @@ -287,23 +286,26 @@ public class DefaultDependencyCollector depTraverser != null ? depTraverser.deriveChildTraverser( context ) : null, verFilter != null ? verFilter.deriveChildFilter( context ) : null ); - //reconcile the skipped nodes - Collection<DependencyResolveReconciler.DependencyResolveSkip> reconcileNodes = - args.reconciler.getNodesToReconcile( session, result ); - for ( DependencyResolveReconciler.DependencyResolveSkip skip : reconcileNodes ) + if ( args.reconciler != null ) { - Args newArgs = - new Args( session, trace, pool, new NodeStack(), args.reconciler, context, versionContext, - request ); - DataPool.GraphKey key = skip.graphKey; - List<DependencyNode> parents = skip.parentPathsOfCurrentNode; - for ( DependencyNode parent : parents ) + //reconcile the skipped nodes + Collection<DependencyResolveReconciler.DependencyResolveSkip> reconcileNodes = + args.reconciler.getNodesToReconcile( session, result ); + for ( DependencyResolveReconciler.DependencyResolveSkip skip : reconcileNodes ) { - newArgs.nodes.push( parent ); + Args newArgs = + new Args( session, trace, pool, new NodeStack(), args.reconciler, context, versionContext, + request ); + DataPool.GraphKey key = skip.graphKey; + List<DependencyNode> parents = skip.parentPathsOfCurrentNode; + for ( DependencyNode parent : parents ) + { + newArgs.nodes.push( parent ); + } + newArgs.nodes.push( skip.node ); + process( newArgs, results, skip.dependencies, key.repositories, key.selector, key.manager, + key.traverser, key.filter ); } - newArgs.nodes.push( skip.node ); - process( newArgs, results, skip.dependencies, key.repositories, key.selector, key.manager, - key.traverser, key.filter ); } errorPath = results.errorPath; @@ -536,47 +538,70 @@ public class DefaultDependencyCollector VersionFilter childFilter = verFilter != null ? verFilter.deriveChildFilter( context ) : null; final List<RemoteRepository> childRepos = - args.ignoreRepos - ? repositories - : remoteRepositoryManager.aggregateRepositories( args.session, repositories, - descriptorResult.getRepositories(), true ); + args.ignoreRepos + ? repositories + : remoteRepositoryManager.aggregateRepositories( args.session, repositories, + descriptorResult.getRepositories(), true ); - Object key = - args.pool.toKey( d.getArtifact(), childRepos, childSelector, childManager, childTraverser, childFilter ); + Object key = args.pool.toKey( + d.getArtifact(), childRepos, childSelector, childManager, childTraverser, childFilter + ); - List<DependencyNode> parents = args.nodes.getParentNodes(); - int depth = parents.size() + 1; //the depth if pushed the child to stack - List<DependencyNode> cachedChildren = args.pool.getChildren( key ); - DependencyResolveReconciler.CacheResult result = args.reconciler.findCache( child, cachedChildren, depth ); - if ( result != null ) + if ( args.reconciler == null ) { - if ( result.candidateWithSameKey ) + List<DependencyNode> children = args.pool.getChildren( key ); + if ( children == null ) { - child.setChildren( result.dependencyNodes ); + args.pool.putChildren( key, child.getChildren() ); + + args.nodes.push( child ); + + process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, + childTraverser, childFilter ); + + args.nodes.pop(); } - else if ( result.candidateWithLowerDepth ) + else { - //No need to set the children as the result can be ignored (won't be picked up) - args.reconciler.addSkip( child, key, descriptorResult.getDependencies(), parents, - result.parentPathsOfCandidateLowerDepth ); - if ( verbose ) - { - LOGGER.info( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); - } + child.setChildren( children ); } } else { - args.nodes.push( child ); - if ( verbose ) + List<DependencyNode> parents = args.nodes.getParentNodes(); + int depth = parents.size() + 1; //the depth if pushed the child to stack + List<DependencyNode> cachedChildren = args.pool.getChildren( key ); + DependencyResolveReconciler.CacheResult result = args.reconciler.findCache( child, cachedChildren, depth ); + if ( result != null ) { - LOGGER.info( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); + if ( result.candidateWithSameKey ) + { + child.setChildren( result.dependencyNodes ); + } + else if ( result.candidateWithLowerDepth ) + { + //No need to set the children as the result can be ignored (won't be picked up) + args.reconciler.addSkip( child, key, descriptorResult.getDependencies(), parents, + result.parentPathsOfCandidateLowerDepth ); + if ( LOGGER.isDebugEnabled() ) + { + LOGGER.debug( "Skipped resolving artifact {} of depth {}", child.getArtifact(), depth ); + } + } + } + else + { + args.nodes.push( child ); + if ( LOGGER.isDebugEnabled() ) + { + LOGGER.debug( "Resolving artifact {} of depth {}", child.getArtifact(), depth ); + } + process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, + childTraverser, childFilter ); + args.pool.putChildren( key, child.getChildren() ); + args.reconciler.cacheChildrenWithDepth( child, parents ); + args.nodes.pop(); } - process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, - childTraverser, childFilter ); - args.pool.putChildren( key, child.getChildren() ); - args.reconciler.cacheChildrenWithDepth( child, parents ); - args.nodes.pop(); } } @@ -769,6 +794,9 @@ public class DefaultDependencyCollector final CollectRequest request; + /** + * Nullable: is {@code null} if {@link #CONFIG_PROP_USE_SKIP_RECONCILE} evaluates to {@code false}. + */ final DependencyResolveReconciler reconciler; @SuppressWarnings( "checkstyle:parameternumber" ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java index d06e448..35da8e6 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconciler.java @@ -19,8 +19,6 @@ package org.eclipse.aether.internal.impl.collect; * under the License. */ -import com.google.common.collect.LinkedHashMultimap; -import com.google.common.collect.Multimap; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.collection.CollectResult; @@ -38,7 +36,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; @@ -47,37 +44,27 @@ import java.util.Set; */ public class DependencyResolveReconciler { + + private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolveReconciler.class ); + /** * Cache maven dependency resolve result by depth gav -> result */ - private Map<String, DependencyResolveResult> nodesWithDepth = new HashMap<>( 256 ); + private final HashMap<String, DependencyResolveResult> nodesWithDepth; + /** * Track the nodes that have been skipped to resolve for later reconciling */ - private LinkedHashSet<DependencyResolveSkip> skippedNodes = new LinkedHashSet<>(); - - private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolveReconciler.class ); - private boolean skipMode; - private boolean verbose; + private final LinkedHashSet<DependencyResolveSkip> skippedNodes; - public DependencyResolveReconciler( boolean skip, boolean verbose ) + public DependencyResolveReconciler() { - this.skipMode = skip; - this.verbose = verbose; - - if ( this.verbose ) - { - LOGGER.info( "Skip & reconcile mode is {} ", this.skipMode ? "on" : "off" ); - } + this.nodesWithDepth = new HashMap<>( 256 ); + this.skippedNodes = new LinkedHashSet<>(); } public void cacheChildrenWithDepth( DependencyNode node, List<DependencyNode> parents ) { - if ( !this.skipMode ) - { - return; - } - int depth = parents.size() + 1; Artifact artifact = node.getArtifact(); List<DependencyNode> children = node.getChildren(); @@ -87,21 +74,12 @@ public class DependencyResolveReconciler public CacheResult findCache( DependencyNode current, List<DependencyNode> cacheByGraphKey, int depth ) { - CacheResult cache = new CacheResult(); - //non skip mode - if ( !skipMode && cacheByGraphKey != null ) - { - cache.dependencyNodes = cacheByGraphKey; - cache.candidateWithSameKey = true; - return cache; - } - - //skip mode DependencyResolveResult result = nodesWithDepth.get( gav( current.getArtifact() ) ); if ( result != null ) { if ( result.depth <= depth ) { + CacheResult cache = new CacheResult(); cache.dependencyNodes = result.children; cache.candidateWithLowerDepth = true; cache.parentPathsOfCandidateLowerDepth = result.parentPaths; @@ -124,7 +102,7 @@ public class DependencyResolveReconciler public void addSkip( DependencyNode current, Object key, List<Dependency> dependencies, List<DependencyNode> parents, List<DependencyNode> skippedBy ) { - if ( skipMode && dependencies != null && dependencies.size() > 0 ) + if ( dependencies != null && dependencies.size() > 0 ) { DependencyResolveSkip skip = new DependencyResolveSkip( current, (DataPool.GraphKey) key, dependencies, parents, skippedBy, @@ -142,12 +120,7 @@ public class DependencyResolveReconciler CollectResult result ) throws DependencyCollectionException { - if ( !this.skipMode ) - { - return Collections.emptyList(); - } - - ConflictWinnerFinder winnerFinder = new ConflictWinnerFinder( verbose ); + ConflictWinnerFinder winnerFinder = new ConflictWinnerFinder(); Collection<ConflictWinnerFinder.Conflict> conflicts = winnerFinder.getVersionConflicts( session, result ); if ( conflicts == null || conflicts.isEmpty() ) { @@ -155,8 +128,7 @@ public class DependencyResolveReconciler } //find all nodes that contain conflict nodes. - Set<String> conflictedNodes = new HashSet<>(); - conflictedNodes.addAll( findNodeHasConflictsInParentPaths( conflicts ) ); + HashSet<String> conflictedNodes = new HashSet<>( findNodeHasConflictsInParentPaths( conflicts ) ); //Evict node from cache if the cached node is based on parent paths contains conflicting nodes for ( String gav : conflictedNodes ) @@ -171,9 +143,9 @@ public class DependencyResolveReconciler return Collections.emptyList(); } - if ( verbose ) + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", + LOGGER.debug( "Skipped resolving {} nodes, and reconciled {} nodes to solve {} dependency conflicts.", skippedNodes.size(), filteredSkips.size(), conflicts.size() ); } return filteredSkips; @@ -211,10 +183,10 @@ public class DependencyResolveReconciler } //group nodes by artifact - Multimap<Artifact, DependencyResolveSkip> reconcileNodes = LinkedHashMultimap.create(); + HashMap<Artifact, ArrayList<DependencyResolveSkip>> reconcileNodes = new HashMap<>(); for ( DependencyResolveSkip skip : skips ) { - reconcileNodes.put( skip.node.getArtifact(), skip ); + reconcileNodes.computeIfAbsent( skip.node.getArtifact(), k -> new ArrayList<>() ).add( skip ); } //only reconcile the node with lowest depth @@ -222,20 +194,11 @@ public class DependencyResolveReconciler for ( Artifact key : reconcileNodes.keySet() ) { Collection<DependencyResolveSkip> col = reconcileNodes.get( key ); - List<DependencyResolveSkip> list = new ArrayList<>(); - list.addAll( col ); - Collections.sort( list, new Comparator<DependencyResolveSkip>() - { - @Override - public int compare( DependencyResolveSkip o1, DependencyResolveSkip o2 ) - { - return o1.depth - o2.depth; - } - } ); - - if ( verbose ) + List<DependencyResolveSkip> list = new ArrayList<>( col ); + list.sort( Comparator.comparingInt( o -> o.depth ) ); + if ( LOGGER.isDebugEnabled() ) { - LOGGER.info( "Reconcile: {}", list.get( 0 ) ); + LOGGER.debug( "Reconcile: {}", list.get( 0 ) ); } filteredSkips.add( list.get( 0 ) ); } diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java index a7932cc..9145ee8 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java @@ -110,7 +110,7 @@ public class DefaultDependencyCollectorTest public void setupCollector(boolean skipAndReconcile) { session = TestUtils.newSession(); - session.setConfigProperty(DefaultDependencyCollector.CONFIG_PROP_RESOLVER_MODE, skipAndReconcile); + session.setConfigProperty(DefaultDependencyCollector.CONFIG_PROP_USE_SKIP_RECONCILE, skipAndReconcile); collector = new DefaultDependencyCollector(); collector.setArtifactDescriptorReader( newReader( "" ) ); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java index 79060bd..c3de72d 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DependencyResolveReconcilerTest.java @@ -131,7 +131,7 @@ public class DependencyResolveReconcilerTest result.setRoot( aNode ); //follow the resolve sequence - DependencyResolveReconciler reconciler = new DependencyResolveReconciler( true, true ); + DependencyResolveReconciler reconciler = new DependencyResolveReconciler(); reconciler.cacheChildrenWithDepth( aNode, new ArrayList<>() ); reconciler.cacheChildrenWithDepth( bNode, mutableList( aNode ) ); reconciler.cacheChildrenWithDepth( c3Node, mutableList( aNode, bNode ) );
