From 163f25f8be117456829c9cd5ca95b07cb4f8fbea Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 01:51:23 +0200 Subject: [PATCH 1/8] 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 | 65 ++++++++--------- src/mdo/merger.vm | 11 +++ src/mdo/model.vm | 71 +++++++++++++++++-- 8 files changed, 159 insertions(+), 52 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 bec34fe0fa76..7d1d756076aa 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 @@ -246,29 +246,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); } /** @@ -283,33 +283,24 @@ 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) { - 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()); - } - - /** - * 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; + 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 + dep.getLocationsHashCode(); + h = 31 * h + Objects.hashCode(dep.getImportedFrom()); + return h; } } 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 27793b0c6e9107487f9fec7f4f541772f4e4b744 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 03:09:01 +0200 Subject: [PATCH 2/8] 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 a51720be285b97139f8f4374ddc41170f6467753 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 09:15:52 +0200 Subject: [PATCH 3/8] 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 0baf595faad3e08928236255501e21bbe0496238 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 23:12:08 +0200 Subject: [PATCH 4/8] 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 e254428219232077a7ef86030e4f7bf378cee5cf Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sat, 1 Aug 2026 23:43:14 +0200 Subject: [PATCH 5/8] Store single model-object fields as sub-builders in generated Builder classes Extend the deferred-build pattern from list-of-model-object fields to single model-object fields (multiplicity=1). Builder classes now store fields like Build, Reporting, DependencyManagement, Parent, Scm, etc. as X.Builder instead of immutable X. This eliminates intermediate immutable object allocations when pipeline stages modify these nested objects. Changes to model.vm: - Builder field declarations use X.Builder for model-object types - Setter wraps immutable input: X.newBuilder(val, false) - Getter resolves through .build() on the sub-builder - New getModifiable*() returns the raw X.Builder for in-place mutation - Constructor calls .build() on sub-builders during materialization - forceCopy wraps base values into sub-builders - build() short-circuit uses null-only check (null = unmodified) Pipeline stage optimizations using getModifiable*(): - DefaultModelPathTranslator: mutates Build/Reporting sub-builders in place instead of creating intermediate immutables - DefaultModelNormalizer: sets plugins directly on Build sub-builder - DefaultPluginConfigurationExpander: mutates Build and nested PluginManagement sub-builders in place - DefaultPluginManagementInjector: extracts merged plugin list and sets it on the Build sub-builder directly Co-Authored-By: Claude Opus 4.6 --- .../DefaultPluginConfigurationExpander.java | 14 ++--- .../impl/model/DefaultModelNormalizer.java | 27 +++----- .../model/DefaultModelPathTranslator.java | 41 ++++-------- .../DefaultPluginManagementInjector.java | 45 ++++++++------ src/mdo/model.vm | 62 +++++++++++++++++++ 5 files changed, 116 insertions(+), 73 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 264d01d35642..95a4c2f9869c 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 @@ -48,18 +48,14 @@ public class DefaultPluginConfigurationExpander implements PluginConfigurationEx public void expandPluginConfiguration( Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - // Use builder getters instead of builder.build() to avoid materializing - // all model-object lists (especially dependencies) just to read Build/Reporting + // Mutate the Build sub-builder in place to avoid intermediate immutable allocations Build build = builder.getBuild(); if (build != null) { - Build newBuild = build.withPlugins(expandPlugin(build.getPlugins())); - PluginManagement pluginManagement = newBuild.getPluginManagement(); + Build.Builder bb = builder.getModifiableBuild(); + bb.plugins(expandPlugin(build.getPlugins())); + PluginManagement pluginManagement = build.getPluginManagement(); if (pluginManagement != null) { - newBuild = newBuild.withPluginManagement( - pluginManagement.withPlugins(expandPlugin(pluginManagement.getPlugins()))); - } - if (newBuild != build) { - builder.build(newBuild); + bb.getModifiablePluginManagement().plugins(expandPlugin(pluginManagement.getPlugins())); } } Reporting reporting = builder.getReporting(); 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 c8eb52a45529..5045c78cbb83 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 @@ -48,8 +48,7 @@ public class DefaultModelNormalizer implements ModelNormalizer { @Override public void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - // Use builder getters instead of builder.build() to avoid materializing - // all model-object lists (especially dependencies) just to read Build + // Use sub-builder mutation to avoid intermediate immutable allocations Build build = builder.getBuild(); if (build != null) { List plugins = build.getPlugins(); @@ -65,8 +64,7 @@ public void mergeDuplicates(Model.Builder builder, ModelBuilderRequest request, } if (plugins.size() != normalized.size()) { - builder.build( - Build.newBuilder(build).plugins(normalized.values()).build()); + builder.getModifiableBuild().plugins(normalized.values()); } } @@ -101,8 +99,7 @@ public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProb } if (plugins.size() != normalized.size()) { - builder.build( - Build.newBuilder(build).plugins(normalized.values()).build()); + builder.getModifiableBuild().plugins(normalized.values()); } } @@ -141,19 +138,15 @@ public Plugin mergePlugin(Plugin target, Plugin source) { public void injectDefaultValues( Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - // 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 = builder.getBuild(); if (build != null) { - Build newBuild = Build.newBuilder(build) - .plugins(injectList(build.getPlugins(), this::injectPlugin)) - .build(); - if (newBuild != build) { - builder.build(newBuild); + List newPlugins = injectList(build.getPlugins(), this::injectPlugin); + if (newPlugins != null) { + builder.getModifiableBuild().plugins(newPlugins); } } } @@ -165,10 +158,10 @@ public Model injectDefaultValues(Model model, ModelBuilderRequest request, Model builder.dependencies(injectList(model.getDependencies(), this::injectDependency)); Build build = model.getBuild(); if (build != null) { - Build newBuild = Build.newBuilder(build) - .plugins(injectList(build.getPlugins(), this::injectPlugin)) - .build(); - builder.build(newBuild != build ? newBuild : null); + List newPlugins = injectList(build.getPlugins(), this::injectPlugin); + if (newPlugins != null) { + builder.getModifiableBuild().plugins(newPlugins); + } } return builder.build(); 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 9e127856ad67..2c5af59ce280 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,13 +56,12 @@ public void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuild if (basedir == null) { return; } - // Use builder getters instead of builder.build() to avoid materializing - // all model-object lists just to read Build and Reporting + // Mutate the Build and Reporting sub-builders in place, avoiding + // intermediate immutable object allocations Build build = builder.getBuild(); - Build newBuild = null; if (build != null) { - newBuild = Build.newBuilder(build) - .sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) + Build.Builder bb = builder.getModifiableBuild(); + bb.sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) .directory(alignToBaseDirectory(build.getDirectory(), basedir)) .sourceDirectory(alignToBaseDirectory(build.getSourceDirectory(), basedir)) .testSourceDirectory(alignToBaseDirectory(build.getTestSourceDirectory(), basedir)) @@ -71,19 +70,12 @@ public void alignToBaseDirectory(Model.Builder builder, Path basedir, ModelBuild .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(); + .testOutputDirectory(alignToBaseDirectory(build.getTestOutputDirectory(), basedir)); } 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); + builder.getModifiableReporting() + .outputDirectory(alignToBaseDirectory(reporting.getOutputDirectory(), basedir)); } } @@ -108,10 +100,9 @@ private boolean alignToBaseDirectory(Model model, Path basedir, Model.Builder bu boolean modified = false; Build build = model.getBuild(); - Build newBuild = null; if (build != null) { - newBuild = Build.newBuilder(build) - .sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) + Build.Builder bb = builder.getModifiableBuild(); + bb.sources(map(build.getSources(), this::alignToBaseDirectory, basedir)) .directory(alignToBaseDirectory(build.getDirectory(), basedir)) .sourceDirectory(alignToBaseDirectory(build.getSourceDirectory(), basedir)) .testSourceDirectory(alignToBaseDirectory(build.getTestSourceDirectory(), basedir)) @@ -120,20 +111,14 @@ private boolean alignToBaseDirectory(Model model, Path basedir, Model.Builder bu .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(); + .testOutputDirectory(alignToBaseDirectory(build.getTestOutputDirectory(), basedir)); + modified = true; } Reporting reporting = model.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); + builder.getModifiableReporting() + .outputDirectory(alignToBaseDirectory(reporting.getOutputDirectory(), basedir)); modified = true; } 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 b3194328ed51..8219b5380ef8 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,15 +49,14 @@ public class DefaultPluginManagementInjector implements PluginManagementInjector @Override public void injectManagement(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - // Use builder getter instead of builder.build() to avoid materializing - // all model-object lists just to read Build + // Use sub-builder mutation to avoid intermediate immutable allocations Build build = builder.getBuild(); if (build != null) { PluginManagement pluginManagement = build.getPluginManagement(); if (pluginManagement != null) { - Build newBuild = merger.mergePluginContainerPlugins(build, pluginManagement); - if (newBuild != build) { - builder.build(newBuild); + List mergedPlugins = merger.mergeManagedPluginList(build, pluginManagement); + if (mergedPlugins != null) { + builder.getModifiableBuild().plugins(mergedPlugins); } } } @@ -73,18 +72,12 @@ public Model injectManagement(Model model, ModelBuilderRequest request, ModelPro */ protected static class ManagementModelMerger extends MavenModelMerger { - public Model mergeManagedBuildPlugins(Model model) { - Build build = model.getBuild(); - if (build != null) { - PluginManagement pluginManagement = build.getPluginManagement(); - if (pluginManagement != null) { - return model.withBuild(mergePluginContainerPlugins(build, pluginManagement)); - } - } - return model; - } - - private Build mergePluginContainerPlugins(Build target, PluginContainer source) { + /** + * Returns the merged plugin list, or {@code null} if no changes were made. + * Used by the builder-accepting pipeline stage to set plugins directly on + * the Build sub-builder without creating intermediate immutable Build objects. + */ + List mergeManagedPluginList(Build target, PluginContainer source) { List src = source.getPlugins(); if (!src.isEmpty()) { Map managedPlugins = new LinkedHashMap<>(src.size() * 2); @@ -105,9 +98,23 @@ private Build mergePluginContainerPlugins(Build target, PluginContainer source) } newPlugins.add(element); } - return target.withPlugins(newPlugins); + return newPlugins; } - return target; + return null; + } + + public Model mergeManagedBuildPlugins(Model model) { + Build build = model.getBuild(); + if (build != null) { + PluginManagement pluginManagement = build.getPluginManagement(); + if (pluginManagement != null) { + List mergedPlugins = mergeManagedPluginList(build, pluginManagement); + if (mergedPlugins != null) { + return model.withBuild(build.withPlugins(mergedPlugins)); + } + } + } + return model; } @Override diff --git a/src/mdo/model.vm b/src/mdo/model.vm index f8255170ae51..78d73032f784 100644 --- a/src/mdo/model.vm +++ b/src/mdo/model.vm @@ -180,6 +180,10 @@ public class ${class.name} #if ( $cType.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) #set ( $cIsModelObjList = true ) #end + #set ( $cIsModelObj = false ) + #if ( !$cType.startsWith("List<") && $modelClassNames.contains($cType) ) + #set ( $cIsModelObj = true ) + #end #if ( $cIsModelObjList ) if (builder.${field.name} != null) { ArrayList<${field.to}> bl = new ArrayList<>(builder.${field.name}.size()); @@ -190,6 +194,8 @@ public class ${class.name} } else { this.${field.name} = ImmutableCollections.copy(builder.base != null ? builder.base.${field.name} : null); } + #elseif ( $cIsModelObj ) + this.${field.name} = builder.${field.name} != null ? builder.${field.name}.build() : (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 @@ -435,6 +441,7 @@ public class ${class.name} #foreach ( $field in $class.getFields($version) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) #set ( $isModelObjList = false ) + #set ( $isModelObj = false ) #if ( $type.startsWith("List<") ) #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) #set ( $isModelObjList = true ) @@ -442,6 +449,9 @@ public class ${class.name} #else #set ( $type = ${type.replace('List<','Collection<')} ) #end + #elseif ( $modelClassNames.contains($type) ) + #set ( $isModelObj = true ) + #set ( $type = "${type}.Builder" ) #end #if ( $type == 'boolean' ) Boolean ${field.name}; @@ -486,8 +496,11 @@ public class ${class.name} #foreach ( $field in $class.getFields($version) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) #set ( $fcIsModelObjList = false ) + #set ( $fcIsModelObj = false ) #if ( $type.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) #set ( $fcIsModelObjList = true ) + #elseif ( !$type.startsWith("List<") && $modelClassNames.contains($type) ) + #set ( $fcIsModelObj = true ) #end #if ( $fcIsModelObjList ) if (base.${field.name} != null && !base.${field.name}.isEmpty()) { @@ -497,6 +510,8 @@ public class ${class.name} } this.${field.name} = bl; } + #elseif ( $fcIsModelObj ) + this.${field.name} = base.${field.name} != null ? ${type}.newBuilder(base.${field.name}, false) : null; #else this.${field.name} = base.${field.name}; #end @@ -528,12 +543,15 @@ public class ${class.name} #set ( $cap = $Helper.capitalise( $field.name ) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) #set ( $sIsModelObjList = false ) + #set ( $sIsModelObj = false ) #if ( $type.startsWith("List<") ) #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) #set ( $sIsModelObjList = true ) #else #set ( $type = ${type.replace('List<','Collection<')} ) #end + #elseif ( $modelClassNames.contains($type) ) + #set ( $sIsModelObj = true ) #end #foreach( $ann in ${field.annotations} ) ${ann} @@ -585,6 +603,36 @@ public class ${class.name} return result; } + #elseif ( $sIsModelObj ) + /** + * Sets the {@code ${field.name}} for this builder, wrapping the immutable + * object in a lightweight builder to support deferred materialization. + * + * @param ${field.name} the {@code ${type}} to set, or {@code null} + * @return this builder + */ + @Nonnull + public Builder ${field.name}(${type} ${field.name}) { + this.${field.name} = ${field.name} != null ? ${type}.newBuilder(${field.name}, false) : null; + return this; + } + + /** + * Returns the mutable {@code ${type}.Builder} for the {@code ${field.name}} field. + * If the builder has not been explicitly set, lazily wraps the base + * object's immutable value into a builder and caches the result. + * + * @return a mutable builder for the ${field.name}, never {@code null} + */ + @Nonnull + public ${type}.Builder getModifiable${cap}() { + if (${field.name} == null) { + ${type} baseVal = base != null ? base.get${cap}() : null; + ${field.name} = baseVal != null ? ${type}.newBuilder(baseVal, false) : ${type}.newBuilder(false); + } + return ${field.name}; + } + #else @Nonnull public Builder ${field.name}(${type} ${field.name}) { @@ -598,6 +646,7 @@ public class ${class.name} #set ( $cap = $Helper.capitalise( $field.name ) ) #set ( $type = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) #set ( $gIsModelObjList = false ) + #set ( $gIsModelObj = false ) ## Builder stores List fields as Collection — match that type for the getter #if ( $type.startsWith("List<") ) #if ( $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) @@ -606,6 +655,8 @@ public class ${class.name} #else #set ( $type = ${type.replace('List<','Collection<')} ) #end + #elseif ( $modelClassNames.contains($type) ) + #set ( $gIsModelObj = true ) #end #if ( $type == "boolean" || $type == "Boolean" ) #set ( $pfx = "is" ) @@ -631,6 +682,12 @@ public class ${class.name} } return result; } + #elseif ( $gIsModelObj ) + ## For single model-object fields, resolve through .build() when the + ## sub-builder has been explicitly set; fall through to base otherwise. + public ${type} ${pfx}${cap}() { + return ${field.name} != null ? ${field.name}.build() : (base != null ? base.${pfx}${cap}() : null); + } #elseif ( $type == "boolean" ) public ${type} ${pfx}${cap}() { return ${field.name} != null ? ${field.name} : (base != null ? base.${pfx}${cap}() : ${field.defaultValue}); @@ -713,11 +770,16 @@ public class ${class.name} #foreach ( $field in $allFields ) #set ( $bType = ${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} ) #set ( $bIsModelObjList = false ) + #set ( $bIsModelObj = false ) #if ( $bType.startsWith("List<") && $field.to && $field.to != "String" && $modelClassNames.contains($field.to) ) #set ( $bIsModelObjList = true ) + #elseif ( !$bType.startsWith("List<") && $modelClassNames.contains($bType) ) + #set ( $bIsModelObj = true ) #end #if ( $bIsModelObjList ) && (${field.name} == null || (${field.name}.isEmpty() && base.${field.name}.isEmpty())) + #elseif ( $bIsModelObj ) + && ${field.name} == null #else && (${field.name} == null || ${field.name} == base.${field.name}) #end From 717582b34813e132ac84877ca48119b2d154faca Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sun, 2 Aug 2026 00:11:46 +0200 Subject: [PATCH 6/8] Use sub-builder mutation in interpolateModel, importDependencyManagement, and transformFileToRaw - interpolateModel: inline parent version interpolation using getModifiableParent().version() instead of withParent(withVersion()), avoiding two intermediate immutable Model/Parent allocations - importDependencyManagement: convert to builder-accepting, use getModifiableDependencyManagement().dependencies() and delegate to the builder-accepting importManagement SPI, eliminating the build()/reset() round-trip at the call site - transformFileToRaw: use getModifiableDependencyManagement() instead of depMgmt.withDependencies() to avoid intermediate DependencyManagement Co-Authored-By: Claude Opus 4.6 --- .../maven/impl/model/DefaultModelBuilder.java | 55 +++++++++++++------ 1 file changed, 38 insertions(+), 17 deletions(-) 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 6b4e9e1ce2bf..e4a3d79e867b 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 @@ -634,7 +634,7 @@ Model transformFileToRaw(Model model) { builder.dependencies(newDeps); } if (managedDepsChanged) { - builder.dependencyManagement(depMgmt.withDependencies(newManagedDeps)); + builder.getModifiableDependencyManagement().dependencies(newManagedDeps); } return builder.build(); } @@ -1031,12 +1031,8 @@ void buildEffectiveModel(Collection importIds) throws ModelBuilderExcept } } - // 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 import + importDependencyManagement(builder, importIds); // dependency management injection dependencyManagementInjector.injectManagement(builder, request, this); @@ -2065,14 +2061,14 @@ protected void mergeModel_Subprojects( return new ParentModelWithProfiles(injectedParentModel.withParent(null), parentActivePomProfiles); } - private Model importDependencyManagement(Model model, Collection importIds) { - DependencyManagement depMgmt = model.getDependencyManagement(); + private void importDependencyManagement(Model.Builder builder, Collection importIds) { + DependencyManagement depMgmt = builder.getDependencyManagement(); if (depMgmt == null) { - return model; + return; } - String importing = model.getGroupId() + ':' + model.getArtifactId() + ':' + model.getVersion(); + String importing = builder.getGroupId() + ':' + builder.getArtifactId() + ':' + builder.getVersion(); importIds.add(importing); @@ -2102,10 +2098,12 @@ private Model importDependencyManagement(Model model, Collection importI importIds.remove(importing); - model = model.withDependencyManagement( - model.getDependencyManagement().withDependencies(deps)); + // Use sub-builder mutation instead of immutable rebuild + builder.getModifiableDependencyManagement().dependencies(deps); - return dependencyManagementImporter.importManagement(model, importMgmts, request, this); + if (importMgmts != null) { + dependencyManagementImporter.importManagement(builder, importMgmts, request, this); + } } private DependencyManagement loadDependencyManagement(Dependency dependency, Collection importIds) { @@ -2438,10 +2436,33 @@ private Model injectProfileActivations(Model model, List activations 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); + Model interpolatedModel = + modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems); + if (interpolatedModel != model) { + builder.reset(interpolatedModel); + } + // Parent version interpolation: use sub-builder mutation to avoid + // intermediate immutable Model/Parent rebuilds + if (interpolatedModel.getParent() != null) { + Map map1 = request.getSession().getUserProperties(); + Map map2 = model.getProperties(); + Map map3 = request.getSession().getSystemProperties(); + UnaryOperator cb = Interpolator.chain(map1::get, map2::get, map3::get); + try { + String interpolated = + interpolator.interpolate(interpolatedModel.getParent().getVersion(), cb); + builder.getModifiableParent().version(interpolated); + } catch (Exception e) { + problems.add( + Severity.ERROR, + Version.BASE, + "Failed to interpolate field: " + + interpolatedModel.getParent().getVersion() + + " on class: ", + e); + } } + builder.pomFile(model.getPomFile()); } private Model interpolateModel(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { From e3361f76c32cd1724f6c4727d940b232ad560993 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sun, 2 Aug 2026 00:44:08 +0200 Subject: [PATCH 7/8] Add visitBuilder() to MavenTransformer and builder-accepting SPI overrides Adds visitBuilder(Builder, Model) to the generated MavenTransformer so that fields can be transformed directly on an existing builder, reading from an immutable snapshot, without the intermediate Model materialization and reset that visit(Model) performs. Overrides the builder-accepting default methods in three SPI implementations: - DefaultModelInterpolator: builds a snapshot for consistent property resolution, then uses visitBuilder() instead of visit() + reset() - DefaultModelUrlNormalizer: normalizes URLs via getModifiableScm(), getModifiableDistributionManagement().getModifiableSite() - DefaultDependencyManagementImporter: merges imports via getModifiableDependencyManagement().dependencies() Simplifies DefaultModelBuilder.interpolateModel to call the builder- accepting SPI directly, eliminating the build/reset round-trip. Co-Authored-By: Claude Opus 4.6 --- .../maven/impl/DefaultModelUrlNormalizer.java | 27 ++++--- .../DefaultDependencyManagementImporter.java | 80 ++++++++++++------- .../maven/impl/model/DefaultModelBuilder.java | 24 +++--- .../impl/model/DefaultModelInterpolator.java | 18 ++++- src/mdo/transformer.vm | 19 +++++ 5 files changed, 114 insertions(+), 54 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java index 36bdb746b59a..fe82c6d41990 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java @@ -52,26 +52,35 @@ public Model normalize(Model model, ModelBuilderRequest request) { } Model.Builder builder = Model.newBuilder(model); - builder.url(normalize(model.getUrl())); + normalizeBuilder(builder); + return builder.build(); + } + + @Override + public void normalize(Model.Builder builder, ModelBuilderRequest request) { + normalizeBuilder(builder); + } + + private void normalizeBuilder(Model.Builder builder) { + builder.url(normalize(builder.getUrl())); - Scm scm = model.getScm(); + Scm scm = builder.getScm(); if (scm != null) { - builder.scm(Scm.newBuilder(scm) + builder.getModifiableScm() .url(normalize(scm.getUrl())) .connection(normalize(scm.getConnection())) - .developerConnection(normalize(scm.getDeveloperConnection())) - .build()); + .developerConnection(normalize(scm.getDeveloperConnection())); } - DistributionManagement dist = model.getDistributionManagement(); + DistributionManagement dist = builder.getDistributionManagement(); if (dist != null) { Site site = dist.getSite(); if (site != null) { - builder.distributionManagement(dist.withSite(site.withUrl(normalize(site.getUrl())))); + builder.getModifiableDistributionManagement() + .getModifiableSite() + .url(normalize(site.getUrl())); } } - - return builder.build(); } private String normalize(String url) { 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 828f8568980c..314b6a57f409 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 @@ -56,43 +56,63 @@ public Model importManagement( ModelBuilderRequest request, ModelProblemCollector problems) { if (sources != null && !sources.isEmpty()) { - Map dependencies = new LinkedHashMap<>(); + Map dependencies = + collectDependencies(target.getDependencyManagement(), sources, request, problems); + return target.withDependencyManagement( + target.getDependencyManagement().withDependencies(dependencies.values())); + } + return target; + } + + @Override + public void importManagement( + Model.Builder builder, + List sources, + ModelBuilderRequest request, + ModelProblemCollector problems) { + if (sources != null && !sources.isEmpty()) { + Map dependencies = + collectDependencies(builder.getDependencyManagement(), sources, request, problems); + builder.getModifiableDependencyManagement().dependencies(dependencies.values()); + } + } - DependencyManagement depMgmt = target.getDependencyManagement(); + private Map collectDependencies( + DependencyManagement depMgmt, + List sources, + ModelBuilderRequest request, + ModelProblemCollector problems) { + Map dependencies = new LinkedHashMap<>(); - if (depMgmt != null) { - for (Dependency dependency : depMgmt.getDependencies()) { - dependencies.put(dependency.getManagementKey(), dependency); - } - } else { - depMgmt = DependencyManagement.newInstance(); + if (depMgmt != null) { + for (Dependency dependency : depMgmt.getDependencies()) { + dependencies.put(dependency.getManagementKey(), dependency); } + } - Set directDependencies = new HashSet<>(dependencies.keySet()); - - for (DependencyManagement source : sources) { - for (Dependency dependency : source.getDependencies()) { - String key = dependency.getManagementKey(); - Dependency present = dependencies.putIfAbsent(key, dependency); - if (present != null && !equals(dependency, present) && !directDependencies.contains(key)) { - // TODO: https://issues.apache.org/jira/browse/MNG-8004 - problems.add( - Severity.WARNING, - Version.V40, - "Ignored POM import for: " + toString(dependency) + " as already imported " - + toString(present) + ". Add the conflicting managed dependency directly " - + "to the dependencyManagement section of the POM."); - } - if (present == null && request.isLocationTracking()) { - Dependency updatedDependency = updateWithImportedFrom(dependency, source); - dependencies.put(key, updatedDependency); - } + Set directDependencies = new HashSet<>(dependencies.keySet()); + + for (DependencyManagement source : sources) { + for (Dependency dependency : source.getDependencies()) { + String key = dependency.getManagementKey(); + Dependency present = dependencies.putIfAbsent(key, dependency); + if (present != null && !equals(dependency, present) && !directDependencies.contains(key)) { + // TODO: https://issues.apache.org/jira/browse/MNG-8004 + problems.add( + Severity.WARNING, + Version.V40, + "Ignored POM import for: " + toString(dependency) + " as already imported " + + toString(present) + ". Add the conflicting managed dependency directly " + + "to the dependencyManagement section of the POM."); + } + if (present == null && request.isLocationTracking()) { + Dependency updatedDependency = updateWithImportedFrom(dependency, source); + dependencies.put(key, updatedDependency); } } - - return target.withDependencyManagement(depMgmt.withDependencies(dependencies.values())); } - return target; + + return dependencies; } private String toString(Dependency dependency) { 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 e4a3d79e867b..16d035fae596 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 @@ -2435,34 +2435,30 @@ private Model injectProfileActivations(Model model, List activations } private void interpolateModel(Model.Builder builder, ModelBuilderRequest request, ModelProblemCollector problems) { - Model model = builder.build(); - Model interpolatedModel = - modelInterpolator.interpolateModel(model, model.getProjectDirectory(), request, problems); - if (interpolatedModel != model) { - builder.reset(interpolatedModel); - } + // The model interpolator transforms builder fields in place via visitBuilder, + // avoiding the intermediate Model materialization + reset the SPI default performs. + Path projectDir = builder.getPomFile() != null ? builder.getPomFile().getParent() : null; + modelInterpolator.interpolateModel(builder, projectDir, request, problems); + // Parent version interpolation: use sub-builder mutation to avoid // intermediate immutable Model/Parent rebuilds - if (interpolatedModel.getParent() != null) { + Parent parent = builder.getParent(); + if (parent != null) { Map map1 = request.getSession().getUserProperties(); - Map map2 = model.getProperties(); + Map map2 = builder.getProperties(); Map map3 = request.getSession().getSystemProperties(); UnaryOperator cb = Interpolator.chain(map1::get, map2::get, map3::get); try { - String interpolated = - interpolator.interpolate(interpolatedModel.getParent().getVersion(), cb); + String interpolated = interpolator.interpolate(parent.getVersion(), cb); builder.getModifiableParent().version(interpolated); } catch (Exception e) { problems.add( Severity.ERROR, Version.BASE, - "Failed to interpolate field: " - + interpolatedModel.getParent().getVersion() - + " on class: ", + "Failed to interpolate field: " + parent.getVersion() + " on class: ", e); } } - builder.pomFile(model.getPomFile()); } private Model interpolateModel(Model model, ModelBuilderRequest request, ModelProblemCollector problems) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java index 1932f3fc5515..f86996a1a08a 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java @@ -103,6 +103,22 @@ interface InnerInterpolator { public Model interpolateModel( Model model, Path projectDir, ModelBuilderRequest request, ModelProblemCollector problems) { InnerInterpolator innerInterpolator = createInterpolator(model, projectDir, request, problems); + return newTransformer(innerInterpolator).visit(model); + } + + @Override + public void interpolateModel( + Model.Builder builder, Path projectDir, ModelBuilderRequest request, ModelProblemCollector problems) { + // Build a snapshot for consistent property resolution during interpolation. + // The per-field transform methods read from this immutable target and write + // changed values to the existing builder, avoiding the second Model + // materialization and reset that the SPI default method performs. + Model model = builder.build(); + InnerInterpolator innerInterpolator = createInterpolator(model, projectDir, request, problems); + newTransformer(innerInterpolator).visitBuilder(builder, model); + } + + private MavenTransformer newTransformer(InnerInterpolator innerInterpolator) { return new MavenTransformer(innerInterpolator::interpolate) { @Override protected String transform(String value) { @@ -115,7 +131,7 @@ protected String transform(String value) { } return super.transform(value); } - }.visit(model); + }; } private InnerInterpolator createInterpolator( diff --git a/src/mdo/transformer.vm b/src/mdo/transformer.vm index e1655e15b850..0f4bd3da0de8 100644 --- a/src/mdo/transformer.vm +++ b/src/mdo/transformer.vm @@ -67,6 +67,25 @@ public class ${className} { return transform${root.name}(target); } + /** + * Transforms fields of the given {@code target} directly on the provided {@code builder}, + * avoiding the intermediate immutable model materialization that {@link #visit} performs. + * Each field is read from the immutable {@code target}, transformed, and any changed + * value is written to the existing {@code builder}. + * + * @param builder the existing builder to update in place + * @param target the immutable model to read field values from + */ + public void visitBuilder(${root.name}.Builder builder, ${root.name} target) { + Objects.requireNonNull(builder, "builder cannot be null"); + Objects.requireNonNull(target, "target cannot be null"); +#set ( $rootAllFields = $Helper.xmlFields( $root ) ) + Supplier<${root.name}.Builder> creator = () -> builder; +#foreach ( $field in $rootAllFields ) + transform${field.modelClass.name}_${Helper.capitalise($field.name)}(creator, builder, target); +#end + } + /** * The transformation function. */ From 20afa1ad747678468e9a5de37a451fd0ab624bf6 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Sun, 2 Aug 2026 01:17:30 +0200 Subject: [PATCH 8/8] Thread builder through readEffectiveModel pipeline stages Adds a public merge(Builder, ...) entry point to the generated MavenMerger so callers can merge into an existing builder without the internal Model.newBuilder() + build() round-trip. Overrides the builder-accepting variants in: - DefaultInheritanceAssembler: delegates to merger.merge(Builder, ...) avoiding the intermediate immutable Model allocation - DefaultProfileInjector: iterates profiles and merges directly into the builder, bypassing the WeakHashMap cache (which requires immutable keys) Threads a single Model.Builder through the readEffectiveModel pipeline from inheritance assembly through mixins, normalization, profile injection, interpolation, URL normalization, to the final build(). Eliminates the intermediate build()/newBuilder() pairs that previously separated these stages. Co-Authored-By: Claude Opus 4.6 --- .../model/DefaultInheritanceAssembler.java | 21 +++++-- .../maven/impl/model/DefaultModelBuilder.java | 55 ++++++++++++------- .../impl/model/DefaultProfileInjector.java | 21 +++++++ .../maven/impl/model/MavenModelMerger.java | 13 +++++ src/mdo/merger.vm | 23 ++++++++ 5 files changed, 109 insertions(+), 24 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java index b6d7230b4f41..69a060bb5aa3 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java @@ -66,11 +66,24 @@ public DefaultInheritanceAssembler(MavenMerger merger) { @Override public Model assembleModelInheritance( Model child, Model parent, ModelBuilderRequest request, ModelProblemCollector problems) { - Map hints = new HashMap<>(); + Map context = createContext(child, parent); + return merger.merge(child, parent, false, context); + } + + @Override + public void assembleModelInheritance( + Model.Builder childBuilder, Model parent, ModelBuilderRequest request, ModelProblemCollector problems) { + Model child = childBuilder.build(); + Map context = createContext(child, parent); + merger.merge(childBuilder, child, parent, false, context); + } + + private Map createContext(Model child, Model parent) { + Map context = new HashMap<>(); String childPath = child.getProperties().getOrDefault(CHILD_DIRECTORY_PROPERTY, child.getArtifactId()); - hints.put(CHILD_DIRECTORY, childPath); - hints.put(MavenModelMerger.CHILD_PATH_ADJUSTMENT, getChildPathAdjustment(child, parent, childPath)); - return merger.merge(child, parent, false, hints); + context.put(CHILD_DIRECTORY, childPath); + context.put(MavenModelMerger.CHILD_PATH_ADJUSTMENT, getChildPathAdjustment(child, parent, childPath)); + return context; } /** 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 16d035fae596..ada763c6e06a 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 @@ -1495,25 +1495,34 @@ private Model readEffectiveModel() throws ModelBuilderException { inputModel = inputModel.withParent(inputModel.getParent().withRelativePath(relPath)); } - Model model = inheritanceAssembler.assembleModelInheritance(inputModel, parentModel, request, this); - - // Mixins - for (Mixin mixin : model.getMixins()) { - Model parent = resolveParent(model, mixin, profileActivationContext, parentChain); - // Merge mixin into model - model = inheritanceAssembler.assembleModelInheritance(model, parent, request, this); - // Ensure mixin properties override any previously inherited properties - // This is necessary because normal inheritance gives child precedence, but for mixins - // we want the mixin to take precedence over inherited parent properties - Map mergedProperties = new java.util.HashMap<>(model.getProperties()); - mergedProperties.putAll(parent.getProperties()); - model = model.withProperties(mergedProperties); + // Thread a single builder from inheritance assembly through the final build(). + // Each stage that needs an immutable snapshot calls builder.build() internally. + Model.Builder builder = Model.newBuilder(inputModel, false); + inheritanceAssembler.assembleModelInheritance(builder, parentModel, request, this); + + // Mixins — the mixin list is fixed by the child POM, so read it from the + // builder's base model. Each iteration needs a snapshot for resolveParent + // and assembleModelInheritance (which builds one internally anyway). + List mixins = builder.getBuiltMixins(); + if (!mixins.isEmpty()) { + for (Mixin mixin : mixins) { + Model snapshot = builder.build(); + Model parent = resolveParent(snapshot, mixin, profileActivationContext, parentChain); + inheritanceAssembler.assembleModelInheritance(builder, parent, request, this); + // Ensure mixin properties override any previously inherited properties + // This is necessary because normal inheritance gives child precedence, but for mixins + // we want the mixin to take precedence over inherited parent properties + Map mergedProperties = new java.util.HashMap<>(builder.getProperties()); + mergedProperties.putAll(parent.getProperties()); + builder.properties(mergedProperties); + } } // model normalization - model = modelNormalizer.mergeDuplicates(model, request, this); + modelNormalizer.mergeDuplicates(builder, request, this); - // profile activation + // profile activation — needs an immutable snapshot for the activation context + Model model = builder.build(); profileActivationContext.setModel(model); // Activate profiles from the input model (before inheritance) to get only local profiles @@ -1523,17 +1532,23 @@ private Model readEffectiveModel() throws ModelBuilderException { // profile injection - inject all profiles (local + inherited) into the model List activePomProfiles = getActiveProfiles(model.getProfiles(), profileActivationContext); - 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); + // Use the builder's getters to compute the id without an intermediate build() + String groupId = builder.getGroupId(); + if (groupId == null && builder.getParent() != null) { + groupId = builder.getParent().getGroupId(); + } + String version = builder.getVersion(); + if (version == null && builder.getParent() != null) { + version = builder.getParent().getVersion(); + } + addActivePomProfiles( + ModelProblemUtils.toId(groupId, builder.getArtifactId(), version), localActivePomProfiles); // model interpolation + normalization + url normalization via builder - builder = Model.newBuilder(model, false); interpolateModel(builder, request, this); // model normalization diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java index 206e8d4834c8..82a996f64752 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultProfileInjector.java @@ -70,6 +70,27 @@ public Model injectProfiles( return result == KEY ? model : result; } + @Override + public void injectProfiles( + Model.Builder builder, + List profiles, + ModelBuilderRequest request, + ModelProblemCollector problems) { + for (Profile profile : profiles) { + if (profile != null) { + Model model = builder.build(); + merger.mergeModelBase(builder, model, profile); + + if (profile.getBuild() != null) { + Build build = model.getBuild() != null ? model.getBuild() : Build.newInstance(); + Build.Builder bbuilder = Build.newBuilder(build); + merger.mergeBuildBase(bbuilder, build, profile.getBuild()); + builder.build(bbuilder.build()); + } + } + } + } + private Model doInjectProfiles(Model model, List profiles) { Model orgModel = model; for (Profile profile : profiles) { diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java index b9f259cced1c..717d4a83aa94 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/MavenModelMerger.java @@ -73,6 +73,11 @@ public Model merge(Model target, Model source, boolean sourceDominant, Map return super.merge(target, source, sourceDominant, hints); } + @Override + public void merge(Model.Builder builder, Model target, Model source, boolean sourceDominant, Map hints) { + super.merge(builder, target, source, sourceDominant, hints); + } + @Override protected Model mergeModel(Model target, Model source, boolean sourceDominant, Map context) { context.put(ARTIFACT_ID, target.getArtifactId()); @@ -80,6 +85,14 @@ protected Model mergeModel(Model target, Model source, boolean sourceDominant, M return super.mergeModel(target, source, sourceDominant, context); } + @Override + protected void mergeModel( + Model.Builder builder, Model target, Model source, boolean sourceDominant, Map context) { + context.put(ARTIFACT_ID, target.getArtifactId()); + + super.mergeModel(builder, target, source, sourceDominant, context); + } + @Override protected void mergeModel_Name( Model.Builder builder, Model target, Model source, boolean sourceDominant, Map context) { diff --git a/src/mdo/merger.vm b/src/mdo/merger.vm index 2ad2e20685d4..e5e5f9f41244 100644 --- a/src/mdo/merger.vm +++ b/src/mdo/merger.vm @@ -88,6 +88,29 @@ public class ${className} { return merge${root.name}(target, source, sourceDominant, context); } + /** + * Builder-accepting variant that merges the source into an existing builder, + * avoiding the intermediate immutable object allocation that {@link #merge} performs. + * + * @param builder The builder to merge into, must not be {@code null}. + * @param target The immutable snapshot to read current field values from, must not be {@code null}. + * @param source The (read-only) source object to merge from, may be {@code null}. + * @param sourceDominant Whether the source provides the dominant data. + * @param hints Domain-specific hints, may be {@code null}. + */ + public void merge(${root.name}.Builder builder, ${root.name} target, ${root.name} source, boolean sourceDominant, Map hints) { + Objects.requireNonNull(builder, "builder cannot be null"); + Objects.requireNonNull(target, "target cannot be null"); + if (source == null) { + return; + } + Map context = new HashMap<>(); + if (hints != null) { + context.putAll(hints); + } + merge${root.name}(builder, target, source, sourceDominant, context); + } + #foreach ( $class in $model.allClasses ) #if ( $class.name != "InputSource" && $class.name != "InputLocation" ) #set ( $ancestors = $Helper.ancestors( $class ) )