[ 
https://issues.apache.org/jira/browse/MRESOLVER-283?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17628339#comment-17628339
 ] 

ASF GitHub Bot commented on MRESOLVER-283:
------------------------------------------

caiwei-ebay commented on code in PR #213:
URL: https://github.com/apache/maven-resolver/pull/213#discussion_r1012891403


##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/concurrency/DefaultResolverExecutor.java:
##########
@@ -0,0 +1,174 @@
+package org.eclipse.aether.internal.impl.concurrency;
+
+/*
+ * 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 javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.Collection;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.aether.impl.RepositorySystemLifecycle;
+import org.eclipse.aether.spi.concurrency.ResolverExecutor;
+import org.eclipse.aether.spi.locator.Service;
+import org.eclipse.aether.spi.locator.ServiceLocator;
+import org.eclipse.aether.util.concurrency.WorkerThreadFactory;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Default implementation of {@link ResolverExecutor}.
+ * <p>
+ * If configured threads count is 1, direct invocation happens, otherwise a 
thread pool ({@link ExecutorService}) is
+ * being used. If needed, creates a single shared {@link ExecutorService} 
instance, that is shut along with repository
+ * system.
+ */
+@Singleton
+@Named
+public final class DefaultResolverExecutor implements ResolverExecutor, Service
+{
+    private static final String CONF_PROP_THREADS = "aether.executor.threads";
+
+    private static final int DEFAULT_THREADS = 5;
+
+    private final ExecutorService executorService;
+
+    private final int threads;
+
+    /**
+     * SL ctor.
+     *
+     * @deprecated For use in SL only.
+     */
+    @Deprecated
+    public DefaultResolverExecutor()
+    {
+        this.threads = DEFAULT_THREADS;
+        this.executorService = createExecutorService( threads );
+    }
+
+    @Inject
+    public DefaultResolverExecutor( @Named( "${" + CONF_PROP_THREADS + ":-" + 
DEFAULT_THREADS + "}" ) final int threads,
+                                    RepositorySystemLifecycle lifecycle )
+    {
+        if ( threads < 1 )
+        {
+            throw new IllegalArgumentException( "threads must be greater than 
zero" );
+        }
+        requireNonNull( lifecycle );
+        this.threads = threads;
+        this.executorService = createExecutorService( threads );
+        lifecycle.addOnSystemEndedHandler( this::shutdown );
+    }
+
+    private ExecutorService createExecutorService( int threads )
+    {
+        if ( threads == 1 )
+        {
+            return null; // we use direct execution
+        }
+        else
+        {
+            return new ThreadPoolExecutor( 0, threads, 3L, TimeUnit.SECONDS,
+                    new LinkedBlockingQueue<>(), new WorkerThreadFactory( 
getClass().getSimpleName() + '-' ) );
+        }
+    }
+
+    @Override
+    public void initService( ServiceLocator locator )
+    {
+        locator.getService( RepositorySystemLifecycle.class 
).addOnSystemEndedHandler( this::shutdown );
+    }
+
+    @Override
+    public void submitOrDirect( Collection<Runnable> tasks )
+    {
+        requireNonNull( tasks );

Review Comment:
   When resolves a snapshot dependency or dependency with version range, Maven 
resolves metadata.xml from multiple repositories in parallel: 
https://github.com/apache/maven/blob/master/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java#L158
   
   With original design, metadata.xml is using a separate pool, pom or jar can 
use RepositoryConnector's pool but pom downloading is in serial while jar is in 
parallel:
   
   - metadata.xml use a separate pool to download metadata.xml from multiple 
repositories, each downloading calls RepositoryConnector's **get(null, 
Collections.singleList(MetadataDownload))**, it actually uses DirectExecutor 
without leveraging RepositoryConnector's thread pool; 
   - pom downloading directly calls RepositoryConnector's 
get(Collections.singleList(ArtifactDownload), null), so it also uses 
DirectExecutor without leveraging RepositoryConnector's thread pool; 
   - jar downloading calls RepositoryConnector's get(List(ArtifactDownload), 
null), it do leverage RepositoryConnector's thread pool. 
   
   With this approach, metadata, pom and jar all share one pool:
   
   Suppose we have multiple repositories like releases, snapshots, commercial, 
legacy, testing, etc. in our private Maven nexus repository and we need to 
resolve 5 dependencies (snapshot or with version range) in parallel. 
   
   When resolve with BFDependencyCollector,  5 tasks to resolve each dependency 
have used up all 5 threads, and due to resolving each dependency (let's name 
with task A) need to resolve metadata (task B), the metadata resolution task B 
will be also submitted to the same pool. Here A depends on B but B has to wait 
available threads. I think this could be lead to endless waiting, right?
   
   As it is better to use separate thread pool for separate tasks, maybe the 
SharedExecutor here should have separate pool for separate tasks? At least, BF 
collect and pom/jar downloading can use one pool and metadata.xml downloading 
still use another pool? Please advice.
   





> Introduce resolver wide "shared" executor service
> -------------------------------------------------
>
>                 Key: MRESOLVER-283
>                 URL: https://issues.apache.org/jira/browse/MRESOLVER-283
>             Project: Maven Resolver
>          Issue Type: Improvement
>          Components: Resolver
>            Reporter: Tamas Cservenak
>            Assignee: Tamas Cservenak
>            Priority: Major
>             Fix For: 1.9.0
>
>
> More and more component in resolver does parallel processing (BF collector, 
> MD resolver, basic connector), and they all create, maintain their own 
> executor instance.
> Instead of this, create one shared "thread pool" (executor) component and 
> just share it accross resovler.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to