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

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

cstamas commented on code in PR #272:
URL: https://github.com/apache/maven-resolver/pull/272#discussion_r1158054237


##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java:
##########
@@ -256,207 +257,226 @@ public List<ArtifactResult> resolveArtifacts(
                 artifacts.add(request.getArtifact());
             }
 
-            syncContext.acquire(artifacts, null);
-
-            return resolve(session, requests);
+            return resolve(shared, exclusive, artifacts, session, requests);
         }
     }
 
     @SuppressWarnings("checkstyle:methodlength")
     private List<ArtifactResult> resolve(
-            RepositorySystemSession session, Collection<? extends 
ArtifactRequest> requests)
+            SyncContext shared,
+            SyncContext exclusive,
+            Collection<Artifact> subjects,
+            RepositorySystemSession session,
+            Collection<? extends ArtifactRequest> requests)
             throws ArtifactResolutionException {
-        List<ArtifactResult> results = new ArrayList<>(requests.size());
-        boolean failures = false;
-        final boolean simpleLrmInterop = ConfigUtils.getBoolean(session, 
false, CONFIG_PROP_SIMPLE_LRM_INTEROP);
+        SyncContext current = shared;
+        try {
+            while (true) {

Review Comment:
   I don't see how: last element within this while-true loop is `return 
results`, meaning if we reach here even only once, we are out of the loop.





> Too eager locking
> -----------------
>
>                 Key: MRESOLVER-346
>                 URL: https://issues.apache.org/jira/browse/MRESOLVER-346
>             Project: Maven Resolver
>          Issue Type: Bug
>          Components: Resolver
>            Reporter: Tamas Cservenak
>            Assignee: Tamas Cservenak
>            Priority: Major
>             Fix For: 1.9.8
>
>
> The locking that is present in Resolver since 1.8.0 is too eager:
>  * there are no shared locks used at all
>  * this implies that local repository access is heavily serialized by locking
>  * there is no "upgrade" of locking due that above
> * consequence is that "hot" artifacts in bigger multi module build run in 
> parallel become bottleneck as all threads will wait for their moment to grab 
> exclusive lock.
> * another consequence: our "wait time" (def 30s) becomes problem, as due that 
> above, if build grows, the plausible "wait time" (as all lock is exclusive, 
> but requester count grows) grows as well. Also, this means we have threads 
> there doing nothing, just sitting as they wait for exclusive lock one after 
> another.
> We can do it better: there are 4 main areas where locking is used:
> * ArtifactInstaller: it is about to install (write) artifact files to local 
> repository, it needs exclusive lock, *no change needed*.
> * ArtifactDeployer: it is about to upload present files to remote, it does 
> not modifies anything on local. *Change its lock to shared*. The exclusive 
> lock also meant that if no DeployAtEnd used, other modules during resolution 
> had to wait while this module uploaded.
> * ArtifactResolver and MetadataResolver: two very similar beasts, they 
> attempt to resolve locally (from local repo) w/o any content modification 
> (read only), and if not successful, they will reach remote to download what 
> is needed (write). Here we *could do something similar to 
> [DCL|https://en.wikipedia.org/wiki/Double-checked_locking] is*: try with 
> shared lock first, and if local content is not fulfilling, release shared, 
> acquire exclusive and REDO all (as meanwhile someone else may downloaded 
> files already).



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

Reply via email to