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 2024/07/13 15:42:47 UTC

[PR] [MRESOLVER-587] Memory usage improvements [maven-resolver]

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

   Do not use Artifact instance as keys, and intern the List<Dependency> on artifact descriptors as well.
   
   ---
   
   https://issues.apache.org/jira/browse/MRESOLVER-587


-- 
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


Re: [PR] [MRESOLVER-587] Memory usage improvements [maven-resolver]

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

   @gsmet this PR now has it all merged changes, plus, introduces config (defaults to "as before" -- false both) to intern or not intern the artifact descriptor deps list and managedDeps lists.
   
   Am tinkering, that maybe feasible default would be false/true (do not intern deps, do intern managedDeps)?


-- 
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


Re: [PR] [MRESOLVER-587] Memory usage improvements [maven-resolver]

Posted by "gsmet (via GitHub)" <gi...@apache.org>.
gsmet commented on code in PR #536:
URL: https://github.com/apache/maven-resolver/pull/536#discussion_r1701742710


##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java:
##########
@@ -257,7 +257,7 @@ public static final class DescriptorKey {
 
         private DescriptorKey(Artifact artifact) {
             this.artifact = artifact;
-            this.hashCode = buildHashCode();
+            this.hashCode = Objects.hashCode(artifact);

Review Comment:
   Ah yeah sorry, I was used to more complex hashCode construction and kept this pattern (we avoid using `Objects.hash` as it allocates an array and can be problematic in some cases so we end up with traditional boilerplate hashCodes).



-- 
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


Re: [PR] [MRESOLVER-587] Memory usage improvements [maven-resolver]

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

   This PR causes noticeable slow down: building Quarkus (mvn clean install -Dquickly) went from 9:20 to 11:10.
   
   Ideas:
   * make interning of dependencies configurable, default NO
   * make interning of dependencyManagement configurable, default YES
   * maybe intern only over some size limit?
   
   The most offender is huge count of depMgt inherited from parent POM, so this above would make sensible defaults.


-- 
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


Re: [PR] [MRESOLVER-587] Memory usage improvements [maven-resolver]

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

   > Am tinkering, that maybe feasible default would be false/true (do not intern deps, do intern managedDeps)?
   
   It looks like a good compromise from my side.


-- 
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


Re: [PR] [MRESOLVER-587] Memory usage improvements [maven-resolver]

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


-- 
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