MRESOLVER-247: Introduce skipper to avoid unnecessary dependency resolution

Author: caiwei-ebay

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java

@@ -253,7 +253,8 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle
 
             DefaultVersionFilterContext versionContext = new DefaultVersionFilterContext( session );
 
-            Args args = new Args( session, trace, pool, context, versionContext, request );
+            Args args =
+                    new Args( session, trace, pool, context, versionContext, request, new DependencyResolveSkipper() );
             Results results = new Results( result, session );
 
             DependencySelector rootDepSelector =
@@ -277,6 +278,7 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle
                         false );
             }
 
+            args.skipper.report();
             errorPath = results.errorPath;
         }
 
@@ -396,6 +398,8 @@ private void processDependency( Args args, Results results, DependencyProcessing
             return;
         }
 
+        //Resolve newer version first to maximize benefits of skipper
+        Collections.reverse( versions );
         for ( Version version : versions )
         {
             Artifact originalArtifact = dependency.getArtifact().setVersion( version.toString() );
@@ -500,15 +504,19 @@ private void doRecurse( Args args, DependencyProcessingContext parentContext,
         List<DependencyNode> children = args.pool.getChildren( key );
         if ( children == null )
         {
-            args.pool.putChildren( key, child.getChildren() );
-
             List<DependencyNode> parents = new ArrayList<>( parentContext.parents );
-            parents.add( child );
-            for ( Dependency dependency : descriptorResult.getDependencies() )
+            boolean skipResolve = args.skipper.skipResolve( child, parents );
+            if ( !skipResolve )
             {
-                args.dependencyProcessingQueue.add(
-                        new DependencyProcessingContext( childSelector, childManager, childTraverser, childFilter,
-                                childRepos, descriptorResult.getManagedDependencies(), parents, dependency ) );
+                parents.add( child );
+                for ( Dependency dependency : descriptorResult.getDependencies() )
+                {
+                    args.dependencyProcessingQueue.add(
+                            new DependencyProcessingContext( childSelector, childManager, childTraverser, childFilter,
+                                    childRepos, descriptorResult.getManagedDependencies(), parents, dependency ) );
+                }
+                args.pool.putChildren( key, child.getChildren() );
+                args.skipper.cacheWithDepth( child, parents );
             }
         }
         else
@@ -697,9 +705,11 @@ static class Args
 
         final CollectRequest request;
 
+        final DependencyResolveSkipper skipper;
+
         Args( RepositorySystemSession session, RequestTrace trace, DataPool pool,
                      DefaultDependencyCollectionContext collectionContext, DefaultVersionFilterContext versionContext,
-                     CollectRequest request )
+                     CollectRequest request, DependencyResolveSkipper skipper )
         {
             this.session = session;
             this.request = request;
@@ -709,6 +719,7 @@ static class Args
             this.pool = pool;
             this.collectionContext = collectionContext;
             this.versionContext = versionContext;
+            this.skipper = skipper;
         }
 
     }

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyResolveSkipper.java

