You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "sanjana2505006 (via GitHub)" <gi...@apache.org> on 2026/02/02 18:34:12 UTC
[PR] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
sanjana2505006 opened a new pull request, #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270
Hi! While reviewing the core analysis logic, I noticed a performance bottleneck in `DefaultProjectDependencyAnalyzer`. \n\nPreviously, it used a linear search to find which artifact a class belongs to ($O(N)$ for every class usage), which could be a bottleneck in large projects with many dependencies. These changes switch to a `Map`-based lookup ($O(1)$) and modernize the code using `computeIfAbsent` and more robust scope checks.\n\nThis significantly improves the efficiency of the analysis process for projects with large dependency trees.
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-4432634409
@sanjana2505006 rebased, and fix format
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "slachiewicz (via GitHub)" <gi...@apache.org>.
slachiewicz commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r3036712675
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -226,19 +230,14 @@ private static Set<Artifact> buildDeclaredArtifacts(MavenProject project) {
}
private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
- Map<Artifact, Set<String>> artifactClassMap, Set<DependencyUsage> dependencyClasses) {
+ Map<String, Artifact> classToArtifactMap, Set<DependencyUsage> dependencyClasses) {
Map<Artifact, Set<DependencyUsage>> usedArtifacts = new HashMap<>();
for (DependencyUsage classUsage : dependencyClasses) {
- Artifact artifact = findArtifactForClassName(artifactClassMap, classUsage.getDependencyClass());
+ Artifact artifact = classToArtifactMap.get(classUsage.getDependencyClass());
if (artifact != null && !includedInJDK(artifact)) {
- Set<DependencyUsage> classesFromArtifact = usedArtifacts.get(artifact);
- if (classesFromArtifact == null) {
- classesFromArtifact = new HashSet<>();
- usedArtifacts.put(artifact, classesFromArtifact);
- }
- classesFromArtifact.add(classUsage);
+ usedArtifacts.computeIfAbsent(artifact, k -> new HashSet<>()).add(classUsage);
}
}
Review Comment:
still no unit test
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-4433058393
> @slawekjaranowski Thank you for the help with the rebase and spotless fix! I’m tied up with my last exam until the 15th, but I'll check back in as soon as I'm done.
don't worry, take care about your exam
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-4299408979
> @sanjana2505006 pleas check build output
I have an exam shortly, but I'll make sure to look into the build output and resolve this as a priority once I'm done.
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r3036714341
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -43,7 +43,9 @@
import org.apache.maven.project.MavenProject;
/**
- * <p>DefaultProjectDependencyAnalyzer class.</p>
+ * <p>
+ * DefaultProjectDependencyAnalyzer class.
+ * </p>
Review Comment:
The class Javadoc formatting was changed to a multi-line `<p> ... </p>` block. In this module, other classes typically use the single-line `<p>Some text.</p>` style (e.g., `DefaultClassAnalyzer`). Consider reverting to the existing format for consistency across the codebase.
```suggestion
* <p>DefaultProjectDependencyAnalyzer class.</p>
```
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-4432665847
@slawekjaranowski Thank you for the help with the rebase and spotless fix! I’m tied up with my last exam until the 15th, but I'll check back in as soon as I'm done.
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-3951265116
Hello @slawekjaranowski, just a gentle follow-up on this PR. Whenever you have time, could you please share your feedback or let me know if any further changes are needed from my side? Happy to update anything required. Thank you
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r3003344776
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -226,19 +230,14 @@ private static Set<Artifact> buildDeclaredArtifacts(MavenProject project) {
}
private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
- Map<Artifact, Set<String>> artifactClassMap, Set<DependencyUsage> dependencyClasses) {
+ Map<String, Artifact> classToArtifactMap, Set<DependencyUsage> dependencyClasses) {
Map<Artifact, Set<DependencyUsage>> usedArtifacts = new HashMap<>();
for (DependencyUsage classUsage : dependencyClasses) {
- Artifact artifact = findArtifactForClassName(artifactClassMap, classUsage.getDependencyClass());
+ Artifact artifact = classToArtifactMap.get(classUsage.getDependencyClass());
if (artifact != null && !includedInJDK(artifact)) {
- Set<DependencyUsage> classesFromArtifact = usedArtifacts.get(artifact);
- if (classesFromArtifact == null) {
- classesFromArtifact = new HashSet<>();
- usedArtifacts.put(artifact, classesFromArtifact);
- }
- classesFromArtifact.add(classUsage);
+ usedArtifacts.computeIfAbsent(artifact, k -> new HashSet<>()).add(classUsage);
}
}
Review Comment:
That's a fair point. I'll add focused unit tests covering the new class-to-artifact resolution path, including duplicate-class scenarios, to ensure the chosen artifact is deterministic. Will update 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
Re: [PR] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r3003341066
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -260,13 +259,16 @@ private static boolean includedInJDK(Artifact artifact) {
return false;
}
- private static Artifact findArtifactForClassName(Map<Artifact, Set<String>> artifactClassMap, String className) {
+ private static Map<String, Artifact> buildClassToArtifactMap(Map<Artifact, Set<String>> artifactClassMap) {
+ Map<String, Artifact> classToArtifactMap = new HashMap<>();
+
for (Map.Entry<Artifact, Set<String>> entry : artifactClassMap.entrySet()) {
- if (entry.getValue().contains(className)) {
- return entry.getKey();
+ Artifact artifact = entry.getKey();
+ for (String className : entry.getValue()) {
+ classToArtifactMap.put(className, artifact);
Review Comment:
I've updated it to `putIfAbsent` in the latest commit to preserve the original first-match-wins semantics. Thank you for flagging this, @slawekjaranowski!
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r3036805834
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -226,19 +230,14 @@ private static Set<Artifact> buildDeclaredArtifacts(MavenProject project) {
}
private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
- Map<Artifact, Set<String>> artifactClassMap, Set<DependencyUsage> dependencyClasses) {
+ Map<String, Artifact> classToArtifactMap, Set<DependencyUsage> dependencyClasses) {
Map<Artifact, Set<DependencyUsage>> usedArtifacts = new HashMap<>();
for (DependencyUsage classUsage : dependencyClasses) {
- Artifact artifact = findArtifactForClassName(artifactClassMap, classUsage.getDependencyClass());
+ Artifact artifact = classToArtifactMap.get(classUsage.getDependencyClass());
if (artifact != null && !includedInJDK(artifact)) {
- Set<DependencyUsage> classesFromArtifact = usedArtifacts.get(artifact);
- if (classesFromArtifact == null) {
- classesFromArtifact = new HashSet<>();
- usedArtifacts.put(artifact, classesFromArtifact);
- }
- classesFromArtifact.add(classUsage);
+ usedArtifacts.computeIfAbsent(artifact, k -> new HashSet<>()).add(classUsage);
}
}
Review Comment:
I have just added comprehensive unit tests in `DefaultProjectDependencyAnalyzerTest.java` to cover the new class-to-artifact resolution path.
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r2997610370
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -260,13 +259,16 @@ private static boolean includedInJDK(Artifact artifact) {
return false;
}
- private static Artifact findArtifactForClassName(Map<Artifact, Set<String>> artifactClassMap, String className) {
+ private static Map<String, Artifact> buildClassToArtifactMap(Map<Artifact, Set<String>> artifactClassMap) {
+ Map<String, Artifact> classToArtifactMap = new HashMap<>();
+
for (Map.Entry<Artifact, Set<String>> entry : artifactClassMap.entrySet()) {
- if (entry.getValue().contains(className)) {
- return entry.getKey();
+ Artifact artifact = entry.getKey();
+ for (String className : entry.getValue()) {
+ classToArtifactMap.put(className, artifact);
Review Comment:
`buildClassToArtifactMap` overwrites any existing mapping when the same class name appears in multiple artifacts. Previously the lookup returned the first matching artifact encountered in `artifactClassMap` iteration order; with `put(...)` the last artifact wins, which can change results (e.g., shaded/duplicate classes) and cause incorrect used/unused dependency reporting. Preserve the previous behavior (e.g., `putIfAbsent` to keep the first mapping) or explicitly detect duplicates and handle/report them deterministically.
```suggestion
classToArtifactMap.putIfAbsent(className, artifact);
```
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -226,19 +230,14 @@ private static Set<Artifact> buildDeclaredArtifacts(MavenProject project) {
}
private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
- Map<Artifact, Set<String>> artifactClassMap, Set<DependencyUsage> dependencyClasses) {
+ Map<String, Artifact> classToArtifactMap, Set<DependencyUsage> dependencyClasses) {
Map<Artifact, Set<DependencyUsage>> usedArtifacts = new HashMap<>();
for (DependencyUsage classUsage : dependencyClasses) {
- Artifact artifact = findArtifactForClassName(artifactClassMap, classUsage.getDependencyClass());
+ Artifact artifact = classToArtifactMap.get(classUsage.getDependencyClass());
if (artifact != null && !includedInJDK(artifact)) {
- Set<DependencyUsage> classesFromArtifact = usedArtifacts.get(artifact);
- if (classesFromArtifact == null) {
- classesFromArtifact = new HashSet<>();
- usedArtifacts.put(artifact, classesFromArtifact);
- }
- classesFromArtifact.add(classUsage);
+ usedArtifacts.computeIfAbsent(artifact, k -> new HashSet<>()).add(classUsage);
}
}
Review Comment:
There are no unit tests covering the new class-to-artifact resolution path (including duplicate-class scenarios and ensuring the chosen artifact is deterministic). Adding focused tests around `buildUsedArtifacts`/`buildClassToArtifactMap` would help prevent regressions in dependency classification as this is core analysis logic.
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#discussion_r2997629755
##########
src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java:
##########
@@ -260,13 +259,16 @@ private static boolean includedInJDK(Artifact artifact) {
return false;
}
- private static Artifact findArtifactForClassName(Map<Artifact, Set<String>> artifactClassMap, String className) {
+ private static Map<String, Artifact> buildClassToArtifactMap(Map<Artifact, Set<String>> artifactClassMap) {
+ Map<String, Artifact> classToArtifactMap = new HashMap<>();
+
for (Map.Entry<Artifact, Set<String>> entry : artifactClassMap.entrySet()) {
- if (entry.getValue().contains(className)) {
- return entry.getKey();
+ Artifact artifact = entry.getKey();
+ for (String className : entry.getValue()) {
+ classToArtifactMap.put(className, artifact);
Review Comment:
@sanjana2505006 please look for this one
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-4298067541
@sanjana2505006 pleas check build output
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-3836982676
Hello @slawekjaranowski, I’ve opened a PR with a small improvement I noticed.
Whenever you have time, I’d appreciate your thoughts on whether this approach makes sense.
Thank you!
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "sanjana2505006 (via GitHub)" <gi...@apache.org>.
sanjana2505006 commented on PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270#issuecomment-4187642839
Whenever you get a free moment, I would love to hear your thoughts on this. Let me know if anything needs to be updated!
--
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] Optimize artifact lookup in DefaultProjectDependencyAnalyzer [maven-dependency-analyzer]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski merged PR #270:
URL: https://github.com/apache/maven-dependency-analyzer/pull/270
--
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