You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/08/28 13:23:40 UTC
(maven) branch master updated: chore: optimize reactor sort, model pool, and phase comparator performance (#12652)
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new b613cea9f3 chore: optimize reactor sort, model pool, and phase comparator performance (#12652)
b613cea9f3 is described below
commit b613cea9f37243cf21f044b32749a4694431af0f
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Fri Aug 28 15:23:25 2026 +0200
chore: optimize reactor sort, model pool, and phase comparator performance (#12652)
JFR profiling of a 4,383-module reactor revealed three hotspots that
together consume ~35% of CPU time during dependency resolution:
1. DefaultGraphBuilder: result.sort(comparing(sortedProjects::indexOf))
uses O(n) ArrayList.indexOf per comparison, causing O(n² log n) total
MavenProject.equals calls (~230M for 4,383 modules). Replace with a
HashMap<MavenProject, Integer> index built in O(n), reducing sort
comparisons to O(1) each. Affects 3 call sites.
2. DefaultModelObjectPool.getPooledTypes(): re-parses a comma-separated
property string into a new Stream → Set on every process() call.
Cache the parsed Set at construction time. Also add hashCode
fast-rejection to PoolKey.equals() to skip expensive deep equality
when hashes differ.
3. DefaultModelObjectPool.PoolKey.dependencyHashCode(): Objects.hash()
with 12 arguments allocates a new Object[12] on every call. Inline
the hash computation to eliminate the varargs allocation.
4. PhaseComparator: List.indexOf() in compare() is O(n) per call.
Pre-build a HashMap<String, Integer> in the constructor for O(1)
phase index lookups.
Co-authored-by: Claude Opus 4.6 <no...@anthropic.com>
---
.../apache/maven/graph/DefaultGraphBuilder.java | 21 +++++++---
.../maven/lifecycle/internal/PhaseComparator.java | 22 ++++++----
.../maven/impl/model/DefaultModelObjectPool.java | 47 +++++++++++++---------
3 files changed, 57 insertions(+), 33 deletions(-)
diff --git a/impl/maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
index 5f20f38e59..4c2e72e335 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/graph/DefaultGraphBuilder.java
@@ -157,8 +157,7 @@ private List<MavenProject> trimProjectsToRequest(
if (request.getPom() != null) {
result = getProjectsInRequestScope(request, activeProjects);
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort(comparing(sortedProjects::indexOf));
+ result.sort(comparing(buildProjectIndexMap(graph.getSortedProjects())::get));
result = includeAlsoMakeTransitively(result, request, graph);
}
@@ -189,8 +188,7 @@ private List<MavenProject> trimSelectedProjects(
result = includeAlsoMakeTransitively(result, request, graph);
// Order the new list in the original order
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort(comparing(sortedProjects::indexOf));
+ result.sort(comparing(buildProjectIndexMap(graph.getSortedProjects())::get));
}
}
@@ -290,13 +288,24 @@ private List<MavenProject> includeAlsoMakeTransitively(
result = new ArrayList<>(projectsSet);
// Order the new list in the original order
- List<MavenProject> sortedProjects = graph.getSortedProjects();
- result.sort(comparing(sortedProjects::indexOf));
+ result.sort(comparing(buildProjectIndexMap(graph.getSortedProjects())::get));
}
return result;
}
+ /**
+ * Builds a Map from MavenProject to its index in the sorted list, enabling O(1) index lookups
+ * for sorting instead of O(n) ArrayList.indexOf() scans that cause O(n² log n) sort performance.
+ */
+ private static Map<MavenProject, Integer> buildProjectIndexMap(List<MavenProject> sortedProjects) {
+ Map<MavenProject, Integer> indexMap = new HashMap<>(sortedProjects.size() * 2);
+ for (int i = 0; i < sortedProjects.size(); i++) {
+ indexMap.put(sortedProjects.get(i), i);
+ }
+ return indexMap;
+ }
+
private void enrichRequestFromResumptionData(List<MavenProject> projects, MavenExecutionRequest request) {
if (request.isResume()) {
projects.stream()
diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseComparator.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseComparator.java
index 2a1efccfbf..575c70ab03 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseComparator.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/PhaseComparator.java
@@ -19,16 +19,19 @@
package org.apache.maven.lifecycle.internal;
import java.util.Comparator;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
/**
* Compares phases within the context of a specific lifecycle with secondary sorting based on the {@link PhaseId}.
*/
public class PhaseComparator implements Comparator<String> {
/**
- * The lifecycle phase ordering.
+ * Map from phase name to its index in the lifecycle, enabling O(1) lookups
+ * instead of O(n) List.indexOf() scans on every comparison.
*/
- private final List<String> lifecyclePhases;
+ private final Map<String, Integer> phaseIndexMap;
/**
* Constructor.
@@ -36,24 +39,27 @@ public class PhaseComparator implements Comparator<String> {
* @param lifecyclePhases the lifecycle phase ordering.
*/
public PhaseComparator(List<String> lifecyclePhases) {
- this.lifecyclePhases = lifecyclePhases;
+ this.phaseIndexMap = new HashMap<>(lifecyclePhases.size() * 2);
+ for (int i = 0; i < lifecyclePhases.size(); i++) {
+ phaseIndexMap.put(lifecyclePhases.get(i), i);
+ }
}
@Override
public int compare(String o1, String o2) {
PhaseId p1 = PhaseId.of(o1);
PhaseId p2 = PhaseId.of(o2);
- int i1 = lifecyclePhases.indexOf(p1.executionPoint().prefix() + p1.phase());
- int i2 = lifecyclePhases.indexOf(p2.executionPoint().prefix() + p2.phase());
- if (i1 == -1 && i2 == -1) {
+ Integer i1 = phaseIndexMap.get(p1.executionPoint().prefix() + p1.phase());
+ Integer i2 = phaseIndexMap.get(p2.executionPoint().prefix() + p2.phase());
+ if (i1 == null && i2 == null) {
// unknown phases, leave in existing order
return 0;
}
- if (i1 == -1) {
+ if (i1 == null) {
// second one is known, so it comes first
return 1;
}
- if (i2 == -1) {
+ if (i2 == null) {
// first one is known, so it comes first
return -1;
}
diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java
index bec34fe0fa..6295ae92f3 100644
--- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java
+++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelObjectPool.java
@@ -60,6 +60,7 @@ public class DefaultModelObjectPool implements ModelObjectProcessor {
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultModelObjectPool.class);
private final Map<?, ?> properties;
+ private final Set<String> pooledTypes;
public DefaultModelObjectPool() {
this(System.getProperties());
@@ -67,6 +68,7 @@ public DefaultModelObjectPool() {
DefaultModelObjectPool(Map<?, ?> properties) {
this.properties = properties;
+ this.pooledTypes = parsePooledTypes(properties);
}
@Override
@@ -79,8 +81,8 @@ public <T> T process(T object) {
Class<?> objectType = object.getClass();
String simpleClassName = objectType.getSimpleName();
- // Check if this object type should be pooled (read configuration dynamically)
- if (!getPooledTypes(properties).contains(simpleClassName)) {
+ // Check if this object type should be pooled
+ if (!pooledTypes.contains(simpleClassName)) {
return object;
}
@@ -96,14 +98,16 @@ private String getProperty(String name, String defaultValue) {
}
/**
- * Gets the set of object types that should be pooled.
+ * Parses the set of object types that should be pooled from properties.
+ * Called once at construction time to avoid re-parsing on every {@link #process} call.
*/
- private Set<String> getPooledTypes(Map<?, ?> properties) {
- String pooledTypesProperty = getProperty(Constants.MAVEN_MODEL_PROCESSOR_POOLED_TYPES, "Dependency");
+ private static Set<String> parsePooledTypes(Map<?, ?> properties) {
+ Object value = properties.get(Constants.MAVEN_MODEL_PROCESSOR_POOLED_TYPES);
+ String pooledTypesProperty = value instanceof String str ? str : "Dependency";
return Arrays.stream(pooledTypesProperty.split(","))
.map(String::trim)
.filter(s -> !s.isEmpty())
- .collect(Collectors.toSet());
+ .collect(Collectors.toUnmodifiableSet());
}
/**
@@ -199,6 +203,9 @@ public boolean equals(Object obj) {
if (!(obj instanceof PoolKey other)) {
return false;
}
+ if (hashCode != other.hashCode) {
+ return false;
+ }
return objectsEqual(object, other.object);
}
@@ -283,21 +290,23 @@ private static int computeHashCode(Object obj) {
/**
* Custom hash code for Dependency objects based on all fields.
+ * Inlined to avoid the Object[] varargs allocation from Objects.hash().
*/
private static int dependencyHashCode(org.apache.maven.api.model.Dependency dep) {
- return Objects.hash(
- dep.getGroupId(),
- dep.getArtifactId(),
- dep.getVersion(),
- dep.getType(),
- dep.getClassifier(),
- dep.getScope(),
- dep.getSystemPath(),
- dep.getExclusions(),
- dep.getOptional(),
- dep.getLocationKeys(),
- locationsHashCode(dep),
- dep.getImportedFrom());
+ int h = 1;
+ h = 31 * h + Objects.hashCode(dep.getGroupId());
+ h = 31 * h + Objects.hashCode(dep.getArtifactId());
+ h = 31 * h + Objects.hashCode(dep.getVersion());
+ h = 31 * h + Objects.hashCode(dep.getType());
+ h = 31 * h + Objects.hashCode(dep.getClassifier());
+ h = 31 * h + Objects.hashCode(dep.getScope());
+ h = 31 * h + Objects.hashCode(dep.getSystemPath());
+ h = 31 * h + Objects.hashCode(dep.getExclusions());
+ h = 31 * h + Objects.hashCode(dep.getOptional());
+ h = 31 * h + Objects.hashCode(dep.getLocationKeys());
+ h = 31 * h + locationsHashCode(dep);
+ h = 31 * h + Objects.hashCode(dep.getImportedFrom());
+ return h;
}
/**