From 2e4e0f045fba0417c07f1e60518eee6d6ef579a8 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 14 Jan 2025 18:34:33 +0100 Subject: [PATCH 1/4] [MNG-8520] Add cache for model resolution during project building --- .../maven/api/services/model/ModelCache.java | 10 +++- .../impl/model/DefaultModelBuilder.java | 60 +++++++++++++++++-- .../impl/model/DefaultModelCache.java | 32 +++++++--- 3 files changed, 87 insertions(+), 15 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java b/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java index bae427bebfbf..a6e098444798 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java @@ -18,8 +18,10 @@ */ package org.apache.maven.api.services.model; +import java.util.List; import java.util.function.Supplier; +import org.apache.maven.api.RemoteRepository; import org.apache.maven.api.annotations.Experimental; import org.apache.maven.api.annotations.ThreadSafe; import org.apache.maven.api.services.Source; @@ -40,7 +42,13 @@ @ThreadSafe public interface ModelCache { - T computeIfAbsent(String groupId, String artifactId, String version, String tag, Supplier data); + T computeIfAbsent( + List repositories, + String groupId, + String artifactId, + String version, + String tag, + Supplier data); T computeIfAbsent(Source path, String tag, Supplier data); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java index 49576268323c..93b9f8b4af7e 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java @@ -40,6 +40,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.function.UnaryOperator; import java.util.regex.Matcher; @@ -129,6 +130,7 @@ public class DefaultModelBuilder implements ModelBuilder { private static final String FILE = "file"; private static final String IMPORT = "import"; private static final String PARENT = "parent"; + private static final String MODEL = "model"; private final Logger logger = LoggerFactory.getLogger(getClass()); @@ -1025,7 +1027,8 @@ Model resolveAndReadParentExternally(Model childModel, DefaultProfileActivationC modelSource = resolveReactorModel(parent.getGroupId(), parent.getArtifactId(), parent.getVersion()); if (modelSource == null) { AtomicReference modified = new AtomicReference<>(); - modelSource = modelResolver.resolveModel(request.getSession(), repositories, parent, modified); + modelSource = new CachingModelResolver() + .resolveModel(request.getSession(), repositories, parent, modified); if (modified.get() != null) { parent = modified.get(); } @@ -1605,6 +1608,7 @@ private DependencyManagement loadDependencyManagement(Dependency dependency, Col } Model importModel = cache( + repositories, groupId, artifactId, version, @@ -1641,8 +1645,8 @@ private Model doLoadDependencyManagement( try { importSource = resolveReactorModel(groupId, artifactId, version); if (importSource == null) { - importSource = modelResolver.resolveModel( - request.getSession(), repositories, dependency, new AtomicReference<>()); + importSource = new CachingModelResolver() + .resolveModel(request.getSession(), repositories, dependency, new AtomicReference<>()); } } catch (ModelBuilderException | ModelResolverException e) { StringBuilder buffer = new StringBuilder(256); @@ -1711,8 +1715,14 @@ ModelSource resolveReactorModel(String groupId, String artifactId, String versio return null; } - private T cache(String groupId, String artifactId, String version, String tag, Supplier supplier) { - return cache.computeIfAbsent(groupId, artifactId, version, tag, supplier); + private T cache( + List repositories, + String groupId, + String artifactId, + String version, + String tag, + Supplier supplier) { + return cache.computeIfAbsent(repositories, groupId, artifactId, version, tag, supplier); } private T cache(Source source, String tag, Supplier supplier) throws ModelBuilderException { @@ -1801,6 +1811,46 @@ private String transformPath(String path, ActivationFile target, String location } return profiles.stream().map(new ProfileInterpolator()).toList(); } + + record ModelResolverResult(ModelSource source, String resolvedVersion) {} + + class CachingModelResolver implements ModelResolver { + @Override + public ModelSource resolveModel( + Session session, + List repositories, + String groupId, + String artifactId, + String version, + String classifier, + Consumer resolvedVersion) + throws ModelResolverException { + ModelResolverResult result = cache.computeIfAbsent( + repositories, + groupId, + artifactId, + classifier != null && !classifier.isEmpty() ? version + ":" + classifier : version, + MODEL, + () -> doResolve(session, repositories, groupId, artifactId, version, classifier)); + if (result.resolvedVersion != null) { + resolvedVersion.accept(result.resolvedVersion); + } + return result.source; + } + + ModelResolverResult doResolve( + Session session, + List repositories, + String groupId, + String artifactId, + String version, + String classifier) { + AtomicReference resolvedVersion = new AtomicReference<>(); + ModelSource source = modelResolver.resolveModel( + session, repositories, groupId, artifactId, version, classifier, resolvedVersion::set); + return new ModelResolverResult(source, resolvedVersion.get()); + } + } } @SuppressWarnings("deprecation") diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java index 47a0469fef14..50529a16531c 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java @@ -18,11 +18,13 @@ */ package org.apache.maven.internal.impl.model; +import java.util.List; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Supplier; +import org.apache.maven.api.RemoteRepository; import org.apache.maven.api.services.Source; import org.apache.maven.api.services.model.ModelCache; @@ -46,8 +48,14 @@ private DefaultModelCache(ConcurrentMap> cache) { @Override @SuppressWarnings({"unchecked"}) - public T computeIfAbsent(String groupId, String artifactId, String version, String tag, Supplier data) { - return (T) computeIfAbsent(new GavCacheKey(groupId, artifactId, version, tag), data); + public T computeIfAbsent( + List repositories, + String groupId, + String artifactId, + String version, + String tag, + Supplier data) { + return (T) computeIfAbsent(new RgavCacheKey(repositories, groupId, artifactId, version, tag), data); } @Override @@ -65,7 +73,9 @@ protected Object computeIfAbsent(Object key, Supplier data) { return cache.computeIfAbsent(key, k -> new CachingSupplier<>(data)).get(); } - static class GavCacheKey { + static class RgavCacheKey { + + private final List repositories; private final String gav; @@ -73,14 +83,16 @@ static class GavCacheKey { private final int hash; - GavCacheKey(String groupId, String artifactId, String version, String tag) { - this(gav(groupId, artifactId, version), tag); + RgavCacheKey( + List repositories, String groupId, String artifactId, String version, String tag) { + this(repositories, gav(groupId, artifactId, version), tag); } - GavCacheKey(String gav, String tag) { + RgavCacheKey(List repositories, String gav, String tag) { + this.repositories = List.copyOf(repositories); this.gav = gav; this.tag = tag; - this.hash = Objects.hash(gav, tag); + this.hash = Objects.hash(this.repositories, this.gav, this.tag); } private static String gav(String groupId, String artifactId, String version) { @@ -107,8 +119,10 @@ public boolean equals(Object obj) { if (null == obj || !getClass().equals(obj.getClass())) { return false; } - GavCacheKey that = (GavCacheKey) obj; - return Objects.equals(this.gav, that.gav) && Objects.equals(this.tag, that.tag); + RgavCacheKey that = (RgavCacheKey) obj; + return Objects.equals(this.repositories, that.repositories) + && Objects.equals(this.gav, that.gav) + && Objects.equals(this.tag, that.tag); } @Override From d3ab299364ae61caac9e2216e2eb3e34c46aeae7 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 14 Jan 2025 20:58:56 +0100 Subject: [PATCH 2/4] Fix --- .../maven/api/services/model/ModelCache.java | 1 + .../impl/model/DefaultModelBuilder.java | 77 ++++++++++++++----- .../impl/model/DefaultModelCache.java | 18 ++++- 3 files changed, 74 insertions(+), 22 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java b/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java index a6e098444798..aa4abc5b2d80 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/api/services/model/ModelCache.java @@ -47,6 +47,7 @@ T computeIfAbsent( String groupId, String artifactId, String version, + String classifier, String tag, Supplier data); diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java index 93b9f8b4af7e..edd3f8822cb2 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java @@ -1612,6 +1612,7 @@ private DependencyManagement loadDependencyManagement(Dependency dependency, Col groupId, artifactId, version, + null, IMPORT, () -> doLoadDependencyManagement(dependency, groupId, artifactId, version, importIds)); DependencyManagement importMgmt = importModel != null ? importModel.getDependencyManagement() : null; @@ -1720,9 +1721,10 @@ private T cache( String groupId, String artifactId, String version, + String classifier, String tag, Supplier supplier) { - return cache.computeIfAbsent(repositories, groupId, artifactId, version, tag, supplier); + return cache.computeIfAbsent(repositories, groupId, artifactId, version, classifier, tag, supplier); } private T cache(Source source, String tag, Supplier supplier) throws ModelBuilderException { @@ -1819,36 +1821,75 @@ class CachingModelResolver implements ModelResolver { public ModelSource resolveModel( Session session, List repositories, - String groupId, - String artifactId, - String version, - String classifier, - Consumer resolvedVersion) + Parent parent, + AtomicReference modified) throws ModelResolverException { ModelResolverResult result = cache.computeIfAbsent( repositories, - groupId, - artifactId, - classifier != null && !classifier.isEmpty() ? version + ":" + classifier : version, + parent.getGroupId(), + parent.getArtifactId(), + parent.getVersion(), + null, MODEL, - () -> doResolve(session, repositories, groupId, artifactId, version, classifier)); - if (result.resolvedVersion != null) { - resolvedVersion.accept(result.resolvedVersion); + () -> { + AtomicReference mod = new AtomicReference<>(); + ModelSource res = modelResolver.resolveModel(session, repositories, parent, mod); + return new ModelResolverResult( + res, mod.get() != null ? mod.get().getVersion() : null); + }); + if (result.resolvedVersion != null && modified != null) { + modified.set(parent.withVersion(result.resolvedVersion)); + } + return result.source; + } + + @Override + public ModelSource resolveModel( + Session session, + List repositories, + Dependency dependency, + AtomicReference modified) + throws ModelResolverException { + ModelResolverResult result = cache.computeIfAbsent( + repositories, + dependency.getGroupId(), + dependency.getArtifactId(), + dependency.getVersion(), + dependency.getClassifier(), + MODEL, + () -> { + AtomicReference mod = new AtomicReference<>(); + ModelSource res = modelResolver.resolveModel(session, repositories, dependency, mod); + return new ModelResolverResult( + res, mod.get() != null ? mod.get().getVersion() : null); + }); + if (result.resolvedVersion != null && modified != null) { + modified.set(dependency.withVersion(result.resolvedVersion)); } return result.source; } - ModelResolverResult doResolve( + @Override + public ModelSource resolveModel( Session session, List repositories, String groupId, String artifactId, String version, - String classifier) { - AtomicReference resolvedVersion = new AtomicReference<>(); - ModelSource source = modelResolver.resolveModel( - session, repositories, groupId, artifactId, version, classifier, resolvedVersion::set); - return new ModelResolverResult(source, resolvedVersion.get()); + String classifier, + Consumer resolvedVersion) + throws ModelResolverException { + ModelResolverResult result = + cache.computeIfAbsent(repositories, groupId, artifactId, version, classifier, MODEL, () -> { + AtomicReference mod = new AtomicReference<>(); + ModelSource res = modelResolver.resolveModel( + session, repositories, groupId, artifactId, version, classifier, mod::set); + return new ModelResolverResult(res, mod.get()); + }); + if (result.resolvedVersion != null) { + resolvedVersion.accept(result.resolvedVersion); + } + return result.source; } } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java index 50529a16531c..0477e86e3f1b 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java @@ -53,9 +53,10 @@ public T computeIfAbsent( String groupId, String artifactId, String version, + String classifier, String tag, Supplier data) { - return (T) computeIfAbsent(new RgavCacheKey(repositories, groupId, artifactId, version, tag), data); + return (T) computeIfAbsent(new RgavCacheKey(repositories, groupId, artifactId, version, classifier, tag), data); } @Override @@ -84,8 +85,13 @@ static class RgavCacheKey { private final int hash; RgavCacheKey( - List repositories, String groupId, String artifactId, String version, String tag) { - this(repositories, gav(groupId, artifactId, version), tag); + List repositories, + String groupId, + String artifactId, + String version, + String classifier, + String tag) { + this(repositories, gav(groupId, artifactId, version, classifier), tag); } RgavCacheKey(List repositories, String gav, String tag) { @@ -95,7 +101,7 @@ static class RgavCacheKey { this.hash = Objects.hash(this.repositories, this.gav, this.tag); } - private static String gav(String groupId, String artifactId, String version) { + private static String gav(String groupId, String artifactId, String version, String classifier) { StringBuilder sb = new StringBuilder(); if (groupId != null) { sb.append(groupId); @@ -108,6 +114,10 @@ private static String gav(String groupId, String artifactId, String version) { if (version != null) { sb.append(version); } + sb.append(":"); + if (classifier != null) { + sb.append(classifier); + } return sb.toString(); } From 129afd91931d6cb8688ab6f03d82c43e6ceb0815 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 16 Jan 2025 11:22:55 +0100 Subject: [PATCH 3/4] Fix OOM error --- .../apache/maven/internal/impl/model/DefaultModelCache.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java index 0477e86e3f1b..0f6ee50d48ae 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java @@ -184,7 +184,7 @@ public int hashCode() { } static class CachingSupplier implements Supplier { - final Supplier supplier; + Supplier supplier; volatile Object value; CachingSupplier(Supplier supplier) { @@ -200,6 +200,7 @@ public T get() { if ((v = value) == null) { try { v = value = supplier.get(); + supplier = null; } catch (Exception e) { v = value = new AltRes(e); } From ea15366fe230d9ef1a07056d01cc84dff5c73720 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Fri, 17 Jan 2025 11:02:38 +0100 Subject: [PATCH 4/4] Use records --- .../impl/model/DefaultModelCache.java | 96 +++---------------- 1 file changed, 15 insertions(+), 81 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java index 0f6ee50d48ae..5318be60f6ee 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelCache.java @@ -19,7 +19,6 @@ package org.apache.maven.internal.impl.model; import java.util.List; -import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Supplier; @@ -74,35 +73,19 @@ protected Object computeIfAbsent(Object key, Supplier data) { return cache.computeIfAbsent(key, k -> new CachingSupplier<>(data)).get(); } - static class RgavCacheKey { - - private final List repositories; - - private final String gav; - - private final String tag; - - private final int hash; - - RgavCacheKey( - List repositories, - String groupId, - String artifactId, - String version, - String classifier, - String tag) { - this(repositories, gav(groupId, artifactId, version, classifier), tag); - } - - RgavCacheKey(List repositories, String gav, String tag) { - this.repositories = List.copyOf(repositories); - this.gav = gav; - this.tag = tag; - this.hash = Objects.hash(this.repositories, this.gav, this.tag); - } + record RgavCacheKey( + List repositories, + String groupId, + String artifactId, + String version, + String classifier, + String tag) { - private static String gav(String groupId, String artifactId, String version, String classifier) { + @Override + public String toString() { StringBuilder sb = new StringBuilder(); + sb.append("GavCacheKey["); + sb.append("gav='"); if (groupId != null) { sb.append(groupId); } @@ -118,69 +101,20 @@ private static String gav(String groupId, String artifactId, String version, Str if (classifier != null) { sb.append(classifier); } + sb.append("', tag='"); + sb.append(tag); + sb.append("']"); return sb.toString(); } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (null == obj || !getClass().equals(obj.getClass())) { - return false; - } - RgavCacheKey that = (RgavCacheKey) obj; - return Objects.equals(this.repositories, that.repositories) - && Objects.equals(this.gav, that.gav) - && Objects.equals(this.tag, that.tag); - } - - @Override - public int hashCode() { - return hash; - } - - @Override - public String toString() { - return "GavCacheKey[" + "gav='" + gav + '\'' + ", tag='" + tag + '\'' + ']'; - } } - private static final class SourceCacheKey { - private final Source source; - - private final String tag; - - private final int hash; - - SourceCacheKey(Source source, String tag) { - this.source = source; - this.tag = tag; - this.hash = Objects.hash(source, tag); - } + record SourceCacheKey(Source source, String tag) { @Override public String toString() { return "SourceCacheKey[" + "location=" + source.getLocation() + ", tag=" + tag + ", path=" + source.getPath() + ']'; } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (null == obj || !getClass().equals(obj.getClass())) { - return false; - } - SourceCacheKey that = (SourceCacheKey) obj; - return Objects.equals(this.source, that.source) && Objects.equals(this.tag, that.tag); - } - - @Override - public int hashCode() { - return hash; - } } static class CachingSupplier implements Supplier {