[ 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)