You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/07/25 07:05:27 UTC
(maven) branch perf/reduce-immutable-model-allocations created (now 5ef63b4d72)
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a change to branch perf/reduce-immutable-model-allocations
in repository https://gitbox.apache.org/repos/asf/maven.git
at 5ef63b4d72 Reduce unnecessary immutable object allocations in model building
This branch includes the following new commits:
new 5ef63b4d72 Reduce unnecessary immutable object allocations in model building
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(maven) 01/01: Reduce unnecessary immutable object allocations in model building
Posted by gn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch perf/reduce-immutable-model-allocations
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 5ef63b4d72268e8b93ec66cfbd33949f0a610d6c
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Sat Jul 25 09:05:14 2026 +0200
Reduce unnecessary immutable object allocations in model building
The immutable model pattern in Maven 4 creates excessive intermediate
objects during effective model construction. This change addresses the
root causes across 7 optimization phases:
- merger.vm: Fix false-positive change detection for string, boolean,
int, and composite fields that defeated Builder.build() short-circuit
- merger.vm: Add identity checks for list merge results before setting
on builder
- model.vm: Replace Stream.concat().collect() in computeLocations()
with direct HashMap merge to avoid Stream pipeline allocations
- model.vm: Skip ImmutableCollections.copy() in constructor when field
value comes unchanged from the base object
- DefaultModelNormalizer: Only create builder and build when changes
are actually detected
- DefaultProfileInjector: Guard sub-object rebuilds with identity check
- DefaultModelBuilder: Replace nested with*() copy-on-write chains with
single-builder patterns
- DefaultPluginConfigurationExpander: Skip model/build rebuilds when no
plugins have configuration to expand
- ImmutableCollections: Recognize JDK unmodifiable maps to avoid
redundant wrapping
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../impl/DefaultPluginConfigurationExpander.java | 42 +++++++++++++---
.../maven/impl/model/DefaultModelBuilder.java | 56 ++++++++++++++--------
.../maven/impl/model/DefaultModelNormalizer.java | 55 ++++++++++++++-------
.../maven/impl/model/DefaultProfileInjector.java | 5 +-
src/mdo/java/ImmutableCollections.java | 6 +++
src/mdo/merger.vm | 51 +++++++++++++-------
src/mdo/model.vm | 15 ++++--
7 files changed, 164 insertions(+), 66 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 fc159b8ccd..89aad11aae 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,19 +48,47 @@ public class DefaultPluginConfigurationExpander implements PluginConfigurationEx
public Model expandPluginConfiguration(Model model, ModelBuilderRequest request, ModelProblemCollector problems) {
Build build = model.getBuild();
if (build != null) {
- build = build.withPlugins(expandPlugin(build.getPlugins()));
+ List<Plugin> expandedPlugins = expandPlugin(build.getPlugins());
PluginManagement pluginManagement = build.getPluginManagement();
- if (pluginManagement != null) {
- build = build.withPluginManagement(
- pluginManagement.withPlugins(expandPlugin(pluginManagement.getPlugins())));
+ List<Plugin> expandedMgmtPlugins =
+ pluginManagement != null ? expandPlugin(pluginManagement.getPlugins()) : null;
+
+ boolean buildModified = expandedPlugins != build.getPlugins()
+ || (expandedMgmtPlugins != null && expandedMgmtPlugins != pluginManagement.getPlugins());
+
+ if (buildModified) {
+ Build.Builder bb = Build.newBuilder(build);
+ if (expandedPlugins != build.getPlugins()) {
+ bb.plugins(expandedPlugins);
+ }
+ if (expandedMgmtPlugins != null && expandedMgmtPlugins != pluginManagement.getPlugins()) {
+ bb.pluginManagement(PluginManagement.newBuilder(pluginManagement)
+ .plugins(expandedMgmtPlugins)
+ .build());
+ }
+ build = bb.build();
}
- model = model.withBuild(build);
}
Reporting reporting = model.getReporting();
+ List<ReportPlugin> expandedReportPlugins = null;
if (reporting != null) {
- expandReport(reporting.getPlugins());
+ expandedReportPlugins = expandReport(reporting.getPlugins());
+ }
+ boolean modelModified = build != model.getBuild()
+ || (expandedReportPlugins != null && expandedReportPlugins != reporting.getPlugins());
+ if (modelModified) {
+ Model.Builder mb = Model.newBuilder(model);
+ if (build != model.getBuild()) {
+ mb.build(build);
+ }
+ if (expandedReportPlugins != null && expandedReportPlugins != reporting.getPlugins()) {
+ mb.reporting(Reporting.newBuilder(reporting)
+ .plugins(expandedReportPlugins)
+ .build());
+ }
+ return mb.build();
}
- return model.withBuild(build);
+ return model;
}
private List<Plugin> expandPlugin(List<Plugin> oldPlugins) {
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 bc50978f4e..7cba8a6fd3 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
@@ -1000,10 +1000,12 @@ void buildEffectiveModel(Collection<String> importIds) throws ModelBuilderExcept
// Set the default relative path for the parent in the file model
if (result.getFileModel().getParent() != null
&& result.getFileModel().getParent().getRelativePath() == null) {
- result.setFileModel(result.getFileModel()
- .withParent(result.getFileModel()
- .getParent()
- .withRelativePath(resultModel.getParent().getRelativePath())));
+ Model fileModel = result.getFileModel();
+ result.setFileModel(Model.newBuilder(fileModel)
+ .parent(Parent.newBuilder(fileModel.getParent())
+ .relativePath(resultModel.getParent().getRelativePath())
+ .build())
+ .build());
}
// effective model validation
@@ -1420,7 +1422,11 @@ private Model readEffectiveModel() throws ModelBuilderException {
} else {
relPath = "..";
}
- inputModel = inputModel.withParent(inputModel.getParent().withRelativePath(relPath));
+ inputModel = Model.newBuilder(inputModel)
+ .parent(Parent.newBuilder(inputModel.getParent())
+ .relativePath(relPath)
+ .build())
+ .build();
}
Model model = inheritanceAssembler.assembleModelInheritance(inputModel, parentModel, request, this);
@@ -1619,11 +1625,13 @@ Model doReadFileModel(Set<Path> activeModelReads) throws ModelBuilderException {
&& (version == null
|| version.equals(parentVersion)
|| versionContainsExpression)) {
- model = model.withParent(parent.with()
- .groupId(parentGroupId)
- .artifactId(parentArtifactId)
- .version(parentVersion)
- .build());
+ model = Model.newBuilder(model)
+ .parent(Parent.newBuilder(parent)
+ .groupId(parentGroupId)
+ .artifactId(parentArtifactId)
+ .version(parentVersion)
+ .build())
+ .build();
} else {
mismatchRelativePathAndGA(model, parentGroupId, parentArtifactId);
}
@@ -1951,12 +1959,15 @@ protected void mergeModel_Subprojects(
getActiveProfiles(parent.getProfiles(), childProfileActivationContext);
// Inject profiles into parent model
- Model injectedParentModel = profileInjector
- .injectProfiles(parent, parentActivePomProfiles, request, this)
- .withProfiles(List.of()); // Remove profiles after injection to avoid double-processing
+ Model injectedParentModel = profileInjector.injectProfiles(parent, parentActivePomProfiles, request, this);
+ // Remove profiles and parent after injection using a single builder
+ injectedParentModel = Model.newBuilder(injectedParentModel)
+ .profiles(List.of())
+ .parent(null)
+ .build();
// Note: addActivePomProfiles() will be called by the caller for cache miss case
- return new ParentModelWithProfiles(injectedParentModel.withParent(null), parentActivePomProfiles);
+ return new ParentModelWithProfiles(injectedParentModel, parentActivePomProfiles);
}
private Model importDependencyManagement(Model model, Collection<String> importIds) {
@@ -2332,10 +2343,15 @@ private Model interpolateModel(Model model, ModelBuilderRequest request, ModelPr
Map<String, String> map3 = request.getSession().getSystemProperties();
UnaryOperator<String> cb = Interpolator.chain(map1::get, map2::get, map3::get);
try {
- String interpolated =
- interpolator.interpolate(interpolatedModel.getParent().getVersion(), cb);
- interpolatedModel = interpolatedModel.withParent(
- interpolatedModel.getParent().withVersion(interpolated));
+ Parent parent = interpolatedModel.getParent();
+ String interpolated = interpolator.interpolate(parent.getVersion(), cb);
+ if (interpolated != parent.getVersion()) {
+ interpolatedModel = Model.newBuilder(interpolatedModel)
+ .parent(Parent.newBuilder(parent)
+ .version(interpolated)
+ .build())
+ .build();
+ }
} catch (Exception e) {
problems.add(
Severity.ERROR,
@@ -2346,7 +2362,9 @@ private Model interpolateModel(Model model, ModelBuilderRequest request, ModelPr
e);
}
}
- interpolatedModel = interpolatedModel.withPomFile(model.getPomFile());
+ if (interpolatedModel.getPomFile() != model.getPomFile()) {
+ interpolatedModel = interpolatedModel.withPomFile(model.getPomFile());
+ }
return interpolatedModel;
}
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 f36918f770..27cf9e2230 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,9 @@ public class DefaultModelNormalizer implements ModelNormalizer {
@Override
public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProblemCollector problems) {
- Model.Builder builder = Model.newBuilder(model);
-
+ boolean modified = false;
Build build = model.getBuild();
+ Build newBuild = null;
if (build != null) {
List<Plugin> plugins = build.getPlugins();
Map<Object, Plugin> normalized = new LinkedHashMap<>(plugins.size() * 2);
@@ -64,8 +64,8 @@ public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProb
}
if (plugins.size() != normalized.size()) {
- builder.build(
- Build.newBuilder(build).plugins(normalized.values()).build());
+ newBuild = Build.newBuilder(build).plugins(normalized.values()).build();
+ modified = true;
}
}
@@ -83,11 +83,18 @@ public Model mergeDuplicates(Model model, ModelBuilderRequest request, ModelProb
normalized.put(dependency.getManagementKey(), dependency);
}
- if (dependencies.size() != normalized.size()) {
- builder.dependencies(normalized.values());
+ boolean depsModified = dependencies.size() != normalized.size();
+ if (modified || depsModified) {
+ Model.Builder builder = Model.newBuilder(model);
+ if (newBuild != null) {
+ builder.build(newBuild);
+ }
+ if (depsModified) {
+ builder.dependencies(normalized.values());
+ }
+ return builder.build();
}
-
- return builder.build();
+ return model;
}
/**
@@ -102,24 +109,36 @@ public Plugin mergePlugin(Plugin target, Plugin source) {
@Override
public Model injectDefaultValues(Model model, ModelBuilderRequest request, ModelProblemCollector problems) {
- Model.Builder builder = Model.newBuilder(model);
+ List<Dependency> newDeps = injectList(model.getDependencies(), this::injectDependency);
- builder.dependencies(injectList(model.getDependencies(), this::injectDependency));
Build build = model.getBuild();
+ Build newBuild = null;
if (build != null) {
- Build newBuild = Build.newBuilder(build)
- .plugins(injectList(build.getPlugins(), this::injectPlugin))
- .build();
- builder.build(newBuild != build ? newBuild : null);
+ List<Plugin> newPlugins = injectList(build.getPlugins(), this::injectPlugin);
+ if (newPlugins != null) {
+ newBuild = Build.newBuilder(build).plugins(newPlugins).build();
+ }
}
- return builder.build();
+ if (newDeps != null || newBuild != null) {
+ Model.Builder builder = Model.newBuilder(model);
+ if (newDeps != null) {
+ builder.dependencies(newDeps);
+ }
+ if (newBuild != null) {
+ builder.build(newBuild);
+ }
+ return builder.build();
+ }
+ return model;
}
private Plugin injectPlugin(Plugin p) {
- return Plugin.newBuilder(p)
- .dependencies(injectList(p.getDependencies(), this::injectDependency))
- .build();
+ List<Dependency> newDeps = injectList(p.getDependencies(), this::injectDependency);
+ if (newDeps != null) {
+ return Plugin.newBuilder(p).dependencies(newDeps).build();
+ }
+ return p;
}
private Dependency injectDependency(Dependency d) {
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 206e8d4834..1065f7d11a 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
@@ -81,7 +81,10 @@ private Model doInjectProfiles(Model model, List<Profile> profiles) {
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());
+ Build newBuild = bbuilder.build();
+ if (newBuild != build) {
+ builder.build(newBuild);
+ }
}
model = builder.build();
diff --git a/src/mdo/java/ImmutableCollections.java b/src/mdo/java/ImmutableCollections.java
index 4b69b4d3f5..6dc8423793 100644
--- a/src/mdo/java/ImmutableCollections.java
+++ b/src/mdo/java/ImmutableCollections.java
@@ -120,6 +120,12 @@ static <K, V> Map<K, V> copy(Map<K, V> map) {
Map.Entry<K, V> entry = map.entrySet().iterator().next();
return singletonMap(entry.getKey(), entry.getValue());
default:
+ // Check if this is already an immutable JDK map (from Map.of(), Map.copyOf(), etc.)
+ // Those throw UnsupportedOperationException on put() and don't need copying
+ String className = map.getClass().getName();
+ if (className.startsWith("java.util.ImmutableCollections$")) {
+ return map;
+ }
return new MapN<>(map);
}
}
diff --git a/src/mdo/merger.vm b/src/mdo/merger.vm
index 984232d022..547a13f509 100644
--- a/src/mdo/merger.vm
+++ b/src/mdo/merger.vm
@@ -116,14 +116,20 @@ public class ${className} {
#if ( $field.type == "String" )
String src = source.get${capField}();
String tgt = target.get${capField}();
- if (src != null && (sourceDominant || tgt == null)) {
+ if (src != null && (sourceDominant || tgt == null) && src != tgt) {
builder.${field.name}(src);
#if ( $locationTracking )
builder.location("${field.name}", source.getLocation("${field.name}"));
#end
}
#elseif ( $field.type == "java.util.List" && $field.to == "String" && $field.multiplicity == "*" )
- builder.${field.name}(merge(target.get${capField}(), source.get${capField}(), sourceDominant, e -> e));
+ {
+ List<String> tgt${capField} = target.get${capField}();
+ List<String> merged${capField} = merge(tgt${capField}, source.get${capField}(), sourceDominant, e -> e);
+ if (merged${capField} != tgt${capField}) {
+ builder.${field.name}(merged${capField});
+ }
+ }
#elseif ( $field.type == "java.util.Properties" && $field.to == "String" && $field.multiplicity == "*" )
Map<String, String> src = source.get${capField}();
if (!src.isEmpty()) {
@@ -151,21 +157,27 @@ public class ${className} {
tgt = ${field.to}.newInstance(false);
}
${field.to} merged = merge${field.to}(tgt, src, sourceDominant, context);
- builder.${field.name}(merged);
+ if (merged != target.get${capField}()) {
+ builder.${field.name}(merged);
#if ( $locationTracking )
- if (target.get${capField}() == null) {
- builder.location("${field.name}", source.getLocation("${field.name}"));
- } else if (merged != tgt) {
- builder.location("${field.name}", new InputLocation(-1, -1));
- }
+ if (target.get${capField}() == null) {
+ builder.location("${field.name}", source.getLocation("${field.name}"));
+ } else {
+ builder.location("${field.name}", new InputLocation(-1, -1));
+ }
#end
+ }
}
#elseif ( $field.to && $field.multiplicity == "*" )
- if (deepMerge) {
- builder.${field.name}(merge(target.get${capField}(), source.get${capField}(), get${field.to}Key(),
- (t, s) -> merge${field.to}(t, s, sourceDominant, context)));
- } else {
- builder.${field.name}(merge(target.get${capField}(), source.get${capField}(), sourceDominant, get${field.to}Key()));
+ {
+ List<${field.to}> tgt${capField} = target.get${capField}();
+ List<${field.to}> merged${capField} = deepMerge
+ ? merge(tgt${capField}, source.get${capField}(), get${field.to}Key(),
+ (t, s) -> merge${field.to}(t, s, sourceDominant, context))
+ : merge(tgt${capField}, source.get${capField}(), sourceDominant, get${field.to}Key());
+ if (merged${capField} != tgt${capField}) {
+ builder.${field.name}(merged${capField});
+ }
}
#elseif ( $field.type == "DOM" )
XmlNode src = source.getConfiguration();
@@ -189,13 +201,20 @@ public class ${className} {
}
}
#elseif ($field.type == "boolean")
- if (sourceDominant) {
+ if (sourceDominant && source.is${capField}() != target.is${capField}()) {
builder.${field.name}(source.is${capField}());
}
- #elseif ($field.type == "int" || $field.type == "java.nio.file.Path")
- if (sourceDominant) {
+ #elseif ($field.type == "int")
+ if (sourceDominant && source.get${capField}() != target.get${capField}()) {
builder.${field.name}(source.get${capField}());
}
+ #elseif ($field.type == "java.nio.file.Path")
+ if (sourceDominant) {
+ ${field.type} srcPath = source.get${capField}();
+ if (!Objects.equals(srcPath, target.get${capField}())) {
+ builder.${field.name}(srcPath);
+ }
+ }
#else
// TODO: type=${field.type} to=${field.to} multiplicity=${field.multiplicity}
#end
diff --git a/src/mdo/model.vm b/src/mdo/model.vm
index 5bcfa7246d..3ef4acf673 100644
--- a/src/mdo/model.vm
+++ b/src/mdo/model.vm
@@ -166,7 +166,9 @@ public class ${class.name}
#end
#foreach ( $field in $class.getFields($version) )
#if ( $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));
+ this.${field.name} = builder.${field.name} != null
+ ? ImmutableCollections.copy(builder.${field.name})
+ : (builder.base != null ? builder.base.${field.name} : #if($field.type == "java.util.List")List.of()#else Map.of()#end);
#else
#if ( $field.type == "boolean" || $field.type == "int" )
this.${field.name} = builder.${field.name} != null ? builder.${field.name} : (builder.base != null ? builder.base.${field.name} : ${field.defaultValue});
@@ -510,14 +512,17 @@ public class ${class.name}
Map<Object, InputLocation> newlocs = locations != null ? locations : Map.of();
Map<Object, InputLocation> oldlocs = base != null ? base.locations : Map.of();
if (newlocs.isEmpty()) {
- return Map.copyOf(oldlocs);
+ // oldlocs is already immutable from a previous build()
+ 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));
+ // Merge: oldlocs as base, newlocs wins on conflict
+ Map<Object, InputLocation> merged = new HashMap<>(oldlocs.size() + newlocs.size());
+ merged.putAll(oldlocs);
+ merged.putAll(newlocs);
+ return Map.copyOf(merged);
}
#end
}