maven-resolver
maven-resolver copied to clipboard
apache
[MRESOLVER-7] download poms in parallel
This PR is for parallel POM downloading, the idea is a bit different than https://github.com/ibabiankou/maven-resolver/pull/2/files:
- only parallelize the VersionRange and ArtifactDesciptor resolution part (poms.xml & maven-metadata.xml downloading in parallel) as the poms downloading or maven-metadata.xml is the most slow part.
- Pure dependency resolution logic and the skip logic are still run in sequence.
@michael-o @cstamas @ibabiankou
I think the code is already ready for review, I would like to run more tests before converting the PR from Draft to final. Please help review and share your comments with this PR.
@michael-o
Created separate PR for moving the PremanagedDependency class. https://github.com/apache/maven-resolver/pull/179
Separated the PR for PremanagedDependency refactoring and rebased the code.
Something is wrong here, unsure what. But, I tried to build maven master using maven-3.9.x + this PR + empty local repo and it fails:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:testCompile (default-testCompile) on project maven-model-builder: Compilation failure: Compilation failure:
[ERROR] /home/cstamas/Worx/apache-maven/maven/maven-model-builder/src/test/java/org/apache/maven/model/interpolation/StringSearchModelInterpolatorTest.java:[36,27] package org.hamcrest does not exist
[ERROR] /home/cstamas/Worx/apache-maven/maven/maven-model-builder/src/test/java/org/apache/maven/model/interpolation/StringSearchModelInterpolatorTest.java:[36,1] static import only from classes and interfaces
[ERROR] /home/cstamas/Worx/apache-maven/maven/maven-model-builder/src/test/java/org/apache/maven/model/interpolation/StringSearchModelInterpolatorTest.java:[37,27] package org.hamcrest does not exist
[ERROR] /home/cstamas/Worx/apache-maven/maven/maven-model-builder/src/test/java/org/apache/maven/model/interpolation/StringSearchModelInterpolatorTest.java:[37,1] static import only from classes and interfaces
[ERROR] -> [Help 1]
The cmd used to test:
[cstamas@infinity maven (maven-3.9.x)]$ ~/bin/maven/apache-maven-3.9.0-SNAPSHOT/bin/mvn clean install -Dmaven.repo.local=local-repo -Drat.skip -DskipTests -Daether.collector.impl=bf
@michael-o @cstamas
I have converted the PR from draft to ready. Please help review.
The basic idea of this PR is:
- Given dependency A's children B, C, D
- Iterate B, C, D one by one, take B as example below.
- Should B be filtered? If yes, won't add to the queue. Logic here
- If B should not be filtered, then figure out the managed version of B, ex B1, resolve B1's descriptor in a thread, then add B1 to the queue. Logic here
In such way, poms and metadata.xml will be downloaded in parallel.
The issue @cstamas raised has been fixed. The failure was caused incorrect filtering (should filter with original dependency, B instead of B1 as described in above flow)
Also runs the ITs with both DF and BF (code based on this PR) mode, same test result below: 2 tests that are not related with this PR failed in both modes. Not sure if this is as expected.
[ERROR] Failures: [ERROR] MavenITmng5576CdFriendlyVersions>AbstractMavenIntegrationTestCase.runTest:260->testContinuousDeliveryFriendlyVersionsAreWarningFreeWithBuildConsumer:98 [WARNING] [ERROR] MavenITmng5576CdFriendlyVersions>AbstractMavenIntegrationTestCase.runTest:260->testContinuousDeliveryFriendlyVersionsAreWarningFreeWithoutBuildConsumer:67 [WARNING] [INFO] [ERROR] Tests run: 912, Failures: 2, Errors: 0, Skipped: 0
Also dry-run 1000+ apps in our company by comparing the maven dependency tree with DF and BF mode (code based on this PR), no issues found so far. Test method like this:
- mvn clean install -DskipTests -Daether.collector.impl=bf -Dmaven.repo.local=bf
- mvn dependency:tree -DoutputFile=bf-tree.txt -Daether.collector.impl=bf -Dmaven.repo.local=bf
- mvn clean install -DskipTests -Dmaven.repo.local=df
- mvn dependency:tree -DoutputFile=df-tree.txt -Dmaven.repo.local=df
- compare bf-tree.txt df-tree.txt
Please help review and feel free to let me know if anything I can help.
@michael-o @cstamas Folks, please help review the PR and let me know if anything needs from me. This PR can make poms/metadata.xml download much faster.
Will likely happen either this Friday or in 10 days. I think this should not be in Maven 3.9.0 just to avoid any breakings.
Any updates on this PR? Can we expect it to be merged soon?
I'm a contributor of JDT Language Server, and one of our core component m2e is depending on maven-resolver. We've been observing that downloading poms one by one takes a lot of time when importing Maven projects. To be honest, it's one of our performance bottleneck and we can hardly improve it on our own, in downstream... So it would be great if this improvement can be shipped in coming releases.
Any updates on this PR? Can we expect it to be merged soon?
I'm a contributor of JDT Language Server, and one of our core component m2e is depending on maven-resolver. We've been observing that downloading poms one by one takes a lot of time when importing Maven projects. To be honest, it's one of our performance bottleneck and we can hardly improve it on our own, in downstream... So it would be great if this improvement can be shipped in coming releases.
Friends, the PR is under review. Feel free to participate into the contribution. :)
Any updates on this PR? Can we expect it to be merged soon?
I'm a contributor of JDT Language Server, and one of our core component m2e is depending on maven-resolver. We've been observing that downloading poms one by one takes a lot of time when importing Maven projects. To be honest, it's one of our performance bottleneck and we can hardly improve it on our own, in downstream... So it would be great if this improvement can be shipped in coming releases.
Proper evaluation I'd mandatory, we already had severe regressions due to the previous approach.
Proper evaluation I'd mandatory
I understand and appreciate all the work you guys have done. If I remember clearly, some integration tests and details were discussed in a private ASF slack channel which I had no access...
Just want to know if there's any ETA (already one month since last update)? Anyway, please let me know if there's anything I can help (if that would help to proceed this).
Proper evaluation I'd mandatory
I understand and appreciate all the work you guys have done. If I remember clearly, some integration tests and details were discussed in a private ASF slack channel which I had no access...
Just want to know if there's any ETA (already one month since last update)? Anyway, please let me know if there's anything I can help (if that would help to proceed this).
@cstamas we need to discuss this sometime this month.
Proper evaluation I'd mandatory
I understand and appreciate all the work you guys have done. If I remember clearly, some integration tests and details were discussed in a private ASF slack channel which I had no access... Just want to know if there's any ETA (already one month since last update)? Anyway, please let me know if there's anything I can help (if that would help to proceed this).
@cstamas we need to discuss this sometime this month.
@michael-o @cstamas
May I know the latest status of discussion? Please never hesitate to let me know if anything I can help or the code I should revise.
@michael-o How's everything? Got a chance to give more comments to this PR? And may I know what's the remaining steps to get this PR merged?
@michael-o can you please specify what changes are requested with your "change request" PR review? It is unclear to me. As for me, this PR is good to merge (and probably improve it later re shared executor, but is def out of scope for this PR)