[MRESOLVER-7] Download dependency POMs in parallel v2 #10

slachiewicz · 2017-11-25T11:16:59Z

Rebased version of@hwellmann pull #2

I do not know if it's faster now, but I managed to compile the Maven project successfully and the it-core-tests completed correctly.

michael-o

There changes need to be separate issues.

michael-o

This should also be a separate issue.

slachiewicz · 2017-11-29T20:21:09Z

@michael-o what changes i should move? all 3 commits?

michael-o · 2017-11-29T20:37:23Z

The subpackage and version bumps.

slachiewicz · 2017-11-30T20:07:38Z

ok, I updated this PR to contain only MRESOLVER-7 and created a new #11 for dependencies update and #12 to move the dependency collector to submodule (#10 must be tested/merged first)

mikehearn · 2018-04-07T16:28:19Z

It looks like this has bit-rotted again. It'd be really nice to get this in.

Some classes have moved into a sub-package in #12, so now package-private visibilities are getting in the way. I wanted to try it out and see how much difference to speed it made, so I just quickly undid #12.

For me it seems to roughly double the speed of artifact fetch. It'll probably make a much bigger difference on higher latency links. Nice work!

…allel dep resolution

michael-o · 2018-04-08T11:34:50Z

@mikehearn What needs to be changed to make classes accesible for this PR? I'll change it.

slachiewicz · 2018-04-08T11:40:34Z

strange, but You requested to move #11 to diffrent PR. So now You need to revert #11 then you can apply again this PR..

michael-o · 2018-04-08T11:49:25Z

@slachiewicz I maybe wrote gibberish, I am working on five projects in parallel and simply lost track. Don't take my request as imperative.

slachiewicz · 2018-04-08T11:56:55Z

and I’m just trying to help:)

steve-todorov · 2018-06-30T18:35:54Z

Hi guys, are there any plans for this to be merged anytime soon? We have a bit project which might benefit from this. Is there anything more needed to be done?

michael-o · 2018-06-30T21:06:38Z

@steve-todorov Did you try the patch yourself? Unfortunately, there is a very little amount of people working on Resolver right now. I am personally too busy. There is so much new code that I have trouble to understand it, especially because I am not fully comfortable with the Resolver code. But I am still looking into it.

@slachiewicz Can you rebase again?

slachiewicz · 2018-07-01T00:10:29Z

I did rebase code and I compile few projects with clean repo - it looks good and fast.
@steve-todorov could you try to test with Your projects also?

mikehearn · 2018-07-04T13:31:49Z

For what it's worth I've been testing it locally a fair bit and it's been working fine. However I had to fork Maven Resolver and add a few commits on top to get it working as well as I wanted it to:

https://github.com/mikehearn/maven-resolver/commits/master

Specifically, I had to roll back a change that conflicted with this PR, and then I added this commit:

mikehearn@1e2ed26

to avoid creating non-daemon threads.

slachiewicz · 2018-07-05T21:08:17Z

Thx @mikehearn for tests and suggestion to use cachedThreadPool. I made small modification to use 5 core threads and max to 10 thread. Could you test new version?

michael-o · 2018-07-22T21:38:05Z

Can someone tell why the daemon threads along with the cached pool is necessary?

slachiewicz · 2018-07-30T22:09:03Z

ok, finaly i reuse in f195414 existing code used to create threads (org.eclipse.aether.util.concurrency.WorkerThreadFactory) in org.eclipse.aether.internal.impl.DefaultMetadataResolver#getExecutor

michael-o · 2018-09-17T21:27:40Z

Is this one now complete and can be reviewed again by me?

slachiewicz · 2018-09-17T21:28:53Z

yes, it’s final version :)

michael-o · 2018-09-25T19:16:22Z

I will drop cf1a1f7 because this is a different issue.

michael-o

Why are you passing DefaultDependencyCollectionContext around where you have an interface for? The callers should be decoupled form the implementations.

michael-o · 2018-09-25T19:24:21Z

...-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java

+    /**
+     * Setup executor with 5 daemon threads in pool, max to 10 concurrent and keep alive to 10 sec inactivity.
+     */
+    private ExecutorService executor = createExecutorService( 5, 10, 10 );


Last arg is 10L.

michael-o · 2018-09-25T19:25:25Z

...-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DefaultDependencyCollector.java

