You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2023/02/14 17:12:24 UTC

[GitHub] [maven-resolver] cstamas opened a new pull request, #243: [MRESOLVER-320] Return to original pool setup

cstamas opened a new pull request, #243:
URL: https://github.com/apache/maven-resolver/pull/243

   There was some information lost on the way how we got to current master: artifact and dependency was ALWAYS weak, and the "cause" commit 6dda69980cf4c13228a4f6561bfa0d8384a0be68 modified ONLY the descriptor pool (was hard by mistake, made it weak). This also explain why we saw with yourkit excess Xpp3 and model building requests as well (as starting with 1.8.x descriptors/POMs were GCed, while before they were not).
   
   This PR essentially returns the state of affairs to pre 1.8.x resolver state re data pools, but also adds ability to "tune" each three of them separately.
   
   ---
   
   https://issues.apache.org/jira/browse/MRESOLVER-320


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas merged pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas merged PR #243:
URL: https://github.com/apache/maven-resolver/pull/243


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431128447

   My results with new baseline, as I decided to move "closer" to Petr case (I was building master that is moving target):
   ```
   Project (tag): https://github.com/quarkusio/quarkus/releases/tag/2.16.1.Final
   OS: Linux Fedora 37 WS (6.1.11-200.fc37.x86_64)
   Java: Temurin-17.0.6+10
   No MAVEN_OPTS present.
   Cmd: /usr/bin/time -f "%e sec -- %M -- %I -- %O" mvn validate -Dversion.enforcer.plugin=3.2.1 -T8 (no settings.xml)
   
   3.8.7:                     25.92 sec -- 3957056 -- 0 -- 624
   
   3.9.0:                     32.96 sec -- 2282404 -- 0 -- 624
   
   3.9.1patch:
   weak+weak+hard (default):  26.70 sec -- 3967968 -- 0 -- 624
   hard+hard+hard:            26.12 sec -- 3839500 -- 0 -- 624
   weak+weak+weak:           218.16 sec -- 2510952 -- 0 -- 720 HIT same NPE as Petr!!!
   none+none+none:           242.61 sec -- 2426684 -- 0 -- 624
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] psiroky commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431301929

   Here are some more numbers with the latest fixes in this PR:
   (Quarkus 2.16.1.Final, Linux x64, JDK 19.0.2, `MAVEN_OPTS="-XX:+UseParallelGC -Xms5g -Xmx5g"`)
   
   | Cmd   |      3.8.7      | 3.9.0 |3.9.1patch |
   |----------|---------------- |---------|---------|
   validate -Dversion.enforcer.plugin=3.2.1 | 29s | 36s | 30s| 
   validate -Dversion.enforcer.plugin=3.2.1 -T8 | 9s | 11s | 9s|


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431336532

   Updated PR description that will become commit message of squashed PR to reflect all the changes here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431324107

   > While this looks fine, what about the NPE?
   
   Fix for NPE (caused by "misbehaving" weak InternPool) is also here in commit 202134e7e2aecd1aed10397c63675d6ed0a21cef. Simply put, original (already merged) weak pool MAY return null, where it never should do that (did not check for WeakReference value nullability).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1430206693

   @psiroky ping, and would be nice if you could get some numbers tomorrow as well


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] michael-o commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1430167679

   What do numbers say now?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1430462282

   Def sus, will look tomorrow. Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431132845

   So some conclusions or proposals:
   * remove "none", it makes no sense? (time growth is huge but memory is not less than with 3.9.0)
   * seems "descriptor" weak is culprit of NPE Petr and I saw, will look into it more 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431183238

   The weak pool had a problem, fixed here 202134e7e2aecd1aed10397c63675d6ed0a21cef


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1430195139

   My licensed YK is on desktop linux, so will have them tomorrow. But all in all, I think my original patch got a bit "carried away", and this really returns things how they were supposed to be after all... IMHO, this is safe to merge, as not only this returns "original state", but also makes possible to tune all. At the end, we may want to change some defaults, but that's it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] psiroky commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "psiroky (via GitHub)" <gi...@apache.org>.
psiroky commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1430456033

   I will run some more tests tomorrow. So far I just tried to run the `validate -Dversion.enforcer.plugin=3.2.1 -T8` (on Quarkus again), which seems to be almost on par with Maven 3.8.7 (9s vs 9.3s, and vs 11s for Maven 3.9.0).
   
   However, I also got this exception in one of the runs
   ```
   Caused by: java.lang.NullPointerException: Cannot invoke "org.eclipse.aether.graph.Dependency.getArtifact()" because "dependency" is null
       at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollectionContext.set (DefaultDependencyCollectionContext.java:75)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.doRecurse (DfDependencyCollector.java:261)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.processDependency (DfDependencyCollector.java:236)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.processDependency (DfDependencyCollector.java:137)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.process (DfDependencyCollector.java:125)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.doRecurse (DfDependencyCollector.java:284)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.processDependency (DfDependencyCollector.java:236)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.processDependency (DfDependencyCollector.java:137)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.process (DfDependencyCollector.java:125)
       at org.eclipse.aether.internal.impl.collect.df.DfDependencyCollector.doCollectDependencies (DfDependencyCollector.java:107)
       at org.eclipse.aether.internal.impl.collect.DependencyCollectorDelegate.collectDependencies (DependencyCollectorDelegate.java:247)
       at org.eclipse.aether.internal.impl.collect.DefaultDependencyCollector.collectDependencies (DefaultDependencyCollector.java:95)
       at org.eclipse.aether.internal.impl.DefaultRepositorySystem.collectDependencies (DefaultRepositorySystem.java:327)
       at org.apache.maven.enforcer.rules.dependency.ResolveUtil.resolveTransitiveDependencies (ResolveUtil.java:101)
       at org.apache.maven.enforcer.rules.dependency.DependencyConvergence.execute (DependencyConvergence.java:65)
       at org.apache.maven.plugins.enforcer.EnforceMojo.executeRuleNew (EnforceMojo.java:351)
       at org.apache.maven.plugins.enforcer.EnforceMojo.executeRule (EnforceMojo.java:325)
       at org.apache.maven.plugins.enforcer.EnforceMojo.execute (EnforceMojo.java:248)
       at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
       at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:342)
       at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:330)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:213)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:175)
       at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:76)
       at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:163)
       at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
       at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:160)
       at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
       at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:193)
       at org.apache.maven.lifecycle.internal.builder.multithreaded.MultiThreadedBuilder$1.call (MultiThreadedBuilder.java:180)
       at java.util.concurrent.FutureTask.run (FutureTask.java:317)
       at java.util.concurrent.Executors$RunnableAdapter.call (Executors.java:577)
       at java.util.concurrent.FutureTask.run (FutureTask.java:317)
       at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1144)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:642)
       at java.lang.Thread.run (Thread.java:1589)
   
   ```
   
   I am not sure if this is related to these changes, but it seemed suspicious as it is coming from the `DfDependencyCollector`, so figured I would mention it here. It is quite rare, I only got this error once in 100 runs (so maybe this there all the time and I was just "lucky" to never encounter it). In any case, it seems like something we should eventually take a look at.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431207367

   With fix/commit 202134e7e2aecd1aed10397c63675d6ed0a21cef (weak pool misbehave) new numbers:
   ```
   3.9.1patch:
   weak+weak+hard (default):  24.41 sec -- 2018196 -- 0 -- 624
   hard+hard+hard:            23.68 sec -- 2000336 -- 0 -- 624
   weak+weak+weak:            35.43 sec -- 1719268 -- 0 -- 624
   none+none+none:           242.61 sec -- 2426684 -- 0 -- 624
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-resolver] cstamas commented on pull request #243: [MRESOLVER-320] Return to original pool setup

Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #243:
URL: https://github.com/apache/maven-resolver/pull/243#issuecomment-1431230534

   The "none" pool removed, as it makes no sense. The users can "play" with "hard" (heap aggressive) or "weak" (GC friendly) pools.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org