This is an automated email from the ASF dual-hosted git repository.
rfscholte pushed a commit to branch MNG-7063
in repository https://gitbox.apache.org/repos/asf/maven.git
commit 6456668c181bcc0d5f1349270dff1d5901654d85
Author: Guillaume Nodet <gno...@gmail.com>
AuthorDate: Sat Nov 28 22:27:49 2020 +0100
Do not use a "persistent" cache which is the root cause MNG-6530
The root of the problem is MNG-5312 which is a performance issue because a
ModelCache was not always involved during a model build. This lead to MNG-6030
(memory consumption issue because of the usage of multiple caches). The fix for
MNG-6030 lead to MNG-6311 (problem with changes not being picked up in M2E).
The root problem is that the fix for MNG-5312 was wrong. The caches and
its data should be managed at the RepositorySystemSession level, especially as
the data is related to a given session parameters (maven global settings).
This commit brings back the cache at the session level, and a default cache
is now set on the session by the MavenCli. The cache is used later by
DefaultArtifactDescriptorReader and DefaultProjectBuilder to build a
DefaultModelCache.
* use the cache on the MavenExecutionRequest
* get rid of the hack for MNG-6530.
* MavenCli configures a cache for the whole execution request
* merge ReactorModelCache into DefaultModelCache
# Conflicts:
#
maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
# maven-core/src/main/java/org/apache/maven/project/ReactorModelCache.java
---
.../maven/project/DefaultProjectBuilder.java | 32 +----
.../apache/maven/project/ReactorModelCache.java | 160 ---------------------
.../apache/maven/project/ProjectBuilderTest.java | 9 --
.../main/java/org/apache/maven/cli/MavenCli.java | 6 +
.../repository/internal/DefaultModelCache.java | 153 ++++++++++++++++----
5 files changed, 137 insertions(+), 223 deletions(-)
diff --git
a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
index fa3b3e4..29f86b8 100644
---
a/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
+++
b/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java
@@ -71,6 +71,7 @@ import org.apache.maven.model.building.StringModelSource;
import org.apache.maven.model.building.TransformerContext;
import org.apache.maven.model.resolution.ModelResolver;
import org.apache.maven.repository.internal.ArtifactDescriptorUtils;
+import org.apache.maven.repository.internal.DefaultModelCache;
import org.codehaus.plexus.logging.Logger;
import org.codehaus.plexus.util.Os;
import org.codehaus.plexus.util.StringUtils;
@@ -92,9 +93,6 @@ public class DefaultProjectBuilder
implements ProjectBuilder
{
- public static final String DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY =
- "maven.defaultProjectBuilder.disableGlobalModelCache";
-
@Inject
private Logger logger;
@@ -119,8 +117,6 @@ public class DefaultProjectBuilder
@Inject
private ProjectDependenciesResolver dependencyResolver;
- private final ReactorModelCache modelCache = new ReactorModelCache();
-
// ----------------------------------------------------------------------
// MavenProjectBuilder Implementation
// ----------------------------------------------------------------------
@@ -130,12 +126,7 @@ public class DefaultProjectBuilder
throws ProjectBuildingException
{
return build( pomFile, new FileModelSource( pomFile ),
- new InternalConfig( request, null, useGlobalModelCache() ?
getModelCache() : null, null ) );
- }
-
- private boolean useGlobalModelCache()
- {
- return !Boolean.getBoolean( DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY
);
+ new InternalConfig( request, null, null ) );
}
@Override
@@ -143,7 +134,7 @@ public class DefaultProjectBuilder
throws ProjectBuildingException
{
return build( null, modelSource,
- new InternalConfig( request, null, useGlobalModelCache() ?
getModelCache() : null, null ) );
+ new InternalConfig( request, null, null ) );
}
private ProjectBuildingResult build( File pomFile, ModelSource
modelSource, InternalConfig config )
@@ -288,7 +279,7 @@ public class DefaultProjectBuilder
request.setUserProperties( configuration.getUserProperties() );
request.setBuildStartTime( configuration.getBuildStartTime() );
request.setModelResolver( resolver );
- request.setModelCache( config.modelCache );
+ request.setModelCache( DefaultModelCache.newInstance( config.session )
);
request.setTransformerContextBuilder( config.transformerContextBuilder
);
return request;
@@ -309,7 +300,7 @@ public class DefaultProjectBuilder
pomArtifact = ArtifactDescriptorUtils.toPomArtifact( pomArtifact );
InternalConfig config =
- new InternalConfig( request, null, useGlobalModelCache() ?
getModelCache() : null, null );
+ new InternalConfig( request, null, null );
boolean localProject;
@@ -382,8 +373,7 @@ public class DefaultProjectBuilder
final ReactorModelPool modelPool = poolBuilder.build();
InternalConfig config =
- new InternalConfig( request, modelPool, useGlobalModelCache() ?
getModelCache() : new ReactorModelCache(),
- modelBuilder.newTransformerContextBuilder() );
+ new InternalConfig( request, modelPool,
modelBuilder.newTransformerContextBuilder() );
Map<File, MavenProject> projectIndex = new HashMap<>( 256 );
@@ -1023,16 +1013,13 @@ public class DefaultProjectBuilder
private final ReactorModelPool modelPool;
- private final ReactorModelCache modelCache;
-
private final TransformerContextBuilder transformerContextBuilder;
- InternalConfig( ProjectBuildingRequest request, ReactorModelPool
modelPool, ReactorModelCache modelCache,
+ InternalConfig( ProjectBuildingRequest request, ReactorModelPool
modelPool,
TransformerContextBuilder transformerContextBuilder )
{
this.request = request;
this.modelPool = modelPool;
- this.modelCache = modelCache;
this.transformerContextBuilder = transformerContextBuilder;
session =
@@ -1044,9 +1031,4 @@ public class DefaultProjectBuilder
}
- private ReactorModelCache getModelCache()
- {
- return this.modelCache;
- }
-
}
diff --git
a/maven-core/src/main/java/org/apache/maven/project/ReactorModelCache.java
b/maven-core/src/main/java/org/apache/maven/project/ReactorModelCache.java
deleted file mode 100644
index 10ca878..0000000
--- a/maven-core/src/main/java/org/apache/maven/project/ReactorModelCache.java
+++ /dev/null
@@ -1,160 +0,0 @@
-package org.apache.maven.project;
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import org.apache.maven.building.Source;
-import org.apache.maven.model.building.ModelCache;
-
-import java.util.Map;
-import java.util.Objects;
-import java.util.concurrent.ConcurrentHashMap;
-
-/**
- * A simple model cache used to accelerate model building during a reactor
build.
- *
- * @author Benjamin Bentmann
- */
-class ReactorModelCache
- implements ModelCache
-{
-
- private final Map<Object, Object> models = new ConcurrentHashMap<>( 256 );
-
- @Override
- public Object get( String groupId, String artifactId, String version,
String tag )
- {
- return models.get( new GavCacheKey( groupId, artifactId, version, tag
) );
- }
-
- @Override
- public void put( String groupId, String artifactId, String version, String
tag, Object data )
- {
- models.put( new GavCacheKey( groupId, artifactId, version, tag ), data
);
- }
-
- @Override
- public Object get( Source source, String tag )
- {
- return models.get( new SourceCacheKey( source, tag ) );
- }
-
- @Override
- public void put( Source source, String tag, Object data )
- {
- models.put( new SourceCacheKey( source, tag ), data );
- }
-
- private static final class GavCacheKey
- {
-
- private final String groupId;
-
- private final String artifactId;
-
- private final String version;
-
- private final String tag;
-
- private final int hashCode;
-
- GavCacheKey( String groupId, String artifactId, String version, String
tag )
- {
- this.groupId = ( groupId != null ) ? groupId : "";
- this.artifactId = ( artifactId != null ) ? artifactId : "";
- this.version = ( version != null ) ? version : "";
- this.tag = ( tag != null ) ? tag : "";
- this.hashCode = Objects.hash( groupId, artifactId, version, tag );
- }
-
- @Override
- public boolean equals( Object obj )
- {
- if ( this == obj )
- {
- return true;
- }
-
- if ( !( obj instanceof GavCacheKey ) )
- {
- return false;
- }
-
- GavCacheKey that = (GavCacheKey) obj;
-
- return artifactId.equals( that.artifactId ) && groupId.equals(
that.groupId )
- && version.equals( that.version ) && tag.equals( that.tag );
- }
-
- @Override
- public int hashCode()
- {
- return hashCode;
- }
- }
-
- private static final class SourceCacheKey
- {
- private final Source source;
-
- private final String tag;
-
- private final int hashCode;
-
- SourceCacheKey( Source source, String tag )
- {
- this.source = source;
- this.tag = tag;
- this.hashCode = Objects.hash( source, tag );
- }
-
- @Override
- public int hashCode()
- {
- return hashCode;
- }
-
- @Override
- public boolean equals( Object obj )
- {
- if ( this == obj )
- {
- return true;
- }
- if ( !( obj instanceof SourceCacheKey ) )
- {
- return false;
- }
-
- SourceCacheKey other = (SourceCacheKey) obj;
- if ( !Objects.equals( this.source, other.source ) )
- {
- return false;
- }
-
- if ( !Objects.equals( this.tag, other.tag ) )
- {
- return false;
- }
-
- return true;
- }
- }
-
-}
diff --git
a/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
b/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
index f23132c..e2babbf 100644
--- a/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
+++ b/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java
@@ -150,7 +150,6 @@ public class ProjectBuilderTest
@Test
public void testReadModifiedPoms() throws Exception {
- String initialValue = System.setProperty(
DefaultProjectBuilder.DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY,
Boolean.toString( true ) );
// TODO a similar test should be created to test the dependency
management (basically all usages
// of DefaultModelBuilder.getCache() are affected by MNG-6530
@@ -177,14 +176,6 @@ public class ProjectBuilderTest
}
finally
{
- if ( initialValue == null )
- {
- System.clearProperty(
DefaultProjectBuilder.DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY );
- }
- else
- {
- System.setProperty(
DefaultProjectBuilder.DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY, initialValue
);
- }
FileUtils.deleteDirectory( tempDir.toFile() );
}
}
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
index 792a831..c217c9e 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
@@ -83,6 +83,7 @@ import
org.codehaus.plexus.component.repository.exception.ComponentLookupExcepti
import org.codehaus.plexus.logging.LoggerManager;
import org.codehaus.plexus.util.StringUtils;
import org.codehaus.plexus.util.xml.pull.XmlPullParserException;
+import org.eclipse.aether.DefaultRepositoryCache;
import org.eclipse.aether.transfer.TransferListener;
import org.slf4j.ILoggerFactory;
import org.slf4j.Logger;
@@ -975,6 +976,11 @@ public class MavenCli
{
MavenExecutionRequest request =
executionRequestPopulator.populateDefaults( cliRequest.request );
+ if ( cliRequest.request.getRepositoryCache() == null )
+ {
+ cliRequest.request.setRepositoryCache( new
DefaultRepositoryCache() );
+ }
+
eventSpyDispatcher.onEvent( request );
MavenExecutionResult result = maven.execute( request );
diff --git
a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelCache.java
b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelCache.java
index 86edf16..339bc08 100644
---
a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelCache.java
+++
b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelCache.java
@@ -19,8 +19,12 @@ package org.apache.maven.repository.internal;
* under the License.
*/
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.building.Source;
import org.apache.maven.model.building.ModelCache;
-import org.eclipse.aether.RepositoryCache;
import org.eclipse.aether.RepositorySystemSession;
/**
@@ -28,68 +32,107 @@ import org.eclipse.aether.RepositorySystemSession;
*
* @author Benjamin Bentmann
*/
-class DefaultModelCache
+public class DefaultModelCache
implements ModelCache
{
- private final RepositorySystemSession session;
+ private static final String KEY = DefaultModelCache.class.getName();
- private final RepositoryCache cache;
+ private final Map<Object, Object> cache;
public static ModelCache newInstance( RepositorySystemSession session )
{
+ Map<Object, Object> cache;
if ( session.getCache() == null )
{
- return null;
+ cache = new ConcurrentHashMap<>();
}
else
{
- return new DefaultModelCache( session );
+ cache = ( Map ) session.getCache().get( session, KEY );
+ if ( cache == null )
+ {
+ cache = new ConcurrentHashMap<>();
+ session.getCache().put( session, KEY, cache );
+ }
}
+ return new DefaultModelCache( cache );
+ }
+
+ private DefaultModelCache( Map<Object, Object> cache )
+ {
+ this.cache = cache;
+ }
+
+ public Object get( Source path, String tag )
+ {
+ return get( new SourceCacheKey( path, tag ) );
}
- private DefaultModelCache( RepositorySystemSession session )
+ public void put( Source path, String tag, Object data )
{
- this.session = session;
- this.cache = session.getCache();
+ put( new SourceCacheKey( path, tag ), data );
}
public Object get( String groupId, String artifactId, String version,
String tag )
{
- return cache.get( session, new Key( groupId, artifactId, version, tag
) );
+ return get( new GavCacheKey( groupId, artifactId, version, tag ) );
}
public void put( String groupId, String artifactId, String version, String
tag, Object data )
{
- cache.put( session, new Key( groupId, artifactId, version, tag ), data
);
+ put( new GavCacheKey( groupId, artifactId, version, tag ), data );
}
- static class Key
+ protected Object get( Object key )
{
+ return cache.get( key );
+ }
- private final String groupId;
+ protected void put( Object key, Object data )
+ {
+ cache.put( key, data );
+ }
- private final String artifactId;
+ static class GavCacheKey
+ {
- private final String version;
+ private final String gav;
private final String tag;
private final int hash;
- Key( String groupId, String artifactId, String version, String tag )
+ GavCacheKey( String groupId, String artifactId, String version, String
tag )
+ {
+ this( gav( groupId, artifactId, version ), tag );
+ }
+
+ GavCacheKey( String gav, String tag )
{
- this.groupId = groupId;
- this.artifactId = artifactId;
- this.version = version;
+ this.gav = gav;
this.tag = tag;
+ this.hash = Objects.hash( gav, tag );
+ }
- int h = 17;
- h = h * 31 + this.groupId.hashCode();
- h = h * 31 + this.artifactId.hashCode();
- h = h * 31 + this.version.hashCode();
- h = h * 31 + this.tag.hashCode();
- hash = h;
+ private static String gav( String groupId, String artifactId, String
version )
+ {
+ StringBuilder sb = new StringBuilder();
+ if ( groupId != null )
+ {
+ sb.append( groupId );
+ }
+ sb.append( ":" );
+ if ( artifactId != null )
+ {
+ sb.append( artifactId );
+ }
+ sb.append( ":" );
+ if ( version != null )
+ {
+ sb.append( version );
+ }
+ return sb.toString();
}
@Override
@@ -103,10 +146,8 @@ class DefaultModelCache
{
return false;
}
-
- Key that = (Key) obj;
- return artifactId.equals( that.artifactId ) && groupId.equals(
that.groupId )
- && version.equals( that.version ) && tag.equals( that.tag );
+ GavCacheKey that = (GavCacheKey) obj;
+ return Objects.equals( this.gav, that.gav ) && Objects.equals(
this.tag, that.tag );
}
@Override
@@ -115,5 +156,59 @@ class DefaultModelCache
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 );
+ }
+
+ @Override
+ public String toString()
+ {
+ return "SourceCacheKey{"
+ + "source=" + source
+ + ", tag='" + tag + '\''
+ + '}';
+ }
+
+ @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;
+ }
}
}
