From ab4149fa227e510721056372d5134bc34eded14f Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Thu, 23 Jan 2025 10:16:26 +0100 Subject: [PATCH] [MNG-8520] Add cache for model resolution during project building (#2047) --- .../maven/api/services/model/ModelCache.java | 11 +- .../impl/model/DefaultModelBuilder.java | 101 ++++++++++++++++- .../impl/model/DefaultModelCache.java | 105 ++++++------------ 3 files changed, 138 insertions(+), 79 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..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 @@ -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,14 @@ @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 classifier, + 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..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 @@ -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,9 +1608,11 @@ private DependencyManagement loadDependencyManagement(Dependency dependency, Col } Model importModel = cache( + repositories, groupId, artifactId, version, + null, IMPORT, () -> doLoadDependencyManagement(dependency, groupId, artifactId, version, importIds)); DependencyManagement importMgmt = importModel != null ? importModel.getDependencyManagement() : null; @@ -1641,8 +1646,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 +1716,15 @@ 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 classifier, + String tag, + Supplier supplier) { + return cache.computeIfAbsent(repositories, groupId, artifactId, version, classifier, tag, supplier); } private T cache(Source source, String tag, Supplier supplier) throws ModelBuilderException { @@ -1801,6 +1813,85 @@ 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, + Parent parent, + AtomicReference modified) + throws ModelResolverException { + ModelResolverResult result = cache.computeIfAbsent( + repositories, + parent.getGroupId(), + parent.getArtifactId(), + parent.getVersion(), + null, + MODEL, + () -> { + 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; + } + + @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, 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; + } + } } @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..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 @@ -18,11 +18,12 @@ */ package org.apache.maven.internal.impl.model; -import java.util.Objects; +import java.util.List; 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 +47,15 @@ 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 classifier, + String tag, + Supplier data) { + return (T) computeIfAbsent(new RgavCacheKey(repositories, groupId, artifactId, version, classifier, tag), data); } @Override @@ -65,26 +73,19 @@ protected Object computeIfAbsent(Object key, Supplier data) { return cache.computeIfAbsent(key, k -> new CachingSupplier<>(data)).get(); } - static class GavCacheKey { + record RgavCacheKey( + List repositories, + String groupId, + String artifactId, + String version, + String classifier, + String tag) { - private final String gav; - - private final String tag; - - private final int hash; - - GavCacheKey(String groupId, String artifactId, String version, String tag) { - this(gav(groupId, artifactId, version), tag); - } - - GavCacheKey(String gav, String tag) { - this.gav = gav; - this.tag = tag; - this.hash = Objects.hash(gav, tag); - } - - private static String gav(String groupId, String artifactId, String version) { + @Override + public String toString() { StringBuilder sb = new StringBuilder(); + sb.append("GavCacheKey["); + sb.append("gav='"); if (groupId != null) { sb.append(groupId); } @@ -96,71 +97,28 @@ private static String gav(String groupId, String artifactId, String version) { if (version != null) { sb.append(version); } - return sb.toString(); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (null == obj || !getClass().equals(obj.getClass())) { - return false; + sb.append(":"); + if (classifier != null) { + sb.append(classifier); } - GavCacheKey that = (GavCacheKey) obj; - return 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 + '\'' + ']'; + sb.append("', tag='"); + sb.append(tag); + sb.append("']"); + return sb.toString(); } } - 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 { - final Supplier supplier; + Supplier supplier; volatile Object value; CachingSupplier(Supplier supplier) { @@ -176,6 +134,7 @@ public T get() { if ((v = value) == null) { try { v = value = supplier.get(); + supplier = null; } catch (Exception e) { v = value = new AltRes(e); }