You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2023/02/02 21:56:58 UTC

[maven-resolver] branch master updated: [MRESOLVER-317] Improvements for BF collector (#238)

This is an automated email from the ASF dual-hosted git repository.

cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git


The following commit(s) were added to refs/heads/master by this push:
     new 2000d315 [MRESOLVER-317] Improvements for BF collector (#238)
2000d315 is described below

commit 2000d315f2bfdd03b6c32e79a1cb21c92d622f0a
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Thu Feb 2 22:56:52 2023 +0100

    [MRESOLVER-317] Improvements for BF collector (#238)
    
    Changes:
    * make skipper closeable
    * make parallel descriptor resolver closeable
    * the two above should be used within try-with-resource block
    * improve encapsulation of resolver
    
    ---
    
    https://issues.apache.org/jira/browse/MRESOLVER-317
---
 .../impl/collect/bf/BfDependencyCollector.java     | 100 ++++++++++-----------
 .../collect/bf/DependencyResolutionSkipper.java    |  12 +--
 2 files changed, 54 insertions(+), 58 deletions(-)

diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollector.java
index 7e996769..c6e63e56 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollector.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/BfDependencyCollector.java
@@ -23,6 +23,7 @@ import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import java.io.Closeable;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -76,8 +77,6 @@ import org.eclipse.aether.util.artifact.ArtifactIdUtils;
 import org.eclipse.aether.util.concurrency.WorkerThreadFactory;
 import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
 import org.eclipse.aether.version.Version;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import static org.eclipse.aether.internal.impl.collect.DefaultDependencyCycle.find;
 
@@ -146,52 +145,54 @@ public class BfDependencyCollector
         boolean useSkip = ConfigUtils.getBoolean(
                 session, CONFIG_PROP_SKIPPER_DEFAULT, CONFIG_PROP_SKIPPER
         );
+        int nThreads = ConfigUtils.getInteger( session, 5, CONFIG_PROP_THREADS, "maven.artifact.threads" );
+        logger.debug( "Using thread pool with {} threads to resolve descriptors.", nThreads );
+
         if ( useSkip )
         {
             logger.debug( "Collector skip mode enabled" );
         }
 
-        Args args =
-                new Args( session, pool, context, versionContext, request,
-                        useSkip ? DependencyResolutionSkipper.defaultSkipper()
-                                : DependencyResolutionSkipper.neverSkipper(),
-                        new ParallelDescriptorResolver( session ) );
-
-        DependencySelector rootDepSelector = session.getDependencySelector() != null
-                ? session.getDependencySelector().deriveChildSelector( context ) : null;
-        DependencyManager rootDepManager = session.getDependencyManager() != null
-                ? session.getDependencyManager().deriveChildManager( context ) : null;
-        DependencyTraverser rootDepTraverser = session.getDependencyTraverser() != null
-                ? session.getDependencyTraverser().deriveChildTraverser( context ) : null;
-        VersionFilter rootVerFilter = session.getVersionFilter() != null
-                ? session.getVersionFilter().deriveChildFilter( context ) : null;
-
-        List<DependencyNode> parents = Collections.singletonList( node );
-        for ( Dependency dependency : dependencies )
+        try ( DependencyResolutionSkipper skipper = useSkip
+                ? DependencyResolutionSkipper.defaultSkipper() : DependencyResolutionSkipper.neverSkipper();
+              ParallelDescriptorResolver parallelDescriptorResolver = new ParallelDescriptorResolver( nThreads ) )
         {
-            RequestTrace childTrace = collectStepTrace( trace, args.request.getRequestContext(), parents,
-                    dependency );
-            DependencyProcessingContext processingContext =
-                    new DependencyProcessingContext( rootDepSelector, rootDepManager, rootDepTraverser,
-                            rootVerFilter, childTrace, repositories, managedDependencies, parents, dependency,
-                            PremanagedDependency.create( rootDepManager, dependency,
-                                    false, args.premanagedState ) );
-            if ( !filter( processingContext ) )
+            Args args = new Args(
+                    session, pool, context, versionContext, request, skipper, parallelDescriptorResolver );
+
+            DependencySelector rootDepSelector = session.getDependencySelector() != null
+                    ? session.getDependencySelector().deriveChildSelector( context ) : null;
+            DependencyManager rootDepManager = session.getDependencyManager() != null
+                    ? session.getDependencyManager().deriveChildManager( context ) : null;
+            DependencyTraverser rootDepTraverser = session.getDependencyTraverser() != null
+                    ? session.getDependencyTraverser().deriveChildTraverser( context ) : null;
+            VersionFilter rootVerFilter = session.getVersionFilter() != null
+                    ? session.getVersionFilter().deriveChildFilter( context ) : null;
+
+            List<DependencyNode> parents = Collections.singletonList( node );
+            for ( Dependency dependency : dependencies )
             {
-                processingContext.withDependency( processingContext.premanagedDependency.getManagedDependency() );
-                resolveArtifactDescriptorAsync( args, processingContext, results );
-                args.dependencyProcessingQueue.add( processingContext );
+                RequestTrace childTrace = collectStepTrace( trace, args.request.getRequestContext(), parents,
+                        dependency );
+                DependencyProcessingContext processingContext =
+                        new DependencyProcessingContext( rootDepSelector, rootDepManager, rootDepTraverser,
+                                rootVerFilter, childTrace, repositories, managedDependencies, parents, dependency,
+                                PremanagedDependency.create( rootDepManager, dependency,
+                                        false, args.premanagedState ) );
+                if ( !filter( processingContext ) )
+                {
+                    processingContext.withDependency( processingContext.premanagedDependency.getManagedDependency() );
+                    resolveArtifactDescriptorAsync( args, processingContext, results );
+                    args.dependencyProcessingQueue.add( processingContext );
+                }
             }
-        }
 
-        while ( !args.dependencyProcessingQueue.isEmpty() )
-        {
-            processDependency( args, results, args.dependencyProcessingQueue.remove(), Collections.emptyList(),
-                    false );
+            while ( !args.dependencyProcessingQueue.isEmpty() )
+            {
+                processDependency( args, results, args.dependencyProcessingQueue.remove(), Collections.emptyList(),
+                        false );
+            }
         }
-
-        args.resolver.shutdown();
-        args.skipper.report();
     }
 
     @SuppressWarnings( "checkstyle:parameternumber" )
@@ -473,19 +474,19 @@ public class BfDependencyCollector
         return descriptorResult;
     }
 
-    static class ParallelDescriptorResolver
+    static class ParallelDescriptorResolver implements Closeable
     {
-        final ExecutorService executorService;
+        private final ExecutorService executorService;
 
         /**
          * Artifact ID -> Future of DescriptorResolutionResult
          */
-        final Map<String, Future<DescriptorResolutionResult>> results = new ConcurrentHashMap<>( 256 );
-        final Logger logger = LoggerFactory.getLogger( getClass() );
+        private final Map<String, Future<DescriptorResolutionResult>> results = new ConcurrentHashMap<>( 256 );
 
-        ParallelDescriptorResolver( RepositorySystemSession session )
+        ParallelDescriptorResolver( int threads )
         {
-            this.executorService = getExecutorService( session );
+            this.executorService = new ThreadPoolExecutor( threads, threads, 3L, TimeUnit.SECONDS,
+                    new LinkedBlockingQueue<>(), new WorkerThreadFactory( getClass().getSimpleName() + "-" ) );
         }
 
         void resolveDescriptors( Artifact artifact, Callable<DescriptorResolutionResult> callable )
@@ -505,18 +506,11 @@ public class BfDependencyCollector
             return results.get( ArtifactIdUtils.toId( artifact ) );
         }
 
-        void shutdown()
+        @Override
+        public void close()
         {
             executorService.shutdown();
         }
-
-        private ExecutorService getExecutorService( RepositorySystemSession session )
-        {
-            int nThreads = ConfigUtils.getInteger( session, 5, CONFIG_PROP_THREADS, "maven.artifact.threads" );
-            logger.debug( "Created thread pool with {} threads to resolve descriptors.", nThreads );
-            return new ThreadPoolExecutor( nThreads, nThreads, 3L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(),
-                    new WorkerThreadFactory( getClass().getSimpleName() ) );
-        }
     }
 
     static class DescriptorResolutionResult
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/DependencyResolutionSkipper.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/DependencyResolutionSkipper.java
index b82a8799..34107046 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/DependencyResolutionSkipper.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/bf/DependencyResolutionSkipper.java
@@ -25,6 +25,7 @@ import org.eclipse.aether.util.artifact.ArtifactIdUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.Closeable;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -37,7 +38,7 @@ import java.util.concurrent.atomic.AtomicInteger;
  *
  * @since 1.8.0
  */
-abstract class DependencyResolutionSkipper
+abstract class DependencyResolutionSkipper implements Closeable
 {
     /**
      * Check whether the resolution of current node can be skipped before resolving.
@@ -58,9 +59,10 @@ abstract class DependencyResolutionSkipper
     abstract void cache( DependencyNode node, List<DependencyNode> parents );
 
     /**
-     * Print the skip/resolve status report for all nodes.
+     * Close: Print the skip/resolve status report for all nodes.
      */
-    abstract void report();
+    @Override
+    public abstract void close();
 
     /**
      * Returns new instance of "default" skipper.
@@ -99,7 +101,7 @@ abstract class DependencyResolutionSkipper
         }
 
         @Override
-        public void report()
+        public void close()
         {
         }
     }
@@ -205,7 +207,7 @@ abstract class DependencyResolutionSkipper
         }
 
         @Override
-        public void report()
+        public void close()
         {
             if ( LOGGER.isTraceEnabled() )
             {