+    private static final ThreadGroup THREAD_GROUP = new ThreadGroup( "Maven Resolver dependency resolution" );
+
+    /**
+     * Setup executor with 5 daemon threads in pool, max to 10 concurrent and keep alive to 10 sec inactivity.


10 sec to > 10 s.

michael-o · 2018-09-25T19:45:11Z

Maven master fails with:

[INFO] Running org.apache.maven.repository.MirrorProcessorTest
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 s - in org.apache.maven.repository.MirrorProcessorTest
[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR]   ProjectInheritanceTest.testDependencyManagement:68 Set size should be 4, is 3
[INFO]
[ERROR] Tests run: 142, Failures: 1, Errors: 0, Skipped: 0

Please review.

michael-o · 2018-09-27T17:59:59Z

@slachiewicz I'd like to publish MRESOLVER in a couple of days. Can you have a look at the PR again please?

Dependencies are now processed asynchronously by an Executor Cherry-pick commit from PR#2 Nov 1, 2016

Logic aligned with DefaultMetadataResolver and BasicRepositoryConnector Executors

slachiewicz · 2018-09-29T12:36:42Z

That use of DDCC is hard to remove without extending resolver-api and its jhard to understand how this work. I’m able to reproduce this bug but no idea about reason. I’ll look into this more later.

slachiewicz · 2018-10-03T00:34:09Z

Testcase:
org.apache.maven.project.inheritance.t06.ProjectInheritanceTest#testDependencyManagement
and poms in maven-compat/src/test/resources/inheritance-repo/t06

Expected:
Artifact: maven-test:t06-a:jar 1.0 scope=compile optional=false
Artifact: maven-test:t06-b:jar 1.0 scope=compile optional=false
Artifact: maven-test:t06-d:jar 1.0 scope=test optional=false
Artifact: maven-test:t06-c:jar 1.0 scope=compile optional=false

Actual:
Artifact: maven-test:t06-a:jar 1.0 scope=compile optional=false
Artifact: maven-test:t06-b:jar 1.0 scope=compile optional=false
Artifact: maven-test:t06-c:jar 1.0 scope=compile optional=false

Can be related to https://issues.apache.org/jira/browse/MRESOLVER-9

michael-o · 2018-10-03T10:12:58Z

While I understand MRESOLVER-9 I do not understand this failure. How can this happen if we don't have any changes in dep resolution, do we? Do I need to merge MRESOLVER-9 first?

michael-o · 2018-10-10T20:13:38Z

@slachiewicz, do you want to take another look at it. We want to roll 1.3.1 next week. I'd be cool if we could include this. My last questions are still open.

michael-o · 2018-12-26T12:45:13Z

@slachiewicz, do you think you can take on this? You know the change best by now.

slachiewicz · 2018-12-30T11:34:17Z

Unfortunately not for this release - I need more time to isolate a problem.

michael-o · 2018-12-30T14:28:16Z

No issue, take your time.

gdubicki · 2019-01-30T21:39:34Z

Hi @slachiewicz ! Were you able to get back to this? A lot of people would love to get this PR merged, see https://stackoverflow.com/questions/32299902/parallel-downloads-of-maven-artifacts/43832592 . :)

ghost · 2019-02-09T17:43:54Z

Hi @slachiewicz, I'm very interested in this PR, what is blocking it ? What can I do to help ?

michael-o · 2019-02-10T07:21:15Z

@juliendemaziere. Time issues. You can work on it if you like, I'll be happy to review it.

hwellmann · 2019-03-23T15:47:24Z

FYI, I've looked at this issue again after all this time, and I've got it working now locally, including the entire maven-integration-testing suite.

It looks like the missing bit is indeed related to https://issues.apache.org/jira/browse/MRESOLVER-9. I'm going to submit a new PR based on this one.

Thanks to @slachiewicz and everyone else who kept the candle burning!

michael-o · 2019-03-23T16:12:17Z

@hwellmann, what exact missing bit?

hwellmann · 2019-03-23T19:19:25Z

The missing bit: see this commit 884928f in the new PR #30.

michael-o · 2019-03-24T08:46:03Z

@hwellmann, why is #30 necessary for this PR?

hwellmann · 2019-03-24T11:07:24Z

It fixes the failing test org.apache.maven.project.inheritance.t06.ProjectInheritanceTest observed by @slachiewicz in October 2018 as well as some failing tests from maven-integration-testing when using a build of Maven with this verion of Maven Resolver.

michael-o · 2019-03-30T18:18:01Z

Can this one be closed in favor of #30?

slachiewicz · 2019-03-31T18:02:51Z

Yes, of course.

jira-importer · 2025-06-13T08:03:20Z

Resolve #817

jira-importer · 2025-06-13T08:27:00Z

Resolve #1397

jira-importer · 2025-06-13T08:31:43Z

Resolve #817

slachiewicz changed the title ~~Feature/mresolver 7v2~~ [MRESOLVER-7] Download dependency POMs in parallel v2 Nov 25, 2017

michael-o reviewed Nov 29, 2017

View reviewed changes

slachiewicz mentioned this pull request Jan 14, 2018

[MRESOLVER-35] maven-resolver-impl move dependency collector and friends to subpackage #12

Closed

mikehearn added a commit to mikehearn/maven-resolver that referenced this pull request Apr 7, 2018

TEMP: Apply PR apache#10 and undo apache#11 from upstream, to get par…

e688237

…allel dep resolution

michael-o reviewed Sep 25, 2018

View reviewed changes

michael-o requested changes Sep 25, 2018

View reviewed changes

hwellmann and others added 2 commits September 28, 2018 21:13

[MRESOLVER-7] Download dependency POMs in parallel

8425128

Dependencies are now processed asynchronously by an Executor Cherry-pick commit from PR#2 Nov 1, 2016

[MRESOLVER-7] Use WorkerThreadFactory to create daemon threads

d418152

Logic aligned with DefaultMetadataResolver and BasicRepositoryConnector Executors

gdubicki mentioned this pull request Dec 28, 2018

[MRESOLVER-7] Download dependency POMs in parallel #2

Closed

hwellmann mentioned this pull request Mar 23, 2019

Fixes MRESOLVER-7, third try #30

Closed

slachiewicz closed this Mar 31, 2019

slachiewicz deleted the feature/MRESOLVER-7v2 branch April 23, 2019 21:11

rmannibucau mentioned this pull request Jan 17, 2024

[MRESOLVER-464] Workaround for JDK-822647 bug #408

Merged

[MRESOLVER-7] Download dependency POMs in parallel v2 #10

[MRESOLVER-7] Download dependency POMs in parallel v2 #10

Uh oh!

Conversation

slachiewicz commented Nov 25, 2017

Uh oh!

michael-o left a comment

Choose a reason for hiding this comment

Uh oh!

michael-o left a comment

Choose a reason for hiding this comment

Uh oh!

slachiewicz commented Nov 29, 2017

Uh oh!

michael-o commented Nov 29, 2017

Uh oh!

slachiewicz commented Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikehearn commented Apr 7, 2018

Uh oh!

michael-o commented Apr 8, 2018

Uh oh!

slachiewicz commented Apr 8, 2018

Uh oh!

michael-o commented Apr 8, 2018

Uh oh!

slachiewicz commented Apr 8, 2018

Uh oh!

steve-todorov commented Jun 30, 2018

Uh oh!

michael-o commented Jun 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slachiewicz commented Jul 1, 2018

Uh oh!

mikehearn commented Jul 4, 2018

Uh oh!

slachiewicz commented Jul 5, 2018

Uh oh!

michael-o commented Jul 22, 2018

Uh oh!

slachiewicz commented Jul 30, 2018

Uh oh!

michael-o commented Sep 17, 2018

Uh oh!

slachiewicz commented Sep 17, 2018

Uh oh!

michael-o commented Sep 25, 2018

Uh oh!

michael-o left a comment

Choose a reason for hiding this comment

Uh oh!

michael-o Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

michael-o Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

michael-o commented Sep 25, 2018

Uh oh!

michael-o commented Sep 27, 2018

Uh oh!

slachiewicz commented Sep 29, 2018

Uh oh!

slachiewicz commented Oct 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michael-o commented Oct 3, 2018

Uh oh!

michael-o commented Oct 10, 2018

Uh oh!

michael-o commented Dec 26, 2018

Uh oh!

slachiewicz commented Dec 30, 2018

Uh oh!

michael-o commented Dec 30, 2018

Uh oh!

gdubicki commented Jan 30, 2019

Uh oh!

slachiewicz commented Nov 30, 2017 •

edited

Loading

michael-o commented Jun 30, 2018 •

edited

Loading

slachiewicz commented Oct 3, 2018 •

edited

Loading