From 405381a8430415c66667430a5188322396c128de Mon Sep 17 00:00:00 2001 From: Ivan Babiankou Date: Sun, 12 Dec 2021 19:27:13 +0100 Subject: [PATCH 1/6] [MRESOLVER-7] Localize usage of DefaultVersionFilterContext Ensures DefaultVersionFilterContext instances are not shared between threads --- .../collect/DefaultDependencyCollector.java | 20 +++----- .../collect/DefaultVersionFilterContext.java | 12 ++--- .../DefaultVersionFilterContextTest.java | 47 ++++++------------- 3 files changed, 25 insertions(+), 54 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index a46499dfd..c78465814 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -176,7 +176,7 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle request.getRequestContext() ); rangeRequest.setTrace( trace ); rangeResult = versionRangeResolver.resolveVersionRange( session, rangeRequest ); - versions = filterVersions( root, rangeResult, verFilter, new DefaultVersionFilterContext( session ) ); + versions = filterVersions( root, rangeResult, verFilter, session ); } catch ( VersionRangeResolutionException e ) { @@ -250,9 +250,7 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle DefaultDependencyCollectionContext context = new DefaultDependencyCollectionContext( session, request.getRootArtifact(), root, managedDependencies ); - DefaultVersionFilterContext versionContext = new DefaultVersionFilterContext( session ); - - Args args = new Args( session, trace, pool, nodes, context, versionContext, request ); + Args args = new Args( session, trace, pool, nodes, context, request ); Results results = new Results( result, session ); process( args, results, dependencies, repositories, @@ -394,7 +392,7 @@ private void processDependency( Args args, Results results, List getRemoteRepositories( ArtifactRepository } private static List filterVersions( Dependency dependency, VersionRangeResult rangeResult, - VersionFilter verFilter, - DefaultVersionFilterContext verContext ) + VersionFilter verFilter, RepositorySystemSession session ) throws VersionRangeResolutionException { if ( rangeResult.getVersions().isEmpty() ) @@ -659,7 +656,8 @@ private static List filterVersions( Dependency dependency, Ve List versions; if ( verFilter != null && rangeResult.getVersionConstraint().getRange() != null ) { - verContext.set( dependency, rangeResult ); + DefaultVersionFilterContext verContext = + new DefaultVersionFilterContext( session, dependency, rangeResult ); try { verFilter.filterVersions( verContext ); @@ -702,13 +700,10 @@ static class Args final DefaultDependencyCollectionContext collectionContext; - final DefaultVersionFilterContext versionContext; - final CollectRequest request; Args( RepositorySystemSession session, RequestTrace trace, DataPool pool, NodeStack nodes, - DefaultDependencyCollectionContext collectionContext, DefaultVersionFilterContext versionContext, - CollectRequest request ) + DefaultDependencyCollectionContext collectionContext, CollectRequest request ) { this.session = session; this.request = request; @@ -718,7 +713,6 @@ static class Args this.pool = pool; this.nodes = nodes; this.collectionContext = collectionContext; - this.versionContext = versionContext; } } diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContext.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContext.java index bfea0626e..7666a2b87 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContext.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContext.java @@ -41,19 +41,15 @@ final class DefaultVersionFilterContext { private final RepositorySystemSession session; - private Dependency dependency; + private final Dependency dependency; - VersionRangeResult result; + private final VersionRangeResult result; - private List versions; + private final List versions; - DefaultVersionFilterContext( RepositorySystemSession session ) + DefaultVersionFilterContext( RepositorySystemSession session, Dependency dependency, VersionRangeResult result ) { this.session = session; - } - - public void set( Dependency dependency, VersionRangeResult result ) - { this.dependency = dependency; this.result = result; this.versions = new ArrayList<>( result.getVersions() ); diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContextTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContextTest.java index b6fbf5fdb..c408c21c4 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContextTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultVersionFilterContextTest.java @@ -39,15 +39,14 @@ public class DefaultVersionFilterContextTest { private static final Dependency FOO_DEPENDENCY = new Dependency( new DefaultArtifact( "group-id:foo:1.0" ), "test" ); - private static final Dependency BAR_DEPENDENCY = new Dependency( new DefaultArtifact( "group-id:bar:1.0" ), "test" ); @Test public void iteratorOneItem() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = + new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); Iterator iterator = context.iterator(); assertTrue( iterator.hasNext() ); @@ -57,10 +56,9 @@ public void iteratorOneItem() @Test public void getCountOneItem() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); assertEquals(1, context.getCount()); } @@ -68,10 +66,10 @@ public void getCountOneItem() @Test public void getOneItem() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = + new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); assertEquals( Collections.singletonList( new TestVersion( "1.0") ), context.get() ); } @@ -79,10 +77,10 @@ public void getOneItem() @Test public void iteratorDelete() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = + new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); Iterator iterator = context.iterator(); iterator.next(); @@ -94,10 +92,10 @@ public void iteratorDelete() @Test(expected = NoSuchElementException.class) public void nextBeyondEnd() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = + new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); Iterator iterator = context.iterator(); iterator.next(); @@ -107,10 +105,10 @@ public void nextBeyondEnd() @Test public void removeOneOfOne() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = + new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); Iterator iterator = context.iterator(); iterator.next(); @@ -122,11 +120,11 @@ public void removeOneOfOne() @Test public void removeOneOfTwo() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); result.addVersion( new TestVersion( "2.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = + new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); Iterator iterator = context.iterator(); iterator.next(); @@ -138,12 +136,11 @@ public void removeOneOfTwo() @Test public void removeOneOfThree() { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); VersionRangeResult result = new VersionRangeResult( new VersionRangeRequest() ); result.addVersion( new TestVersion( "1.0" ) ); result.addVersion( new TestVersion( "2.0" ) ); result.addVersion( new TestVersion( "3.0" ) ); - context.set( FOO_DEPENDENCY, result ); + DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession(), FOO_DEPENDENCY, result ); Iterator iterator = context.iterator(); iterator.next(); @@ -151,20 +148,4 @@ public void removeOneOfThree() assertEquals( Arrays.asList( new TestVersion( "2.0" ), new TestVersion( "3.0" ) ), context.get() ); } - - @Test - public void setTwice() - { - DefaultVersionFilterContext context = new DefaultVersionFilterContext( new DefaultRepositorySystemSession() ); - VersionRangeResult fooResult = new VersionRangeResult( new VersionRangeRequest() ); - fooResult.addVersion( new TestVersion( "1.0" ) ); - context.set( FOO_DEPENDENCY, fooResult ); - - VersionRangeResult barResult = new VersionRangeResult( new VersionRangeRequest() ); - barResult.addVersion( new TestVersion( "1.0" ) ); - barResult.addVersion( new TestVersion( "2.0" ) ); - context.set( BAR_DEPENDENCY, barResult ); - - assertEquals( Arrays.asList( new TestVersion( "1.0" ), new TestVersion( "2.0" ) ), context.get() ); - } } \ No newline at end of file From ff896811439c8921509ac4fbaf46b137a605f028 Mon Sep 17 00:00:00 2001 From: Ivan Babiankou Date: Sun, 12 Dec 2021 19:47:08 +0100 Subject: [PATCH 2/6] [MRESOLVER-7] Make DefaultDependencyCollectionContext immutable DefaultDependencyCollectionContext will be shared among the threads, so make it immutable to avoid side effects. --- .../DefaultDependencyCollectionContext.java | 16 +++++----------- .../impl/collect/DefaultDependencyCollector.java | 12 +++++------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectionContext.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectionContext.java index 3bf4fe1a0..310ec2209 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectionContext.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollectionContext.java @@ -19,6 +19,7 @@ * under the License. */ +import java.util.Collections; import java.util.List; import org.eclipse.aether.RepositorySystemSession; @@ -35,11 +36,11 @@ final class DefaultDependencyCollectionContext private final RepositorySystemSession session; - private Artifact artifact; + private final Artifact artifact; - private Dependency dependency; + private final Dependency dependency; - private List managedDependencies; + private final List managedDependencies; DefaultDependencyCollectionContext( RepositorySystemSession session, Artifact artifact, Dependency dependency, List managedDependencies ) @@ -47,7 +48,7 @@ final class DefaultDependencyCollectionContext this.session = session; this.artifact = ( dependency != null ) ? dependency.getArtifact() : artifact; this.dependency = dependency; - this.managedDependencies = managedDependencies; + this.managedDependencies = Collections.unmodifiableList( managedDependencies ); } public RepositorySystemSession getSession() @@ -70,13 +71,6 @@ public List getManagedDependencies() return managedDependencies; } - public void set( Dependency dependency, List managedDependencies ) - { - artifact = dependency.getArtifact(); - this.dependency = dependency; - this.managedDependencies = managedDependencies; - } - @Override public String toString() { diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index c78465814..104db2351 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -250,7 +250,7 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle DefaultDependencyCollectionContext context = new DefaultDependencyCollectionContext( session, request.getRootArtifact(), root, managedDependencies ); - Args args = new Args( session, trace, pool, nodes, context, request ); + Args args = new Args( session, trace, pool, nodes, request ); Results results = new Results( result, session ); process( args, results, dependencies, repositories, @@ -480,8 +480,9 @@ private void doRecurse( Args args, Results results, List repos DependencyTraverser depTraverser, VersionFilter verFilter, Dependency d, ArtifactDescriptorResult descriptorResult, DefaultDependencyNode child ) { - DefaultDependencyCollectionContext context = args.collectionContext; - context.set( d, descriptorResult.getManagedDependencies() ); + DefaultDependencyCollectionContext context = + new DefaultDependencyCollectionContext( args.session, d.getArtifact(), d, + descriptorResult.getManagedDependencies() ); DependencySelector childSelector = depSelector != null ? depSelector.deriveChildSelector( context ) : null; DependencyManager childManager = depManager != null ? depManager.deriveChildManager( context ) : null; @@ -698,12 +699,10 @@ static class Args final NodeStack nodes; - final DefaultDependencyCollectionContext collectionContext; - final CollectRequest request; Args( RepositorySystemSession session, RequestTrace trace, DataPool pool, NodeStack nodes, - DefaultDependencyCollectionContext collectionContext, CollectRequest request ) + CollectRequest request ) { this.session = session; this.request = request; @@ -712,7 +711,6 @@ static class Args this.trace = trace; this.pool = pool; this.nodes = nodes; - this.collectionContext = collectionContext; } } From 1006385b0b80a71a1af67600639fda52ca5a58e4 Mon Sep 17 00:00:00 2001 From: Ivan Babiankou Date: Sun, 12 Dec 2021 19:54:30 +0100 Subject: [PATCH 3/6] [MRESOLVER-7] Make NodeStack immutable NodeStack is used by multiple threads handling dependencies on the same level of the tree, making it immutable to avoid side effects. --- .../collect/DefaultDependencyCollector.java | 13 ++---- .../internal/impl/collect/NodeStack.java | 44 ++++++++----------- 2 files changed, 22 insertions(+), 35 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index 104db2351..99c7d58e8 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -244,13 +244,10 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle { DataPool pool = new DataPool( session ); - NodeStack nodes = new NodeStack(); - nodes.push( node ); - DefaultDependencyCollectionContext context = new DefaultDependencyCollectionContext( session, request.getRootArtifact(), root, managedDependencies ); - Args args = new Args( session, trace, pool, nodes, request ); + Args args = new Args( session, trace, pool, new NodeStack( node ), request ); Results results = new Results( result, session ); process( args, results, dependencies, repositories, @@ -502,13 +499,9 @@ private void doRecurse( Args args, Results results, List repos if ( children == null ) { args.pool.putChildren( key, child.getChildren() ); - - args.nodes.push( child ); - - process( args, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, + Args childArgs = new Args( args.session, args.trace, args.pool, args.nodes.push( child ), args.request ); + process( childArgs, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, childTraverser, childFilter ); - - args.nodes.pop(); } else { diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java index 668dbc4b0..66cc1067b 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/NodeStack.java @@ -30,45 +30,39 @@ final class NodeStack { - @SuppressWarnings( {"unchecked", "checkstyle:magicnumber" } ) - // CHECKSTYLE_OFF: MagicNumber - private DependencyNode[] nodes = new DependencyNode[96]; - // CHECKSTYLE_ON: MagicNumber + private final DependencyNode[] nodes; - private int size; - - public DependencyNode top() + NodeStack( DependencyNode node ) { - if ( size <= 0 ) - { - throw new IllegalStateException( "stack empty" ); - } - return nodes[size - 1]; + this( new DependencyNode[1] ); + nodes[0] = node; } - public void push( DependencyNode node ) + private NodeStack( DependencyNode[] nodes ) { - if ( size >= nodes.length ) - { - DependencyNode[] tmp = new DependencyNode[size + 64]; - System.arraycopy( nodes, 0, tmp, 0, nodes.length ); - nodes = tmp; - } - nodes[size++] = node; + this.nodes = nodes; } - public void pop() + public DependencyNode top() { - if ( size <= 0 ) + if ( nodes.length == 0 ) { throw new IllegalStateException( "stack empty" ); } - size--; + return nodes[nodes.length - 1]; + } + + public NodeStack push( DependencyNode node ) + { + DependencyNode[] copyNodes = new DependencyNode[nodes.length + 1]; + System.arraycopy( nodes, 0, copyNodes, 0, nodes.length ); + copyNodes[copyNodes.length - 1] = node; + return new NodeStack( copyNodes ); } public int find( Artifact artifact ) { - for ( int i = size - 1; i >= 0; i-- ) + for ( int i = nodes.length - 1; i >= 0; i-- ) { DependencyNode node = nodes[i]; @@ -110,7 +104,7 @@ public int find( Artifact artifact ) public int size() { - return size; + return nodes.length; } public DependencyNode get( int index ) From 571db8dac132fd2bd80ce7c8582b837e985b3333 Mon Sep 17 00:00:00 2001 From: Ivan Babiankou Date: Sun, 12 Dec 2021 20:03:36 +0100 Subject: [PATCH 4/6] [MRESOLVER-7] Use thread-safe variants of collections --- .../org/eclipse/aether/graph/DefaultDependencyNode.java | 8 ++++---- .../eclipse/aether/internal/impl/collect/DataPool.java | 6 +++--- .../eclipse/aether/internal/impl/collect/ObjectPool.java | 4 +++- .../internal/impl/collect/DefaultDependencyCycleTest.java | 3 +-- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java b/maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java index a2c1c8b0b..aa112a80a 100644 --- a/maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java +++ b/maven-resolver-api/src/main/java/org/eclipse/aether/graph/DefaultDependencyNode.java @@ -70,7 +70,7 @@ public DefaultDependencyNode( Dependency dependency ) { this.dependency = dependency; artifact = ( dependency != null ) ? dependency.getArtifact() : null; - children = new ArrayList<>( 0 ); + children = Collections.synchronizedList( new ArrayList( 0 ) ); aliases = Collections.emptyList(); relocations = Collections.emptyList(); repositories = Collections.emptyList(); @@ -88,7 +88,7 @@ public DefaultDependencyNode( Dependency dependency ) public DefaultDependencyNode( Artifact artifact ) { this.artifact = artifact; - children = new ArrayList<>( 0 ); + children = Collections.synchronizedList( new ArrayList( 0 ) ); aliases = Collections.emptyList(); relocations = Collections.emptyList(); repositories = Collections.emptyList(); @@ -106,7 +106,7 @@ public DefaultDependencyNode( DependencyNode node ) { dependency = node.getDependency(); artifact = node.getArtifact(); - children = new ArrayList<>( 0 ); + children = Collections.synchronizedList( new ArrayList( 0 ) ); setAliases( node.getAliases() ); setRequestContext( node.getRequestContext() ); setManagedBits( node.getManagedBits() ); @@ -127,7 +127,7 @@ public void setChildren( List children ) { if ( children == null ) { - this.children = new ArrayList<>( 0 ); + this.children = Collections.synchronizedList( new ArrayList( 0 ) ); } else { diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java index 4a145558d..7faf47619 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java @@ -21,12 +21,12 @@ import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.WeakHashMap; +import java.util.concurrent.ConcurrentHashMap; import org.eclipse.aether.RepositoryCache; import org.eclipse.aether.RepositorySystemSession; @@ -67,9 +67,9 @@ final class DataPool private Map descriptors; - private final Map constraints = new HashMap<>(); + private final Map constraints = new ConcurrentHashMap<>(); - private final Map> nodes = new HashMap<>( 256 ); + private final Map> nodes = new ConcurrentHashMap<>( 256 ); @SuppressWarnings( "unchecked" ) DataPool( RepositorySystemSession session ) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java index c4117eb02..f4df0a761 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/ObjectPool.java @@ -21,6 +21,7 @@ import java.lang.ref.Reference; import java.lang.ref.WeakReference; +import java.util.Collections; import java.util.Map; import java.util.WeakHashMap; @@ -31,7 +32,8 @@ class ObjectPool { - private final Map> objects = new WeakHashMap<>( 256 ); + private final Map> objects = + Collections.synchronizedMap( new WeakHashMap>( 256 ) ); public synchronized T intern( T object ) { diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCycleTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCycleTest.java index 53cddc22b..18bc7d580 100644 --- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCycleTest.java +++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCycleTest.java @@ -35,8 +35,7 @@ public class DefaultDependencyCycleTest @Test public void testToString() { - NodeStack nodeStack = new NodeStack(); - nodeStack.push( new DefaultDependencyNode( FOO_DEPENDENCY ) ); + NodeStack nodeStack = new NodeStack(new DefaultDependencyNode( FOO_DEPENDENCY )); DependencyCycle cycle = new DefaultDependencyCycle( nodeStack, 1, BAR_DEPENDENCY ); assertEquals( "group-id:foo:jar -> group-id:bar:jar", cycle.toString() ); From 487a2ad4ce7507af7269044884cebf375747070d Mon Sep 17 00:00:00 2001 From: Ivan Babiankou Date: Sun, 12 Dec 2021 20:09:45 +0100 Subject: [PATCH 5/6] [MRESOLVER-7] Process dependencies concurrently All child dependencies resolved asynchronously in the ExecutorService. --- .../collect/DefaultDependencyCollector.java | 91 ++++++++++++++----- 1 file changed, 68 insertions(+), 23 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index 99c7d58e8..998da800f 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -27,6 +27,13 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.locks.LockSupport; + import static java.util.Objects.requireNonNull; import javax.inject.Inject; @@ -247,14 +254,21 @@ public CollectResult collectDependencies( RepositorySystemSession session, Colle DefaultDependencyCollectionContext context = new DefaultDependencyCollectionContext( session, request.getRootArtifact(), root, managedDependencies ); - Args args = new Args( session, trace, pool, new NodeStack( node ), request ); + Args args = new Args( session, trace, pool, new NodeStack( node ), request, getExecutorService( session ) ); Results results = new Results( result, session ); - - process( args, results, dependencies, repositories, - depSelector != null ? depSelector.deriveChildSelector( context ) : null, - depManager != null ? depManager.deriveChildManager( context ) : null, - depTraverser != null ? depTraverser.deriveChildTraverser( context ) : null, - verFilter != null ? verFilter.deriveChildFilter( context ) : null ); + try + { + process( args, results, dependencies, repositories, + depSelector != null ? depSelector.deriveChildSelector( context ) : null, + depManager != null ? depManager.deriveChildManager( context ) : null, + depTraverser != null ? depTraverser.deriveChildTraverser( context ) : null, + verFilter != null ? verFilter.deriveChildFilter( context ) : null ); + waitUntilComplete( results.collectionTasks ); + } + finally + { + args.executorService.shutdown(); + } errorPath = results.errorPath; } @@ -338,27 +352,52 @@ private static String getId( Artifact a ) return a.getGroupId() + ':' + a.getArtifactId() + ':' + a.getClassifier() + ':' + a.getExtension(); } - @SuppressWarnings( "checkstyle:parameternumber" ) - private void process( final Args args, Results results, List dependencies, - List repositories, DependencySelector depSelector, - DependencyManager depManager, DependencyTraverser depTraverser, VersionFilter verFilter ) + private ExecutorService getExecutorService( RepositorySystemSession session ) + { + int nThreads = ConfigUtils.getInteger( session, 5, "maven.descriptor.threads", "maven.artifact.threads" ); + return Executors.newFixedThreadPool( nThreads ); + } + + private void waitUntilComplete( Iterable> tasks ) { - for ( Dependency dependency : dependencies ) + while ( anyIncomplete( tasks ) ) { - processDependency( args, results, repositories, depSelector, depManager, depTraverser, verFilter, - dependency ); + // CHECKSTYLE_OFF: MagicNumber + LockSupport.parkNanos( 50 ); + // CHECKSTYLE_ON: MagicNumber } } - @SuppressWarnings( "checkstyle:parameternumber" ) - private void processDependency( Args args, Results results, List repositories, - DependencySelector depSelector, DependencyManager depManager, - DependencyTraverser depTraverser, VersionFilter verFilter, Dependency dependency ) + private boolean anyIncomplete( Iterable> tasks ) { + for ( Future task : tasks ) + { + if ( !task.isDone() ) + { + return true; + } + } + return false; + } - List relocations = Collections.emptyList(); - processDependency( args, results, repositories, depSelector, depManager, depTraverser, verFilter, dependency, - relocations, false ); + @SuppressWarnings( "checkstyle:parameternumber" ) + private void process( final Args args, final Results results, final List dependencies, + final List repositories, final DependencySelector depSelector, + final DependencyManager depManager, final DependencyTraverser depTraverser, + final VersionFilter verFilter ) + { + for ( final Dependency dependency : dependencies ) + { + results.collectionTasks.add( args.executorService.submit( new Runnable() + { + @Override + public void run() + { + processDependency( args, results, repositories, depSelector, depManager, depTraverser, verFilter, + dependency, Collections.emptyList(), false ); + } + } ) ); + } } @SuppressWarnings( "checkstyle:parameternumber" ) @@ -499,7 +538,8 @@ private void doRecurse( Args args, Results results, List repos if ( children == null ) { args.pool.putChildren( key, child.getChildren() ); - Args childArgs = new Args( args.session, args.trace, args.pool, args.nodes.push( child ), args.request ); + Args childArgs = new Args( args.session, args.trace, args.pool, args.nodes.push( child ), args.request, + args.executorService ); process( childArgs, results, descriptorResult.getDependencies(), childRepos, childSelector, childManager, childTraverser, childFilter ); } @@ -694,8 +734,10 @@ static class Args final CollectRequest request; + final ExecutorService executorService; + Args( RepositorySystemSession session, RequestTrace trace, DataPool pool, NodeStack nodes, - CollectRequest request ) + CollectRequest request, ExecutorService executorService ) { this.session = session; this.request = request; @@ -704,6 +746,7 @@ static class Args this.trace = trace; this.pool = pool; this.nodes = nodes; + this.executorService = executorService; } } @@ -719,6 +762,8 @@ static class Results String errorPath; + final Queue> collectionTasks = new ConcurrentLinkedQueue<>(); + Results( CollectResult result, RepositorySystemSession session ) { this.result = result; From c889a50f628b148b5a9bd8a95e7cbf03177d16ee Mon Sep 17 00:00:00 2001 From: Ivan Babiankou Date: Sun, 26 Dec 2021 21:15:27 +0100 Subject: [PATCH 6/6] [MRESOLVER-7] Restore order of resolved child dependencies --- .../collect/DefaultDependencyCollector.java | 52 ++++++++++++++++++- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java index 998da800f..a8d2c4f37 100644 --- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java +++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -74,6 +75,7 @@ import org.eclipse.aether.spi.locator.Service; import org.eclipse.aether.spi.locator.ServiceLocator; import org.eclipse.aether.util.ConfigUtils; +import org.eclipse.aether.util.artifact.ArtifactIdUtils; import org.eclipse.aether.util.graph.manager.DependencyManagerUtils; import org.eclipse.aether.util.graph.transformer.TransformationContextKeys; import org.eclipse.aether.version.Version; @@ -386,9 +388,10 @@ private void process( final Args args, final Results results, final List> childrenTasks = new ArrayList<>(); for ( final Dependency dependency : dependencies ) { - results.collectionTasks.add( args.executorService.submit( new Runnable() + Future task = args.executorService.submit( new Runnable() { @Override public void run() @@ -396,7 +399,52 @@ public void run() processDependency( args, results, repositories, depSelector, depManager, depTraverser, verFilter, dependency, Collections.emptyList(), false ); } - } ) ); + } ); + results.collectionTasks.add( task ); + childrenTasks.add( task ); + } + Future sortingTask = args.executorService.submit( new Runnable() + { + @Override + public void run() + { + waitUntilComplete( childrenTasks ); + Collections.sort( args.nodes.top().getChildren(), new ChildrenComparator( dependencies ) ); + } + } ); + results.collectionTasks.add( sortingTask ); + } + + private static class ChildrenComparator implements Comparator + { + + private final Map order; + + private ChildrenComparator( List dependencies ) + { + this.order = new HashMap<>(); + for ( int i = 0; i < dependencies.size(); i++ ) + { + order.put( ArtifactIdUtils.toVersionlessId( dependencies.get( i ).getArtifact() ), i ); + } + } + + @Override + public int compare( DependencyNode o1, DependencyNode o2 ) + { + int result = getOrder( o1 ) - getOrder( o2 ); + if ( result != 0 ) + { + return result < 0 ? -1 : 1; + } + return 0; + } + + private Integer getOrder( DependencyNode node ) + { + String id = ArtifactIdUtils.toVersionlessId( node.getArtifact() ); + Integer result = order.get( id ); + return result == null ? 0 : result; } }