From 2515c3af36a9958236595fa25b22bc2fb84f5b60 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 31 Jul 2026 23:52:34 +0200 Subject: [PATCH 1/6] chore: optimize reactor sort, model pool, and phase comparator performance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 in the constructor for O(1) phase index lookups. Co-Authored-By: Claude Opus 4.6 --- .../maven/graph/DefaultGraphBuilder.java | 21 ++++++--- .../lifecycle/internal/PhaseComparator.java | 22 +++++---- .../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 5f20f38e594c..4c2e72e33541 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 trimProjectsToRequest( if (request.getPom() != null) { result = getProjectsInRequestScope(request, activeProjects); - List 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 trimSelectedProjects( result = includeAlsoMakeTransitively(result, request, graph); // Order the new list in the original order - List sortedProjects = graph.getSortedProjects(); - result.sort(comparing(sortedProjects::indexOf)); + result.sort(comparing(buildProjectIndexMap(graph.getSortedProjects())::get)); } } @@ -290,13 +288,24 @@ private List includeAlsoMakeTransitively( result = new ArrayList<>(projectsSet); // Order the new list in the original order - List 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 buildProjectIndexMap(List sortedProjects) { + Map 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 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 2a1efccfbfbf..575c70ab03c7 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 { /** - * 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 lifecyclePhases; + private final Map phaseIndexMap; /** * Constructor. @@ -36,24 +39,27 @@ public class PhaseComparator implements Comparator { * @param lifecyclePhases the lifecycle phase ordering. */ public PhaseComparator(List 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 bec34fe0fa76..6295ae92f3ee 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 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 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 getPooledTypes(Map properties) { - String pooledTypesProperty = getProperty(Constants.MAVEN_MODEL_PROCESSOR_POOLED_TYPES, "Dependency"); + private static Set 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; } /** From 16757521f9866addb59d0ae2e5bb8ee977f4a26c Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 01:51:23 +0200 Subject: [PATCH 2/6] Optimize model building pipeline: defer Dependency.build() and reduce allocations Reduce CPU and memory overhead in Maven 4's immutable model building pipeline by deferring Dependency.build() across pipeline stages and optimizing hot paths in model object pooling. Key changes: - Add Builder getters to generated model classes (model.vm) enabling field access without materializing immutable objects - Add *ToBuilder merger variants (merger.vm) that return Builder instead of calling build(), letting callers accumulate changes across stages - Defer build() in DependencyManagementInjector to batch-build only modified dependencies at the end of the merge loop - Replace Stream.concat().collect() with HashMap.putAll() in computeLocations() and precompute locations hash code to eliminate repeated map iteration during pooling - Optimize PoolKey.locationsEqual() to use direct map comparison with fast-path for empty maps and hash-based inequality check - Add addLocationInformation API to XmlReaderRequest for future use in skipping location tracking on imported BOMs Co-Authored-By: Claude Opus 4.6 --- .../api/services/xml/XmlReaderRequest.java | 33 ++++++++- .../building/FileToRawModelMergerTest.java | 4 ++ .../maven/impl/DefaultModelXmlFactory.java | 1 + .../DefaultDependencyManagementImporter.java | 2 + .../DefaultDependencyManagementInjector.java | 24 ++++--- .../impl/model/DefaultModelObjectPool.java | 41 ++++------- src/mdo/merger.vm | 11 +++ src/mdo/model.vm | 71 +++++++++++++++++-- 8 files changed, 146 insertions(+), 41 deletions(-) diff --git a/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java b/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java index 41733eb08bf3..07fd999b9f8b 100644 --- a/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java +++ b/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java @@ -66,6 +66,20 @@ public interface XmlReaderRequest { boolean isAddDefaultEntities(); + /** + * Indicates whether location information (line/column tracking) should be + * recorded during parsing. Defaults to {@code true}. Setting this to + * {@code false} for imported dependency management POMs avoids allocating + * location maps that are never read, significantly reducing memory churn + * in large reactors. + * + * @return {@code true} if location information should be tracked + * @since 4.0.0 + */ + default boolean isAddLocationInformation() { + return true; + } + interface Transformer { /** * Interpolate the value read from the xml document @@ -95,6 +109,7 @@ class XmlReaderRequestBuilder { String modelId; String location; boolean addDefaultEntities = true; + boolean addLocationInformation = true; public XmlReaderRequestBuilder path(Path path) { this.path = path; @@ -146,6 +161,11 @@ public XmlReaderRequestBuilder addDefaultEntities(boolean addDefaultEntities) { return this; } + public XmlReaderRequestBuilder addLocationInformation(boolean addLocationInformation) { + this.addLocationInformation = addLocationInformation; + return this; + } + public XmlReaderRequest build() { return new DefaultXmlReaderRequest( path, @@ -157,7 +177,8 @@ public XmlReaderRequest build() { strict, modelId, location, - addDefaultEntities); + addDefaultEntities, + addLocationInformation); } private static class DefaultXmlReaderRequest implements XmlReaderRequest { @@ -171,6 +192,7 @@ private static class DefaultXmlReaderRequest implements XmlReaderRequest { final String modelId; final String location; final boolean addDefaultEntities; + final boolean addLocationInformation; @SuppressWarnings("checkstyle:ParameterNumber") DefaultXmlReaderRequest( @@ -183,7 +205,8 @@ private static class DefaultXmlReaderRequest implements XmlReaderRequest { boolean strict, String modelId, String location, - boolean addDefaultEntities) { + boolean addDefaultEntities, + boolean addLocationInformation) { this.path = path; this.rootDirectory = rootDirectory; this.url = url; @@ -194,6 +217,7 @@ private static class DefaultXmlReaderRequest implements XmlReaderRequest { this.modelId = modelId; this.location = location; this.addDefaultEntities = addDefaultEntities; + this.addLocationInformation = addLocationInformation; } @Override @@ -245,6 +269,11 @@ public String getLocation() { public boolean isAddDefaultEntities() { return addDefaultEntities; } + + @Override + public boolean isAddLocationInformation() { + return addLocationInformation; + } } } } diff --git a/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java b/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java index 640579cb8871..b5b785c7cb7f 100644 --- a/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java +++ b/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileToRawModelMergerTest.java @@ -40,6 +40,10 @@ class FileToRawModelMergerTest { void testOverriddenMergeMethods() { List methodNames = Stream.of(MavenMerger.class.getDeclaredMethods()) .filter(m -> m.getName().startsWith("merge")) + // Exclude *ToBuilder variants and void methods whose first parameter + // is a Builder — only the object-returning merge methods need overriding + .filter(m -> !m.getName().endsWith("ToBuilder")) + .filter(m -> !m.getParameterTypes()[0].getSimpleName().equals("Builder")) .filter(m -> { String baseName = m.getName().substring(5 /* merge */); String entity = baseName.substring(baseName.indexOf('_') + 1); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java index 575d35e230d8..c87ce5034315 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java @@ -132,6 +132,7 @@ private Model doRead(XmlReaderRequest request) throws XmlReaderException { ? new MavenStaxReader(request.getTransformer()::transform) : new MavenStaxReader(); xml.setAddDefaultEntities(request.isAddDefaultEntities()); + xml.setAddLocationInformation(request.isAddLocationInformation()); if (inputStream != null) { return xml.read(inputStream, request.isStrict(), source); } else if (reader != null) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java index 45de07f83a3c..828f8568980c 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementImporter.java @@ -167,6 +167,8 @@ static Dependency updateWithImportedFrom(Dependency dependency, DependencyManage if (dependencySource == null || bomSource == null || Objects.equals(dependencySource.getModelId(), bomSource.getModelId())) { + // Use forceCopy=true since we only set importedFrom (no field changes that would + // trigger copy-on-write), and build immediately as we need the immutable result. return Dependency.newBuilder(dependency, true) .importedFrom(bomLocation) .build(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java index d5c7d5d3e1b1..1ca7e2bcbed9 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java @@ -58,33 +58,39 @@ protected static class ManagementModelMerger extends MavenModelMerger { public Model mergeManagedDependencies(Model model) { DependencyManagement dependencyManagement = model.getDependencyManagement(); if (dependencyManagement != null) { - Map dependencies = new HashMap<>(); + // Use Builders to accumulate changes across all managed dependencies, + // deferring build() until after all merges are complete + Map originalDeps = new HashMap<>(); + Map builderDeps = new HashMap<>(); Map context = Collections.emptyMap(); for (Dependency dependency : model.getDependencies()) { Object key = getDependencyKey().apply(dependency); - dependencies.put(key, dependency); + originalDeps.put(key, dependency); } boolean modified = false; for (Dependency managedDependency : dependencyManagement.getDependencies()) { Object key = getDependencyKey().apply(managedDependency); - Dependency dependency = dependencies.get(key); + Dependency dependency = originalDeps.get(key); if (dependency != null) { - Dependency merged = mergeDependency(dependency, managedDependency, false, context); - if (merged != dependency) { - dependencies.put(key, merged); + Dependency.Builder merged = + mergeDependencyToBuilder(dependency, managedDependency, false, context); + // Only track modifications if the builder actually changed something + if (merged != null) { + builderDeps.put(key, merged); modified = true; } } } if (modified) { - List newDeps = new ArrayList<>(dependencies.size()); + List newDeps = new ArrayList<>(originalDeps.size()); for (Dependency dep : model.getDependencies()) { Object key = getDependencyKey().apply(dep); - Dependency dependency = dependencies.get(key); - newDeps.add(dependency); + Dependency.Builder builder = builderDeps.get(key); + // Only build() the dependencies that were actually merged + newDeps.add(builder != null ? builder.build() : dep); } return Model.newBuilder(model).dependencies(newDeps).build(); } 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 6295ae92f3ee..2509dc5a4d64 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 @@ -253,29 +253,29 @@ private static boolean dependenciesEqual( && Objects.equals(dep1.getSystemPath(), dep2.getSystemPath()) && Objects.equals(dep1.getExclusions(), dep2.getExclusions()) && Objects.equals(dep1.getOptional(), dep2.getOptional()) - && Objects.equals(dep1.getLocationKeys(), dep2.getLocationKeys()) && locationsEqual(dep1, dep2) && Objects.equals(dep1.getImportedFrom(), dep2.getImportedFrom()); } /** * Compare locations maps for two dependencies. + * Uses the direct getLocations() map instead of iterating through + * individual keys to avoid KeyValueHolder allocation overhead. + * Short-circuits on empty maps (common for imported dependencies). */ private static boolean locationsEqual( org.apache.maven.api.model.Dependency dep1, org.apache.maven.api.model.Dependency dep2) { - var keys1 = dep1.getLocationKeys(); - var keys2 = dep2.getLocationKeys(); - - if (!Objects.equals(keys1, keys2)) { - return false; + var locs1 = dep1.getLocations(); + var locs2 = dep2.getLocations(); + // Fast path: both empty (common for imported deps with location tracking off) + if (locs1.isEmpty() && locs2.isEmpty()) { + return true; } - - for (Object key : keys1) { - if (!Objects.equals(dep1.getLocation(key), dep2.getLocation(key))) { - return false; - } + // Use precomputed hash code for fast inequality check + if (dep1.getLocationsHashCode() != dep2.getLocationsHashCode()) { + return false; } - return true; + return locs1.equals(locs2); } /** @@ -291,6 +291,8 @@ 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(). + * Uses the precomputed locations hash code to avoid re-iterating + * the locations map entries. */ private static int dependencyHashCode(org.apache.maven.api.model.Dependency dep) { int h = 1; @@ -303,23 +305,10 @@ private static int dependencyHashCode(org.apache.maven.api.model.Dependency dep) 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 + dep.getLocationsHashCode(); h = 31 * h + Objects.hashCode(dep.getImportedFrom()); return h; } - - /** - * Compute hash code for locations map. - */ - private static int locationsHashCode(org.apache.maven.api.model.Dependency dep) { - int hash = 1; - for (Object key : dep.getLocationKeys()) { - hash = 31 * hash + Objects.hashCode(key); - hash = 31 * hash + Objects.hashCode(dep.getLocation(key)); - } - return hash; - } } /** diff --git a/src/mdo/merger.vm b/src/mdo/merger.vm index 6724b09742de..2ad2e20685d4 100644 --- a/src/mdo/merger.vm +++ b/src/mdo/merger.vm @@ -101,6 +101,17 @@ public class ${className} { return builder.build(); } + /** + * Merges the source into a builder based on the target, returning the Builder + * without calling build(). This allows callers to defer the build() call and + * avoid intermediate immutable object allocations in multi-stage pipelines. + */ + protected ${class.name}.Builder merge${class.name}ToBuilder(${class.name} target, ${class.name} source, boolean sourceDominant, Map context) { + ${class.name}.Builder builder = ${class.name}.newBuilder(target); + merge${class.name}(builder, target, source, sourceDominant, context); + return builder; + } + protected void merge${class.name}(${class.name}.Builder builder, ${class.name} target, ${class.name} source, boolean sourceDominant, Map context) { #if ( $class.superClass ) merge${class.superClass}(builder, target, source, sourceDominant, context); diff --git a/src/mdo/model.vm b/src/mdo/model.vm index 481ee0b4891a..ed42df9bcf59 100644 --- a/src/mdo/model.vm +++ b/src/mdo/model.vm @@ -151,6 +151,8 @@ public class ${class.name} #if ( $locationTracking && ! $class.superClass ) /** Locations */ final Map locations; + /** Cached hash code for the locations map, precomputed at build time */ + final int locationsHashCode; /** Location tracking */ final InputLocation importedFrom; #end @@ -180,6 +182,7 @@ public class ${class.name} #end #if ( $locationTracking && ! $class.superClass ) this.locations = builder.computeLocations(); + this.locationsHashCode = this.locations.hashCode(); this.importedFrom = builder.importedFrom; #end } @@ -264,6 +267,26 @@ public class ${class.name} return locations.keySet().stream(); } + /** + * Gets the locations map. Provides direct access to avoid individual key lookups + * when comparing or hashing all locations at once. + * + * @return an unmodifiable map of locations, never {@code null} + */ + public Map getLocations() { + return locations; + } + + /** + * Gets the precomputed hash code for the locations map. + * This avoids re-iterating the map entries during pooling/interning operations. + * + * @return the cached hash code of the locations map + */ + public int getLocationsHashCode() { + return locationsHashCode; + } + /** * Gets the input location that caused this model to be read. */ @@ -474,6 +497,33 @@ public class ${class.name} return this; } + #end + #foreach ( $field in $allFields ) + #set ( $cap = $Helper.capitalise( $field.name ) ) + #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + ## Builder stores List fields as Collection — match that type for the getter + #if ( $type.startsWith("List<") ) + #set ( $type = ${type.replace('List<','Collection<')} ) + #end + #if ( $type == "boolean" || $type == "Boolean" ) + #set ( $pfx = "is" ) + #else + #set ( $pfx = "get" ) + #end + #if ( $type == "boolean" ) + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); + } + #elseif ( $type == "int" ) + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); + } + #else + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : null); + } + #end + #end #if ( $locationTracking ) @@ -494,6 +544,17 @@ public class ${class.name} return this; } + public InputLocation getLocation(Object key) { + if (locations != null && locations.containsKey(key)) { + return locations.get(key); + } + return base != null ? base.getLocation(key) : null; + } + + public InputLocation getImportedFrom() { + return importedFrom != null ? importedFrom : (base != null ? base.getImportedFrom() : null); + } + #end @Nonnull public ${class.name} build() { @@ -518,14 +579,16 @@ public class ${class.name} Map newlocs = locations != null ? locations : Map.of(); Map oldlocs = base != null ? base.locations : Map.of(); if (newlocs.isEmpty()) { - return Map.copyOf(oldlocs); + return oldlocs; } if (oldlocs.isEmpty()) { return Map.copyOf(newlocs); } - return Stream.concat(newlocs.entrySet().stream(), oldlocs.entrySet().stream()) - // Keep value from newlocs in case of duplicates - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue, (v1, v2) -> v1)); + // Use HashMap.putAll instead of Stream.concat().collect() to avoid + // Stream allocation and intermediate Map.Entry iteration overhead + HashMap merged = new HashMap<>(oldlocs); + merged.putAll(newlocs); // newlocs entries override oldlocs (same semantics as before) + return Map.copyOf(merged); } #end } From 5e74cd4261a9ae6780d5444bd58349981f56c450 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 03:09:01 +0200 Subject: [PATCH 3/6] Store mutable builders in Builder list fields and thread Model.Builder through pipeline Extend the model code generation (model.vm) so that Builder classes store Collection instead of Collection for model-class list fields. This enables accumulating changes across pipeline stages without intermediate build() calls. Key changes: - model.vm: Builder fields for model-class lists now use child builders. Backward-compatible setter wraps immutable objects into builders. Added getModifiable*() methods for lazy base-list wrapping. Added reset(T base) method to replace builder state in-place. Short-circuit optimization: skip build when no fields are set. - Pipeline stage interfaces (10 interfaces): added default builder-accepting methods that bridge to the existing Model-accepting implementations. Fully backward compatible for existing implementations. - DefaultModelBuilder: buildEffectiveModel() and readEffectiveModel() now thread a Model.Builder between stages instead of rebuilding at each step. - Hot stage implementations: overrode builder-accepting methods in DefaultModelNormalizer, DefaultDependencyManagementInjector, DefaultPluginManagementInjector, DefaultModelPathTranslator, and DefaultPluginConfigurationExpander to write directly to the passed builder, avoiding redundant newBuilder() allocations and intermediate build() calls. Co-Authored-By: Claude Opus 4.6 --- .../model/DependencyManagementImporter.java | 17 ++ .../model/DependencyManagementInjector.java | 20 +++ .../services/model/InheritanceAssembler.java | 14 ++ .../api/services/model/ModelInterpolator.java | 17 ++ .../api/services/model/ModelNormalizer.java | 27 +++ .../services/model/ModelPathTranslator.java | 13 ++ .../services/model/ModelUrlNormalizer.java | 13 ++ .../model/PluginConfigurationExpander.java | 14 ++ .../model/PluginManagementInjector.java | 13 ++ .../api/services/model/ProfileInjector.java | 17 ++ .../DefaultPluginConfigurationExpander.java | 22 +++ .../DefaultDependencyManagementInjector.java | 27 ++- .../maven/impl/model/DefaultModelBuilder.java | 67 +++++--- .../impl/model/DefaultModelNormalizer.java | 56 +++++++ .../model/DefaultModelPathTranslator.java | 32 +++- .../DefaultPluginManagementInjector.java | 15 ++ src/mdo/model.vm | 156 +++++++++++++++++- 17 files changed, 502 insertions(+), 38 deletions(-) diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java index cdcb385a7b5d..118ba1ac1dbd 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementImporter.java @@ -45,4 +45,21 @@ Model importManagement( List sources, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #importManagement}. + * + * @since 4.0.0 + */ + default void importManagement( + Model.Builder builder, + List sources, + ModelBuilderRequest request, + ModelProblemCollector problems) { + Model built = builder.build(); + Model result = importManagement(built, sources, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java index fb91b232cf60..fc29dc218e58 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/DependencyManagementInjector.java @@ -38,4 +38,24 @@ public interface DependencyManagementInjector { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant that operates on a {@link Model.Builder} directly, + * avoiding an intermediate {@code Model.build()} between pipeline stages. + *

+ * The default implementation bridges to {@link #injectManagement(Model, ModelBuilderRequest, ModelProblemCollector)} + * by building the model, processing it, and resetting the builder to the result. + * + * @param builder The model builder to modify in place, must not be {@code null}. + * @param request The model building request, must not be {@code null}. + * @param problems The container used to collect problems, must not be {@code null}. + * @since 4.0.0 + */ + default void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectManagement(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java index 59b24f370d04..3fdb1d3a6e81 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/InheritanceAssembler.java @@ -42,4 +42,18 @@ public interface InheritanceAssembler { */ Model assembleModelInheritance( Model child, Model parent, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant that merges parent values into the child builder directly. + * + * @since 4.0.0 + */ + default void assembleModelInheritance( + Model.Builder childBuilder, Model parent, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = childBuilder.build(); + Model result = assembleModelInheritance(built, parent, request, problems); + if (result != built) { + childBuilder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java index c24a8a4d7f1e..4d678b21b4d3 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelInterpolator.java @@ -51,4 +51,21 @@ Model interpolateModel( @Nullable Path projectDir, @Nonnull ModelBuilderRequest request, @Nonnull ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #interpolateModel}. + * + * @since 4.0.0 + */ + default void interpolateModel( + @Nonnull Model.Builder builder, + @Nullable Path projectDir, + @Nonnull ModelBuilderRequest request, + @Nonnull ModelProblemCollector problems) { + Model built = builder.build(); + Model result = interpolateModel(built, projectDir, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java index 50225fd417a5..a8b19a4cd9cb 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelNormalizer.java @@ -48,4 +48,31 @@ public interface ModelNormalizer { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model injectDefaultValues(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #mergeDuplicates}. + * + * @since 4.0.0 + */ + default void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = mergeDuplicates(built, request, problems); + if (result != built) { + builder.reset(result); + } + } + + /** + * Builder-accepting variant of {@link #injectDefaultValues}. + * + * @since 4.0.0 + */ + default void injectDefaultValues( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectDefaultValues(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java index c2ec4ce522ac..eee49ecf486e 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelPathTranslator.java @@ -40,4 +40,17 @@ public interface ModelPathTranslator { * @since 4.0.0 */ Model alignToBaseDirectory(Model model, Path basedir, ModelBuilderRequest request); + + /** + * Builder-accepting variant of {@link #alignToBaseDirectory}. + * + * @since 4.0.0 + */ + default void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuilderRequest request) { + Model built = builder.build(); + Model result = alignToBaseDirectory(built, basedir, request); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java index a216b99d7a86..f89cf80e9c1c 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ModelUrlNormalizer.java @@ -36,4 +36,17 @@ public interface ModelUrlNormalizer { * @param request The model building request that holds further settings, must not be {@code null}. */ Model normalize(Model model, ModelBuilderRequest request); + + /** + * Builder-accepting variant of {@link #normalize}. + * + * @since 4.0.0 + */ + default void normalize(Model.Builder builder, ModelBuilderRequest request) { + Model built = builder.build(); + Model result = normalize(built, request); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java index bdd249489df0..a5f15cb111ea 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginConfigurationExpander.java @@ -37,4 +37,18 @@ public interface PluginConfigurationExpander { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model expandPluginConfiguration(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #expandPluginConfiguration}. + * + * @since 4.0.0 + */ + default void expandPluginConfiguration( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = expandPluginConfiguration(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java index 36d7f7e19f86..a2e39c63ca40 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/PluginManagementInjector.java @@ -38,4 +38,17 @@ public interface PluginManagementInjector { * @param problems The container used to collect problems that were encountered, must not be {@code null}. */ Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant of {@link #injectManagement}. + * + * @since 4.0.0 + */ + default void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectManagement(built, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java index 7361318f9801..256ecabdbf17 100644 --- a/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java +++ b/api/maven-api-spi/src/main/java/org/apache/maven/api/services/model/ProfileInjector.java @@ -57,4 +57,21 @@ default Model injectProfile( */ Model injectProfiles( Model model, List profiles, ModelBuilderRequest request, ModelProblemCollector problems); + + /** + * Builder-accepting variant that injects profile values into the model builder directly. + * + * @since 4.0.0 + */ + default void injectProfiles( + Model.Builder builder, + List profiles, + ModelBuilderRequest request, + ModelProblemCollector problems) { + Model built = builder.build(); + Model result = injectProfiles(built, profiles, request, problems); + if (result != built) { + builder.reset(result); + } + } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java index fc159b8ccd2a..74338374cf2f 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java @@ -44,6 +44,28 @@ @Singleton public class DefaultPluginConfigurationExpander implements PluginConfigurationExpander { + @Override + public void expandPluginConfiguration( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model model = builder.build(); + Build build = model.getBuild(); + if (build != null) { + Build newBuild = build.withPlugins(expandPlugin(build.getPlugins())); + PluginManagement pluginManagement = newBuild.getPluginManagement(); + if (pluginManagement != null) { + newBuild = newBuild.withPluginManagement( + pluginManagement.withPlugins(expandPlugin(pluginManagement.getPlugins()))); + } + if (newBuild != build) { + builder.build(newBuild); + } + } + Reporting reporting = model.getReporting(); + if (reporting != null) { + expandReport(reporting.getPlugins()); + } + } + @Override public Model expandPluginConfiguration(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Build build = model.getBuild(); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java index 1ca7e2bcbed9..86b821691e45 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java @@ -45,6 +45,15 @@ public class DefaultDependencyManagementInjector implements DependencyManagement private ManagementModelMerger merger = new ManagementModelMerger(); + @Override + public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model model = builder.build(); + List merged = merger.computeMergedDependencies(model); + if (merged != null) { + builder.dependencies(merged); + } + } + @Override public Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { return merger.mergeManagedDependencies(model); @@ -55,11 +64,12 @@ public Model injectManagement(Model model, ModelBuilderRequest request, ModelPro */ protected static class ManagementModelMerger extends MavenModelMerger { - public Model mergeManagedDependencies(Model model) { + /** + * Computes the merged dependency list, or returns {@code null} if no dependencies were modified. + */ + List computeMergedDependencies(Model model) { DependencyManagement dependencyManagement = model.getDependencyManagement(); if (dependencyManagement != null) { - // Use Builders to accumulate changes across all managed dependencies, - // deferring build() until after all merges are complete Map originalDeps = new HashMap<>(); Map builderDeps = new HashMap<>(); Map context = Collections.emptyMap(); @@ -76,7 +86,6 @@ public Model mergeManagedDependencies(Model model) { if (dependency != null) { Dependency.Builder merged = mergeDependencyToBuilder(dependency, managedDependency, false, context); - // Only track modifications if the builder actually changed something if (merged != null) { builderDeps.put(key, merged); modified = true; @@ -89,13 +98,17 @@ public Model mergeManagedDependencies(Model model) { for (Dependency dep : model.getDependencies()) { Object key = getDependencyKey().apply(dep); Dependency.Builder builder = builderDeps.get(key); - // Only build() the dependencies that were actually merged newDeps.add(builder != null ? builder.build() : dep); } - return Model.newBuilder(model).dependencies(newDeps).build(); + return newDeps; } } - return model; + return null; + } + + public Model mergeManagedDependencies(Model model) { + List merged = computeMergedDependencies(model); + return merged != null ? Model.newBuilder(model).dependencies(merged).build() : model; } @Override diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index 496eb81ad156..6b4e9e1ce2bf 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -1007,35 +1007,48 @@ void buildEffectiveModel(Collection importIds) throws ModelBuilderExcept setSource(resultModel); setRootModel(resultModel); + // Thread remaining stages through a Model.Builder to avoid intermediate build() calls. + // The builder-accepting default methods on each interface bridge to the Model-accepting + // versions; Phase E overrides these defaults for the hot stages. + Model.Builder builder = Model.newBuilder(resultModel, false); + // model path translation - resultModel = - modelPathTranslator.alignToBaseDirectory(resultModel, resultModel.getProjectDirectory(), request); + modelPathTranslator.alignToBaseDirectory(builder, resultModel.getProjectDirectory(), request); // plugin management injection - resultModel = pluginManagementInjector.injectManagement(resultModel, request, this); + pluginManagementInjector.injectManagement(builder, request, this); - // lifecycle bindings injection + // lifecycle bindings injection (ModelTransformer API — no builder variant) if (request.getRequestType() != ModelBuilderRequest.RequestType.CONSUMER_DEPENDENCY) { org.apache.maven.api.services.ModelTransformer lifecycleBindingsInjector = request.getLifecycleBindingsInjector(); if (lifecycleBindingsInjector != null) { - resultModel = lifecycleBindingsInjector.transform(resultModel, request, this); + Model built = builder.build(); + Model transformed = lifecycleBindingsInjector.transform(built, request, this); + if (transformed != built) { + builder.reset(transformed); + } } } - // dependency management import - resultModel = importDependencyManagement(resultModel, importIds); + // dependency management import (complex — needs Model access internally) + Model builtForImport = builder.build(); + Model imported = importDependencyManagement(builtForImport, importIds); + if (imported != builtForImport) { + builder.reset(imported); + } // dependency management injection - resultModel = dependencyManagementInjector.injectManagement(resultModel, request, this); + dependencyManagementInjector.injectManagement(builder, request, this); - resultModel = modelNormalizer.injectDefaultValues(resultModel, request, this); + modelNormalizer.injectDefaultValues(builder, request, this); if (request.getRequestType() != ModelBuilderRequest.RequestType.CONSUMER_DEPENDENCY) { // plugins configuration - resultModel = pluginConfigurationExpander.expandPluginConfiguration(resultModel, request, this); + pluginConfigurationExpander.expandPluginConfiguration(builder, request, this); } + resultModel = builder.build(); for (var transformer : transformers) { resultModel = transformer.transformEffectiveModel(resultModel); } @@ -1431,11 +1444,12 @@ Model activateFileModel(Model inputModel) throws ModelBuilderException { List interpolatedActivations = getProfileActivations(inputModel); inputModel = injectProfileActivations(inputModel, interpolatedActivations); - // profile injection - inputModel = profileInjector.injectProfiles(inputModel, activePomProfiles, request, this); - inputModel = profileInjector.injectProfiles(inputModel, activeExternalProfiles, request, this); + // profile injection via builder to avoid intermediate build between injections + Model.Builder builder = Model.newBuilder(inputModel, false); + profileInjector.injectProfiles(builder, activePomProfiles, request, this); + profileInjector.injectProfiles(builder, activeExternalProfiles, request, this); - return inputModel; + return builder.build(); } @SuppressWarnings("checkstyle:methodlength") @@ -1513,22 +1527,25 @@ private Model readEffectiveModel() throws ModelBuilderException { // profile injection - inject all profiles (local + inherited) into the model List activePomProfiles = getActiveProfiles(model.getProfiles(), profileActivationContext); - model = profileInjector.injectProfiles(model, activePomProfiles, request, this); - model = profileInjector.injectProfiles(model, activeExternalProfiles, request, this); + Model.Builder builder = Model.newBuilder(model, false); + profileInjector.injectProfiles(builder, activePomProfiles, request, this); + profileInjector.injectProfiles(builder, activeExternalProfiles, request, this); + model = builder.build(); // Track only the local profiles for this model // Use ModelProblemUtils.toId() to get groupId:artifactId:version format (without packaging) addActivePomProfiles(ModelProblemUtils.toId(model), localActivePomProfiles); - // model interpolation - Model resultModel = model; - resultModel = interpolateModel(resultModel, request, this); + // model interpolation + normalization + url normalization via builder + builder = Model.newBuilder(model, false); + interpolateModel(builder, request, this); // model normalization - resultModel = modelNormalizer.mergeDuplicates(resultModel, request, this); + modelNormalizer.mergeDuplicates(builder, request, this); // url normalization - resultModel = modelUrlNormalizer.normalize(resultModel, request); + modelUrlNormalizer.normalize(builder, request); + Model resultModel = builder.build(); // Now the fully interpolated model is available: reconfigure the resolver if (!resultModel.getRepositories().isEmpty()) { @@ -2419,6 +2436,14 @@ private Model injectProfileActivations(Model model, List activations return modified ? model.withProfiles(profiles) : model; } + private void interpolateModel(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model model = builder.build(); + Model result = interpolateModel(model, request, problems); + if (result != model) { + builder.reset(result); + } + } + private Model interpolateModel(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model interpolatedModel = modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java index f36918f770aa..9da91b4e30de 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java @@ -45,6 +45,42 @@ public class DefaultModelNormalizer implements ModelNormalizer { private DuplicateMerger merger = new DuplicateMerger(); + @Override + public void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model model = builder.build(); + + Build build = model.getBuild(); + if (build != null) { + List plugins = build.getPlugins(); + Map normalized = new LinkedHashMap<>(plugins.size() * 2); + + for (Plugin plugin : plugins) { + Object key = plugin.getKey(); + Plugin first = normalized.get(key); + if (first != null) { + plugin = merger.mergePlugin(plugin, first); + } + normalized.put(key, plugin); + } + + if (plugins.size() != normalized.size()) { + builder.build( + Build.newBuilder(build).plugins(normalized.values()).build()); + } + } + + List dependencies = model.getDependencies(); + Map normalizedDeps = new LinkedHashMap<>(dependencies.size() * 2); + + for (Dependency dependency : dependencies) { + normalizedDeps.put(dependency.getManagementKey(), dependency); + } + + if (dependencies.size() != normalizedDeps.size()) { + builder.dependencies(normalizedDeps.values()); + } + } + @Override public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model.Builder builder = Model.newBuilder(model); @@ -100,6 +136,26 @@ public Plugin mergePlugin(Plugin target, Plugin source) { } } + @Override + public void injectDefaultValues( + Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model model = builder.build(); + + List newDeps = injectList(model.getDependencies(), this::injectDependency); + if (newDeps != null) { + builder.dependencies(newDeps); + } + Build build = model.getBuild(); + if (build != null) { + Build newBuild = Build.newBuilder(build) + .plugins(injectList(build.getPlugins(), this::injectPlugin)) + .build(); + if (newBuild != build) { + builder.build(newBuild); + } + } + } + @Override public Model injectDefaultValues(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { Model.Builder builder = Model.newBuilder(model); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java index 52ebbbf9d9c6..f3431ec72646 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java @@ -51,12 +51,35 @@ public DefaultModelPathTranslator(PathTranslator pathTranslator) { this.pathTranslator = pathTranslator; } + @Override + public void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuilderRequest request) { + if (basedir == null) { + return; + } + Model model = builder.build(); + alignToBaseDirectory(model, basedir, builder); + } + @Override public Model alignToBaseDirectory(Model model, Path basedir, ModelBuilderRequest request) { if (model == null || basedir == null) { return model; } + Model.Builder builder = Model.newBuilder(model); + if (alignToBaseDirectory(model, basedir, builder)) { + return builder.build(); + } + return model; + } + + /** + * Shared logic: reads from {@code model}, writes modified fields to {@code builder}. + * Returns {@code true} if any field was modified. + */ + private boolean alignToBaseDirectory(Model model, Path basedir, Model.Builder builder) { + boolean modified = false; + Build build = model.getBuild(); Build newBuild = null; if (build != null) { @@ -82,12 +105,11 @@ public Model alignToBaseDirectory(Model model, Path basedir, ModelBuilderRequest .build(); } if (newBuild != build || newReporting != reporting) { - model = Model.newBuilder(model) - .build(newBuild) - .reporting(newReporting) - .build(); + builder.build(newBuild); + builder.reporting(newReporting); + modified = true; } - return model; + return modified; } /** diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java index 4f10a4ca1f26..c219816e2772 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java @@ -47,6 +47,21 @@ public class DefaultPluginManagementInjector implements PluginManagementInjector private ManagementModelMerger merger = new ManagementModelMerger(); + @Override + public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { + Model model = builder.build(); + Build build = model.getBuild(); + if (build != null) { + PluginManagement pluginManagement = build.getPluginManagement(); + if (pluginManagement != null) { + Build newBuild = merger.mergePluginContainerPlugins(build, pluginManagement); + if (newBuild != build) { + builder.build(newBuild); + } + } + } + } + @Override public Model injectManagement(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { return merger.mergeManagedBuildPlugins(model); diff --git a/src/mdo/model.vm b/src/mdo/model.vm index ed42df9bcf59..3496a96ed550 100644 --- a/src/mdo/model.vm +++ b/src/mdo/model.vm @@ -20,6 +20,11 @@ # #set ( $package = "${packageModelV4}" ) #set ( $root = $model.getClass( $model.getRoot($version), $version ) ) +## Build set of model class names for detecting list-of-model-object fields +#set ( $modelClassNames = [] ) +#foreach ( $c in $model.allClasses ) + #set ( $dummy = $modelClassNames.add($c.name) ) +#end #foreach ( $class in $model.allClasses ) #set ( $ancestors = $Helper.ancestors( $class ) ) #set ( $allFields = [] ) @@ -170,7 +175,22 @@ public class ${class.name} this.modelEncoding = builder.modelEncoding != null ? builder.modelEncoding : (builder.base != null ? builder.base.modelEncoding : "UTF-8"); #end #foreach ( $field in $class.getFields($version) ) - #if ( $field.type == "java.util.List" || $field.type == "java.util.Properties" || $field.type == "java.util.Map" ) + #set ( $cType = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $cIsModelObjList = false ) + #if ( $cType.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $cIsModelObjList = true ) + #end + #if ( $cIsModelObjList ) + if (builder.${field.name} != null) { + ArrayList<${field.to}> bl = new ArrayList<>(builder.${field.name}.size()); + for (${field.to}.Builder b : builder.${field.name}) { + bl.add(b.build()); + } + this.${field.name} = ImmutableCollections.copy(bl); + } else { + this.${field.name} = ImmutableCollections.copy(builder.base != null ? builder.base.${field.name} : null); + } + #elseif ( $field.type == "java.util.List" || $field.type == "java.util.Properties" || $field.type == "java.util.Map" ) this.${field.name} = ImmutableCollections.copy(builder.${field.name} != null ? builder.${field.name} : (builder.base != null ? builder.base.${field.name} : null)); #else #if ( $field.type == "boolean" || $field.type == "int" ) @@ -414,8 +434,14 @@ public class ${class.name} #end #foreach ( $field in $class.getFields($version) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $isModelObjList = false ) #if ( $type.startsWith("List<") ) - #set ( $type = ${type.replace('List<','Collection<')} ) + #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $isModelObjList = true ) + #set ( $type = "Collection<${field.to}.Builder>" ) + #else + #set ( $type = ${type.replace('List<','Collection<')} ) + #end #end #if ( $type == 'boolean' ) Boolean ${field.name}; @@ -458,7 +484,22 @@ public class ${class.name} #end if (forceCopy) { #foreach ( $field in $class.getFields($version) ) + #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $fcIsModelObjList = false ) + #if ( $type.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $fcIsModelObjList = true ) + #end + #if ( $fcIsModelObjList ) + if (base.${field.name} != null && !base.${field.name}.isEmpty()) { + ArrayList<${field.to}.Builder> bl = new ArrayList<>(base.${field.name}.size()); + for (${field.to} v : base.${field.name}) { + bl.add(${field.to}.newBuilder(v, false)); + } + this.${field.name} = bl; + } + #else this.${field.name} = base.${field.name}; + #end #end #if ( $locationTracking ) this.locations = base.locations; @@ -484,33 +525,100 @@ public class ${class.name} #end #foreach ( $field in $allFields ) + #set ( $cap = $Helper.capitalise( $field.name ) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $sIsModelObjList = false ) #if ( $type.startsWith("List<") ) - #set ( $type = ${type.replace('List<','Collection<')} ) + #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $sIsModelObjList = true ) + #else + #set ( $type = ${type.replace('List<','Collection<')} ) + #end #end #foreach( $ann in ${field.annotations} ) ${ann} #end + #if ( $sIsModelObjList ) + /** + * Sets the {@code ${field.name}} for this builder. Each element is wrapped + * in a lightweight builder to support deferred materialization. + * + * @param ${field.name} the elements to set + * @return this builder + */ + @Nonnull + public Builder ${field.name}(Collection<${field.to}> ${field.name}) { + if (${field.name} != null) { + ArrayList<${field.to}.Builder> bl = new ArrayList<>(${field.name}.size()); + for (${field.to} v : ${field.name}) { + bl.add(${field.to}.newBuilder(v, false)); + } + this.${field.name} = bl; + } else { + this.${field.name} = null; + } + return this; + } + + /** + * Returns the mutable list of {@code ${field.to}.Builder} elements. + * If the builder has not been explicitly set, lazily wraps the base + * object's immutable list into builders and caches the result. + * + * @return a mutable list of element builders, never {@code null} + */ + @Nonnull + public List<${field.to}.Builder> getModifiable${cap}() { + if (${field.name} == null) { + if (base != null && !base.${field.name}.isEmpty()) { + ArrayList<${field.to}.Builder> bl = new ArrayList<>(base.${field.name}.size()); + for (${field.to} v : base.${field.name}) { + bl.add(${field.to}.newBuilder(v, false)); + } + this.${field.name} = bl; + } else { + this.${field.name} = new ArrayList<>(); + } + } + @SuppressWarnings("unchecked") + List<${field.to}.Builder> result = (List<${field.to}.Builder>) (List) ${field.name}; + return result; + } + + #else @Nonnull public Builder ${field.name}(${type} ${field.name}) { this.${field.name} = ${field.name}; return this; } + #end #end #foreach ( $field in $allFields ) #set ( $cap = $Helper.capitalise( $field.name ) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $gIsModelObjList = false ) ## Builder stores List fields as Collection — match that type for the getter #if ( $type.startsWith("List<") ) - #set ( $type = ${type.replace('List<','Collection<')} ) + #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $gIsModelObjList = true ) + #set ( $type = "Collection<${field.to}.Builder>" ) + #else + #set ( $type = ${type.replace('List<','Collection<')} ) + #end #end #if ( $type == "boolean" || $type == "Boolean" ) #set ( $pfx = "is" ) #else #set ( $pfx = "get" ) #end - #if ( $type == "boolean" ) + #if ( $gIsModelObjList ) + ## For model-object lists, return the raw field only (don't resolve through base + ## to avoid unwanted lazy-wrapping that would defeat the short-circuit optimization) + public ${type} ${pfx}${cap}() { + return ${field.name}; + } + #elseif ( $type == "boolean" ) public ${type} ${pfx}${cap}() { return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); } @@ -556,12 +664,50 @@ public class ${class.name} } #end + /** + * Resets this builder to wrap the specified base object, clearing all + * explicitly set fields. After reset, all getters resolve through the + * new base. Used by pipeline stage default methods to replace the + * builder's state with a stage's result without allocating a new Builder. + * + * @param base the new base object to wrap + * @return this builder + */ + @Nonnull + public Builder reset(${class.name} base) { + #if ( $class.superClass ) + super.reset(base); + #end + this.base = base; + #if ( $class == $root ) + this.namespaceUri = null; + this.modelEncoding = null; + #end + #foreach ( $field in $class.getFields($version) ) + this.${field.name} = null; + #end + #if ( ! $class.superClass && $locationTracking ) + this.locations = null; + this.importedFrom = null; + #end + return this; + } + @Nonnull public ${class.name} build() { // this method should not contain any logic other than creating (or reusing) an object in order to ease subclassing if (base != null #foreach ( $field in $allFields ) + #set ( $bType = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) + #set ( $bIsModelObjList = false ) + #if ( $bType.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) + #set ( $bIsModelObjList = true ) + #end + #if ( $bIsModelObjList ) + && (${field.name} == null || (${field.name}.isEmpty() && base.${field.name}.isEmpty())) + #else && (${field.name} == null || ${field.name} == base.${field.name}) + #end #end ) { return base; From 568b44b8613ef718207890c72c654c8e330c253c Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 09:15:52 +0200 Subject: [PATCH 4/6] Eliminate unnecessary builder.build() calls in pipeline stages Add getBuilt*() methods to Builder for model-object list fields that return List by resolving through base when unmodified (zero cost) or building just that field's builders (avoids full model materialization). Fix 5 pipeline stages to use builder getters instead of builder.build(): - DefaultPluginConfigurationExpander: getBuild()/getReporting() - DefaultModelNormalizer: getBuild()/getBuiltDependencies() - DefaultDependencyManagementInjector: getDependencyManagement()/getBuiltDependencies() - DefaultPluginManagementInjector: getBuild() - DefaultModelPathTranslator: getBuild()/getReporting() Each stage previously called builder.build() to read 1-2 fields, which triggered full model materialization including wrap/unwrap of all model-object list fields. With builder getters, only the needed fields are accessed. Co-Authored-By: Claude Opus 4.6 --- .../DefaultPluginConfigurationExpander.java | 8 +- .../DefaultDependencyManagementInjector.java | 74 +++++++++++-------- .../impl/model/DefaultModelNormalizer.java | 14 ++-- .../model/DefaultModelPathTranslator.java | 31 +++++++- .../DefaultPluginManagementInjector.java | 5 +- src/mdo/model.vm | 13 ++++ 6 files changed, 102 insertions(+), 43 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java index 74338374cf2f..264d01d35642 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultPluginConfigurationExpander.java @@ -47,8 +47,10 @@ public class DefaultPluginConfigurationExpander implements PluginConfigurationEx @Override public void expandPluginConfiguration( Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - Model model = builder.build(); - Build build = model.getBuild(); + + // Use builder getters instead of builder.build() to avoid materializing + // all model-object lists (especially dependencies) just to read Build/Reporting + Build build = builder.getBuild(); if (build != null) { Build newBuild = build.withPlugins(expandPlugin(build.getPlugins())); PluginManagement pluginManagement = newBuild.getPluginManagement(); @@ -60,7 +62,7 @@ public void expandPluginConfiguration( builder.build(newBuild); } } - Reporting reporting = model.getReporting(); + Reporting reporting = builder.getReporting(); if (reporting != null) { expandReport(reporting.getPlugins()); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java index 86b821691e45..0013bd03c670 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java @@ -47,10 +47,15 @@ public class DefaultDependencyManagementInjector implements DependencyManagement @Override public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - Model model = builder.build(); - List merged = merger.computeMergedDependencies(model); - if (merged != null) { - builder.dependencies(merged); + // Use builder getters instead of builder.build() to avoid materializing + // all model-object lists just to read Dependencies and DependencyManagement + DependencyManagement depMgmt = builder.getDependencyManagement(); + if (depMgmt != null) { + List deps = builder.getBuiltDependencies(); + List merged = merger.computeMergedDependencies(deps, depMgmt); + if (merged != null) { + builder.dependencies(merged); + } } } @@ -70,38 +75,47 @@ protected static class ManagementModelMerger extends MavenModelMerger { List computeMergedDependencies(Model model) { DependencyManagement dependencyManagement = model.getDependencyManagement(); if (dependencyManagement != null) { - Map originalDeps = new HashMap<>(); - Map builderDeps = new HashMap<>(); - Map context = Collections.emptyMap(); + return computeMergedDependencies(model.getDependencies(), dependencyManagement); + } + return null; + } - for (Dependency dependency : model.getDependencies()) { - Object key = getDependencyKey().apply(dependency); - originalDeps.put(key, dependency); - } + /** + * Computes the merged dependency list from pre-extracted deps and dep management, + * or returns {@code null} if no dependencies were modified. + */ + List computeMergedDependencies( + List dependencies, DependencyManagement dependencyManagement) { + Map originalDeps = new HashMap<>(); + Map builderDeps = new HashMap<>(); + Map context = Collections.emptyMap(); + + for (Dependency dependency : dependencies) { + Object key = getDependencyKey().apply(dependency); + originalDeps.put(key, dependency); + } - boolean modified = false; - for (Dependency managedDependency : dependencyManagement.getDependencies()) { - Object key = getDependencyKey().apply(managedDependency); - Dependency dependency = originalDeps.get(key); - if (dependency != null) { - Dependency.Builder merged = - mergeDependencyToBuilder(dependency, managedDependency, false, context); - if (merged != null) { - builderDeps.put(key, merged); - modified = true; - } + boolean modified = false; + for (Dependency managedDependency : dependencyManagement.getDependencies()) { + Object key = getDependencyKey().apply(managedDependency); + Dependency dependency = originalDeps.get(key); + if (dependency != null) { + Dependency.Builder merged = mergeDependencyToBuilder(dependency, managedDependency, false, context); + if (merged != null) { + builderDeps.put(key, merged); + modified = true; } } + } - if (modified) { - List newDeps = new ArrayList<>(originalDeps.size()); - for (Dependency dep : model.getDependencies()) { - Object key = getDependencyKey().apply(dep); - Dependency.Builder builder = builderDeps.get(key); - newDeps.add(builder != null ? builder.build() : dep); - } - return newDeps; + if (modified) { + List newDeps = new ArrayList<>(originalDeps.size()); + for (Dependency dep : dependencies) { + Object key = getDependencyKey().apply(dep); + Dependency.Builder builder = builderDeps.get(key); + newDeps.add(builder != null ? builder.build() : dep); } + return newDeps; } return null; } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java index 9da91b4e30de..c8eb52a45529 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelNormalizer.java @@ -47,9 +47,10 @@ public class DefaultModelNormalizer implements ModelNormalizer { @Override public void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - Model model = builder.build(); - Build build = model.getBuild(); + // Use builder getters instead of builder.build() to avoid materializing + // all model-object lists (especially dependencies) just to read Build + Build build = builder.getBuild(); if (build != null) { List plugins = build.getPlugins(); Map normalized = new LinkedHashMap<>(plugins.size() * 2); @@ -69,7 +70,7 @@ public void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, } } - List dependencies = model.getDependencies(); + List dependencies = builder.getBuiltDependencies(); Map normalizedDeps = new LinkedHashMap<>(dependencies.size() * 2); for (Dependency dependency : dependencies) { @@ -139,13 +140,14 @@ public Plugin mergePlugin(Plugin target, Plugin source) { @Override public void injectDefaultValues( Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - Model model = builder.build(); - List newDeps = injectList(model.getDependencies(), this::injectDependency); + // Use builder getters instead of builder.build() to avoid materializing + // all model-object lists just to read Dependencies and Build + List newDeps = injectList(builder.getBuiltDependencies(), this::injectDependency); if (newDeps != null) { builder.dependencies(newDeps); } - Build build = model.getBuild(); + Build build = builder.getBuild(); if (build != null) { Build newBuild = Build.newBuilder(build) .plugins(injectList(build.getPlugins(), this::injectPlugin)) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java index f3431ec72646..9e127856ad67 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelPathTranslator.java @@ -56,8 +56,35 @@ public void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuild if (basedir == null) { return; } - Model model = builder.build(); - alignToBaseDirectory(model, basedir, builder); + // Use builder getters instead of builder.build() to avoid materializing + // all model-object lists just to read Build and Reporting + Build build = builder.getBuild(); + Build newBuild = null; + if (build != null) { + newBuild = Build.newBuilder(build) + .sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) + .directory(alignToBaseDirectory(build.getDirectory(), basedir)) + .sourceDirectory(alignToBaseDirectory(build.getSourceDirectory(), basedir)) + .testSourceDirectory(alignToBaseDirectory(build.getTestSourceDirectory(), basedir)) + .scriptSourceDirectory(alignToBaseDirectory(build.getScriptSourceDirectory(), basedir)) + .resources(map(build.getResources(), this::alignToBaseDirectory, basedir)) + .testResources(map(build.getTestResources(), this::alignToBaseDirectory, basedir)) + .filters(map(build.getFilters(), this::alignToBaseDirectory, basedir)) + .outputDirectory(alignToBaseDirectory(build.getOutputDirectory(), basedir)) + .testOutputDirectory(alignToBaseDirectory(build.getTestOutputDirectory(), basedir)) + .build(); + } + Reporting reporting = builder.getReporting(); + Reporting newReporting = null; + if (reporting != null) { + newReporting = Reporting.newBuilder(reporting) + .outputDirectory(alignToBaseDirectory(reporting.getOutputDirectory(), basedir)) + .build(); + } + if (newBuild != build || newReporting != reporting) { + builder.build(newBuild); + builder.reporting(newReporting); + } } @Override diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java index c219816e2772..b3194328ed51 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultPluginManagementInjector.java @@ -49,8 +49,9 @@ public class DefaultPluginManagementInjector implements PluginManagementInjector @Override public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - Model model = builder.build(); - Build build = model.getBuild(); + // Use builder getter instead of builder.build() to avoid materializing + // all model-object lists just to read Build + Build build = builder.getBuild(); if (build != null) { PluginManagement pluginManagement = build.getPluginManagement(); if (pluginManagement != null) { diff --git a/src/mdo/model.vm b/src/mdo/model.vm index 3496a96ed550..f8255170ae51 100644 --- a/src/mdo/model.vm +++ b/src/mdo/model.vm @@ -618,6 +618,19 @@ public class ${class.name} public ${type} ${pfx}${cap}() { return ${field.name}; } + ## Provide a method to get the built list, resolving through base when unmodified. + ## This avoids the overhead of a full model build() just to read one list field + ## in pipeline stages that only need to inspect a specific list. + public List<${field.to}> getBuilt${cap}() { + if (${field.name} == null) { + return base != null ? base.get${cap}() : List.of(); + } + ArrayList<${field.to}> result = new ArrayList<>(${field.name}.size()); + for (${field.to}.Builder b : ${field.name}) { + result.add(b.build()); + } + return result; + } #elseif ( $type == "boolean" ) public ${type} ${pfx}${cap}() { return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); From 05b9583a9a70163a9d9c71144b4b8fa7b50888a4 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 23:12:08 +0200 Subject: [PATCH 5/6] Move XML location-tracking API to PR #12655 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove addLocationInformation from XmlReaderRequest and DefaultModelXmlFactory — these changes belong in the wire-location-tracking-to-parser branch (PR #12655), not in the model-building-pipeline optimization PR. Co-Authored-By: Claude Opus 4.6 --- .../api/services/xml/XmlReaderRequest.java | 33 ++----------------- .../maven/impl/DefaultModelXmlFactory.java | 1 - 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java b/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java index 07fd999b9f8b..41733eb08bf3 100644 --- a/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java +++ b/api/maven-api-core/src/main/java/org/apache/maven/api/services/xml/XmlReaderRequest.java @@ -66,20 +66,6 @@ public interface XmlReaderRequest { boolean isAddDefaultEntities(); - /** - * Indicates whether location information (line/column tracking) should be - * recorded during parsing. Defaults to {@code true}. Setting this to - * {@code false} for imported dependency management POMs avoids allocating - * location maps that are never read, significantly reducing memory churn - * in large reactors. - * - * @return {@code true} if location information should be tracked - * @since 4.0.0 - */ - default boolean isAddLocationInformation() { - return true; - } - interface Transformer { /** * Interpolate the value read from the xml document @@ -109,7 +95,6 @@ class XmlReaderRequestBuilder { String modelId; String location; boolean addDefaultEntities = true; - boolean addLocationInformation = true; public XmlReaderRequestBuilder path(Path path) { this.path = path; @@ -161,11 +146,6 @@ public XmlReaderRequestBuilder addDefaultEntities(boolean addDefaultEntities) { return this; } - public XmlReaderRequestBuilder addLocationInformation(boolean addLocationInformation) { - this.addLocationInformation = addLocationInformation; - return this; - } - public XmlReaderRequest build() { return new DefaultXmlReaderRequest( path, @@ -177,8 +157,7 @@ public XmlReaderRequest build() { strict, modelId, location, - addDefaultEntities, - addLocationInformation); + addDefaultEntities); } private static class DefaultXmlReaderRequest implements XmlReaderRequest { @@ -192,7 +171,6 @@ private static class DefaultXmlReaderRequest implements XmlReaderRequest { final String modelId; final String location; final boolean addDefaultEntities; - final boolean addLocationInformation; @SuppressWarnings("checkstyle:ParameterNumber") DefaultXmlReaderRequest( @@ -205,8 +183,7 @@ private static class DefaultXmlReaderRequest implements XmlReaderRequest { boolean strict, String modelId, String location, - boolean addDefaultEntities, - boolean addLocationInformation) { + boolean addDefaultEntities) { this.path = path; this.rootDirectory = rootDirectory; this.url = url; @@ -217,7 +194,6 @@ private static class DefaultXmlReaderRequest implements XmlReaderRequest { this.modelId = modelId; this.location = location; this.addDefaultEntities = addDefaultEntities; - this.addLocationInformation = addLocationInformation; } @Override @@ -269,11 +245,6 @@ public String getLocation() { public boolean isAddDefaultEntities() { return addDefaultEntities; } - - @Override - public boolean isAddLocationInformation() { - return addLocationInformation; - } } } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java index c87ce5034315..575d35e230d8 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelXmlFactory.java @@ -132,7 +132,6 @@ private Model doRead(XmlReaderRequest request) throws XmlReaderException { ? new MavenStaxReader(request.getTransformer()::transform) : new MavenStaxReader(); xml.setAddDefaultEntities(request.isAddDefaultEntities()); - xml.setAddLocationInformation(request.isAddLocationInformation()); if (inputStream != null) { return xml.read(inputStream, request.isStrict(), source); } else if (reader != null) { From 394b9f2555801c57451f4d504f2a50ba4762c6f3 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 28 Aug 2026 22:00:38 +0200 Subject: [PATCH 6/6] fix: preserve first-wins semantics for duplicate managed dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple managed dependency declarations exist for the same key (e.g. MNG-4005), the merge loop must use the already-merged result as the target for subsequent merges. The refactored code used two separate maps (originalDeps for lookups, builderDeps for results), so each merge always started from the original dependency — causing the last managed declaration to overwrite earlier values instead of being ignored. This broke MavenITmng4403LenientDependencyPomParsingTest where duplicate managed deps for artifact 'c' (v0.1 compile, v0.2 test) resulted in c:0.2 (unresolvable) instead of c:0.1. Co-Authored-By: Claude Opus 4.6 --- .../impl/model/DefaultDependencyManagementInjector.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java index 0013bd03c670..d7015c93ab36 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultDependencyManagementInjector.java @@ -100,6 +100,15 @@ List computeMergedDependencies( Object key = getDependencyKey().apply(managedDependency); Dependency dependency = originalDeps.get(key); if (dependency != null) { + // When duplicate managed deps exist for the same key (e.g. MNG-4005), + // each subsequent merge must use the already-merged result as the target + // so that "first declaration wins" semantics are preserved (target dominates + // when sourceDominant=false). Without this, the last managed dep would + // overwrite earlier values instead of being ignored. + Dependency.Builder prev = builderDeps.get(key); + if (prev != null) { + dependency = prev.build(); + } Dependency.Builder merged = mergeDependencyToBuilder(dependency, managedDependency, false, context); if (merged != null) { builderDeps.put(key, merged);