(maven) branch optimize-model-builder-pipeline-fix updated: Add withImportedFrom() copy method and replace CopyOnWriteArraySet

[gn...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch optimize-model-builder-pipeline-fix
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/optimize-model-builder-pipeline-fix by this push:
     new d30efb592e Add withImportedFrom() copy method and replace CopyOnWriteArraySet
d30efb592e is described below

commit d30efb592eb4acf333842c14010c029c7432cfa8
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Sat Aug 1 11:24:54 2026 +0200

    Add withImportedFrom() copy method and replace CopyOnWriteArraySet
    
    JFR profiling identified two hotspots:
    
    1. updateWithImportedFrom (35% of CPU): Rebuilt entire Dependency objects
       through Builder+forceCopy+build+interning just to set the importedFrom
       metadata field. Added a lightweight withImportedFrom(InputLocation) method
       and private copy constructor to model.vm that shares all model fields by
       reference and bypasses Builder wrapping and object pool interning.
       Result: 88% reduction in updateWithImportedFrom CPU samples.
    
    2. CopyOnWriteArraySet (6.6% of CPU): ConsumerPomArtifactTransformer used
       CopyOnWriteArraySet<Path> for deferDeleteFile(), causing O(n) linear
       scan + array copy per add. With 4383 modules this is O(n^2). Replaced
       with ConcurrentHashMap.newKeySet() for O(1) amortized add.
       Result: 100% eliminated from CPU profile.
    
    Overall: 32% reduction in CPU samples (1028 -> 701) per JFR comparison.
    
    Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
 .../impl/ConsumerPomArtifactTransformer.java       |  4 +--
 .../model/DefaultDependencyManagementImporter.java | 10 +++----
 src/mdo/model.vm                                   | 32 ++++++++++++++++++++++
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformer.java b/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformer.java
index f32c962886..2c15edde61 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformer.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/internal/transformation/impl/ConsumerPomArtifactTransformer.java
@@ -33,7 +33,7 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.maven.api.feature.Features;
 import org.apache.maven.api.model.Model;
@@ -62,7 +62,7 @@ class ConsumerPomArtifactTransformer extends TransformerSupport {
 
     private static final String BUILD_POM_CLASSIFIER = "build";
 
-    private final Set<Path> toDelete = new CopyOnWriteArraySet<>();
+    private final Set<Path> toDelete = ConcurrentHashMap.newKeySet();
 
     private final PomBuilder builder;
 
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 828f856898..8a1858abe6 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,11 +167,9 @@ 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();
+            // Use withImportedFrom() — a lightweight copy that shares all model fields
+            // by reference and bypasses Builder wrapping and object pool interning.
+            return dependency.withImportedFrom(bomLocation);
         }
 
         while (dependencySource.getImportedFrom() != null) {
@@ -187,6 +185,6 @@ static Dependency updateWithImportedFrom(Dependency dependency, DependencyManage
 
         // We modify the input location that is used for the whole file.
         // This is likely correct because the POM hierarchy applies to the whole POM, not just one dependency.
-        return Dependency.newBuilder(dependency, true).importedFrom(bomLocation).build();
+        return dependency.withImportedFrom(bomLocation);
     }
 }
diff --git a/src/mdo/model.vm b/src/mdo/model.vm
index a034c33e55..9afddcf92d 100644
--- a/src/mdo/model.vm
+++ b/src/mdo/model.vm
@@ -318,6 +318,38 @@ public class ${class.name}
         return importedFrom;
     }
 
+    /**
+     * Creates a copy of this object with a different {@code importedFrom} value.
+     * This is a lightweight operation that bypasses the Builder and object pool
+     * interning, suitable for metadata-only changes that don't affect model field
+     * values (all model fields are shared by reference with the original).
+     *
+     * @param importedFrom the new imported-from location
+     * @return a new instance with the updated importedFrom, or {@code this} if unchanged
+     */
+    @Nonnull
+    public ${class.name} withImportedFrom(InputLocation importedFrom) {
+        return this.importedFrom == importedFrom ? this : new ${class.name}(this, importedFrom);
+    }
+
+    /**
+     * Copy constructor for {@link #withImportedFrom(InputLocation)}.
+     * Copies all fields by reference (they are immutable) and sets
+     * a new {@code importedFrom}. Bypasses Builder and object pool.
+     */
+    private ${class.name}(${class.name} base, InputLocation importedFrom) {
+    #if ( $class == $root )
+        this.namespaceUri = base.namespaceUri;
+        this.modelEncoding = base.modelEncoding;
+    #end
+    #foreach ( $field in $class.getFields($version) )
+        this.${field.name} = base.${field.name};
+    #end
+        this.locations = base.locations;
+        this.locationsHashCode = base.locationsHashCode;
+        this.importedFrom = importedFrom;
+    }
+
     #end
     /**
      * Creates a new builder with this object as the basis.