@@ -0,0 +1,260 @@
+package org.eclipse.aether.internal.impl.collect;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.graph.DependencyNode;
+import org.eclipse.aether.util.artifact.ArtifactIdUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+final class DependencyResolveSkipper
+{
+    private static final Logger LOGGER = LoggerFactory.getLogger( DependencyResolveSkipper.class );
+
+    private Map<DependencyNode, DependencyResolveResult> resolveResults = new LinkedHashMap<>( 256 );
+    private CacheManager cacheManager = new CacheManager();
+    private CoordinateManager coordinateManager = new CoordinateManager();
+
+    DependencyResolveSkipper()
+    {
+        // enables default constructor
+    }
+
+    Map<DependencyNode, DependencyResolveResult> report()
+    {
+        LOGGER.trace( "Skipped {} nodes as having deeper depth.",
+                resolveResults.entrySet().stream().filter( n -> n.getValue().skippedAsDeeper ).count() );
+        LOGGER.trace( "Skipped {} nodes as having version conflict.",
+                resolveResults.entrySet().stream().filter( n -> n.getValue().skippedAsVersionConflict ).count() );
+        LOGGER.trace( "Resolved {} nodes.",
+                resolveResults.entrySet().stream().filter( n -> n.getValue().resolve ).count() );
+        LOGGER.trace( "Force resolved {} nodes for scope selection.",
+                resolveResults.entrySet().stream().filter( n -> n.getValue().forceResolve ).count() );
+
+        return resolveResults;
+    }
+
+
+    boolean skipResolve( DependencyNode node, List<DependencyNode> parents )
+    {
+        DependencyResolveResult resolveResult = new DependencyResolveResult( node );
+        resolveResults.put( node, resolveResult );
+
+        int depth = parents.size() + 1;
+        coordinateManager.createCoordinate( node, depth );
+
+        if ( cacheManager.isVersionConflict( node ) )
+        {
+            //skip resolving version conflict losers (omitted conflict)
+            resolveResult.skippedAsVersionConflict = true;
+            LOGGER.trace( "Skipped resolving node: {} as version conflict",
+                    ArtifactIdUtils.toId( node.getArtifact() ) );
+        }
+        else if ( coordinateManager.isLeftmost( node, parents ) )
+        {
+            /*
+             * Force resolving the node to retain conflict paths when its coordinate is more left than last resolved.
+             * This is because maven picks the widest scope present among conflicting dependencies.
+             */
+            resolveResult.forceResolve = true;
+            LOGGER.trace( "Force resolving node: {} for scope selection",
+                    ArtifactIdUtils.toId( node.getArtifact() ) );
+        }
+        else if ( cacheManager.getWinnerDepth( node ) <= depth )
+        {
+            //skip resolve if depth deeper (omitted duplicate)
+            resolveResult.skippedAsDeeper = true;
+            LOGGER.trace( "Skipped resolving node: {} as the node's depth is deeper than winner",
+                    ArtifactIdUtils.toId( node.getArtifact() ) );
+        }
+        else
+        {
+            resolveResult.resolve = true;
+            LOGGER.trace( "Resolving node: {}",
+                    ArtifactIdUtils.toId( node.getArtifact() ) );
+        }
+
+        if ( resolveResult.toResolve() )
+        {
+            coordinateManager.updateLeftmost( node );
+            return false;
+        }
+
+        return true;
+    }
+
+    void cacheWithDepth( DependencyNode node, List<DependencyNode> parents )
+    {
+        boolean parentForceResolve = parents.stream()
+                .anyMatch( n -> resolveResults.containsKey( n ) && resolveResults.get( n ).forceResolve );
+        if ( parentForceResolve )
+        {
+            LOGGER.trace(
+                    "Won't cache as current node: {} inherits from a force-resolved node and will be omitted duplicate",
+                    ArtifactIdUtils.toId( node.getArtifact() ) );
+        }
+        else
+        {
+            cacheManager.cacheWinnerDepth( node, parents.size() + 1 );
+        }
+    }
+
+
+    static final class DependencyResolveResult
+    {
+        DependencyNode current;
+        boolean skippedAsVersionConflict; //omitted conflict
+        boolean skippedAsDeeper; //omitted duplicate
+        boolean resolve; //node to resolve (winner node)
+        boolean forceResolve; //force resolve (duplicate code) for scope selection
+
+        DependencyResolveResult( DependencyNode current )
+        {
+            this.current = current;
+        }
+
+        boolean toResolve()
+        {
+            return resolve || forceResolve;
+        }
+    }
+
+    static final class CacheManager
+    {
+
+        /**
+         * artifact -> depth, only cache winners.
+         */
+        private final HashMap<Artifact, Integer> winnerDepths = new HashMap<>( 256 );
+
+
+        /**
+         * versionLessId -> Artifact, only cache winners
+         */
+        private final Map<String, Artifact> winnerGAs = new HashMap<>( 256 );
+
+        boolean isVersionConflict( DependencyNode node )
+        {
+            String ga = ArtifactIdUtils.toVersionlessId( node.getArtifact() );
+            if ( winnerGAs.containsKey( ga ) )
+            {
+                Artifact result = winnerGAs.get( ga );
+                return !node.getArtifact().getVersion().equals( result.getVersion() );
+            }
+
+            return false;
+        }
+
+        void cacheWinnerDepth( DependencyNode node, int depth )
+        {
+            LOGGER.trace( "Artifact {} with depth {} cached", node.getArtifact(), depth );
+            winnerDepths.put( node.getArtifact(), depth );
+            winnerGAs.put( ArtifactIdUtils.toVersionlessId( node.getArtifact() ), node.getArtifact() );
+        }
+
+        int getWinnerDepth( DependencyNode node )
+        {
+            return winnerDepths.getOrDefault( node.getArtifact(), Integer.MAX_VALUE );
+        }
+
+    }
+
+
+    static final class CoordinateManager
+    {
+        private final Map<Integer, AtomicInteger> sequenceGen = new HashMap<>( 256 );
+
+        /**
+         * Dependency node -> Coordinate
+         */
+        private final Map<DependencyNode, Coordinate> coordinateMap = new HashMap<>( 256 );
+
+        /**
+         * Leftmost coordinate of given artifact
+         */
+        private final Map<Artifact, Coordinate> leftmostCoordinates = new HashMap<>( 256 );
+
+
+        Coordinate getCoordinate( DependencyNode node )
+        {
+            return coordinateMap.get( node );
+        }
+
+        Coordinate createCoordinate( DependencyNode node, int depth )
+        {
+            int seq = sequenceGen.computeIfAbsent( depth, k -> new AtomicInteger() ).incrementAndGet();
+            Coordinate coordinate = new Coordinate( depth, seq );
+            coordinateMap.put( node, coordinate );
+            return coordinate;
+        }
+
+        void updateLeftmost( DependencyNode current )
+        {
+            leftmostCoordinates.put( current.getArtifact(), getCoordinate( current ) );
+        }
+
+        boolean isLeftmost( DependencyNode node, List<DependencyNode> parents )
+        {
+            Coordinate leftmost = leftmostCoordinates.get( node.getArtifact() );
+            if ( leftmost != null && leftmost.depth <= parents.size() )
+            {
+                DependencyNode sameLevelNode = parents.get( leftmost.depth - 1 );
+                if ( getCoordinate( sameLevelNode ).sequence < leftmost.sequence )
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+    }
+
+    static final class Coordinate
+    {
+        int depth;
+        int sequence;
+
+        Coordinate( int depth, int sequence )
+        {
+            this.depth = depth;
+            this.sequence = sequence;
+        }
+
+        @Override
+        public String toString()
+        {
+            return "{"
+                    + "depth="
+                    + depth
+                    + ", sequence="
+                    + sequence
+                    + '}';
+        }
+    }
+
+
+}

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectorTest.java

@@ -69,6 +69,7 @@
 import org.eclipse.aether.util.graph.manager.DefaultDependencyManager;
 import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
 import org.eclipse.aether.util.graph.manager.TransitiveDependencyManager;
+import org.eclipse.aether.util.graph.selector.ExclusionDependencySelector;
 import org.eclipse.aether.util.graph.version.HighestVersionFilter;
 import org.junit.Before;
 import org.junit.Test;
@@ -101,6 +102,12 @@ private Dependency newDep( String coords, String scope )
         return new Dependency( new DefaultArtifact( coords ), scope );
     }
 
+    private Dependency newDep( String coords, String scope, Collection<Exclusion> exclusions )
+    {
+        Dependency d = new Dependency( new DefaultArtifact( coords ), scope );
+        return d.setExclusions(exclusions);
+    }
+
     @Before
     public void setup()
     {
@@ -222,6 +229,26 @@ public void testMissingDependencyDescription()
         }
     }
 
+    @Test
+    public void testExclusion1()
+            throws DependencyCollectionException {
+        collector.setArtifactDescriptorReader(newReader("managed/"));
+        parser = new DependencyGraphParser("artifact-descriptions/managed/");
+        session.setDependencyManager(new TransitiveDependencyManager());
+
+        ExclusionDependencySelector exclSel1 = new ExclusionDependencySelector();
+        session.setDependencySelector(exclSel1);
+
+        Dependency root1 = newDep("gid:root:ext:ver", "compile", Collections.singleton(new Exclusion("gid", "transitive-1", "", "ext")));
+        Dependency root2 = newDep("gid:root:ext:ver", "compile", Collections.singleton(new Exclusion("gid", "transitive-2", "", "ext")));
+        List<Dependency> dependencies = Arrays.asList(root1, root2);
+        CollectRequest request = new CollectRequest(dependencies, null, Arrays.asList(repository));
+        request.addManagedDependency(newDep("gid:direct:ext:managed-by-dominant-request"));
+        request.addManagedDependency( newDep( "gid:transitive-1:ext:managed-by-root" ) );
+        CollectResult result = collector.collectDependencies(session, request);
+        System.err.println(result);
+    }
+
     @Test
     public void testDuplicates()
         throws DependencyCollectionException