Yougoss commented on PR #768:
URL: https://github.com/apache/maven/pull/768#issuecomment-1208283097

   
   
   
   > The idea of maintaining pool of immutable objects is good, however the 
implementation proposed isn't: the JVM is free to cleanup dependencyCache as 
soon as execution leaves #toDependency method - there are no more references to 
depCacheKeykey. The author is observing some performance improvements just 
because his JVM has enough free memory to not trigger GC whilst maven builds 
dependency graph (give JVM more memory to save memory).
   
   Hi @andreybpanfilov, glad to see your comments, I think there may be some 
misunderstanding for this change.
   
   So first, I need to declare that the main change I want to make should be in 
ArtifactDescriptorReaderDelegate(maven-resolver-provider), but I noticed that 
there is same code in  RepositoryUtils(maven-core), maybe for some historic 
reason, so I just made a same change here to keep them consistent. 
   
   For the toDependency method in RepositoryUtils only collect 
dependencyManagement in project pom, as you side, the change here doesn't bring 
too much increasement.(Entrance: getDependencies:243, 
LifecycleDependencyResolver (org.apache.maven.lifecycle.internal))
   
   But for the convert method in ArtifactDescriptorReaderDelegate is totally 
different. It's used for calculating all the dependency relationship. It will 
travels all the dependencies in the project pom, and collect dependency 
manageMent for each dependency. 
   Image one case, there are two dependencies in the project. And in each 
dependency pom, there are two dependencyManagement, so there should be 4 
dependency instance will be created to save the information of the 
dependencyManagement.
   Image a case which is a little complicated, if the dependenyManagement in 
above two dependencise are one same parent pom, and there are 1000 dependencies 
managed in this parent pom. There would be 1000 * 2 = 2000 instance will be 
created for dependencyManagement, although there is only 1000 after remove 
duplicate ones.
   So for our real case in ebay project, there is about 3000 dependencies with 
a parent pom which managed all these 3000 dependencies version. So when 
building the project, there will be 3000 * 3000 = 9,000,000 instance will be 
created for dependencyManagement. But after removing duplicate, there are only 
3000 are necessary. That's the change here why I use a hashmap the cache all 
the dependencyManagement instance.
   
   And why I use a weakHashMap here is I want to make sure when the dependency 
instance are not refered by graph anymore it can be cleanup by GC, won't stay 
in memory since the reference from hashMap key.
   
   And for the Memory cost you mentioned, I have also shared a build comparison 
for our project with command 'export MAVEN_OPTS="-Xms1g -Xmx1g”' to limit the 
memory  in jira ticket. With the PR, it can build successfully, but with 
current maven code, it will throw OOM exception.
   
   Please let me know if I should share some fake project to simulate this case 
to make you more clear
   
    
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to