Skip to content

Commit

Permalink
Do not use a "persistent" cache which is the root cause MNG-6530
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gnodet committed Nov 27, 2020
1 parent 7777c6f commit 331e402
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 235 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,15 @@
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.model.building.ModelBuildingRequest;
import org.apache.maven.model.building.ModelBuildingResult;
import org.apache.maven.model.building.ModelCache;
import org.apache.maven.model.building.ModelProblem;
import org.apache.maven.model.building.ModelProcessor;
import org.apache.maven.model.building.ModelSource;
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;
Expand All @@ -91,9 +93,6 @@ public class DefaultProjectBuilder
implements ProjectBuilder
{

public static final String DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY =
"maven.defaultProjectBuilder.disableGlobalModelCache";

private final Logger logger;

private final ModelBuilder modelBuilder;
Expand All @@ -110,8 +109,6 @@ public class DefaultProjectBuilder

private final ProjectDependenciesResolver dependencyResolver;

private final ReactorModelCache modelCache;

@Inject
@SuppressWarnings( "checkstyle:parameternumber" )
public DefaultProjectBuilder(
Expand All @@ -133,7 +130,6 @@ public DefaultProjectBuilder(
this.repoSystem = repoSystem;
this.repositoryManager = repositoryManager;
this.dependencyResolver = dependencyResolver;
this.modelCache = new ReactorModelCache();

This comment has been minimized.

Copy link
@rfscholte

rfscholte Jan 7, 2021

Contributor

IIRC this seems very important to IDEs and the the MavenDaemon. They must be able to empty the global cache for every execution.
Did you do a git blame to see who were involved in this?

}

// ----------------------------------------------------------------------
Expand All @@ -145,20 +141,15 @@ public ProjectBuildingResult build( File pomFile, ProjectBuildingRequest request
throws ProjectBuildingException
{
return build( pomFile, new FileModelSource( pomFile ),
new InternalConfig( request, null, useGlobalModelCache() ? getModelCache() : null ) );
}

private boolean useGlobalModelCache()
{
return !Boolean.getBoolean( DISABLE_GLOBAL_MODEL_CACHE_SYSTEM_PROPERTY );
new InternalConfig( request, null ) );
}

@Override
public ProjectBuildingResult build( ModelSource modelSource, ProjectBuildingRequest request )
throws ProjectBuildingException
{
return build( null, modelSource,
new InternalConfig( request, null, useGlobalModelCache() ? getModelCache() : null ) );
new InternalConfig( request, null ) );
}

private ProjectBuildingResult build( File pomFile, ModelSource modelSource, InternalConfig config )
Expand Down Expand Up @@ -301,6 +292,8 @@ private ModelBuildingRequest getModelBuildingRequest( InternalConfig config )
new ProjectModelResolver( config.session, trace, repoSystem, repositoryManager, config.repositories,
configuration.getRepositoryMerging(), config.modelPool );

ModelCache modelCache = DefaultModelCache.newInstance( config.session );

request.setValidationLevel( configuration.getValidationLevel() );
request.setProcessPlugins( configuration.isProcessPlugins() );
request.setProfiles( configuration.getProfiles() );
Expand All @@ -310,7 +303,7 @@ private ModelBuildingRequest getModelBuildingRequest( InternalConfig config )
request.setUserProperties( configuration.getUserProperties() );
request.setBuildStartTime( configuration.getBuildStartTime() );
request.setModelResolver( resolver );
request.setModelCache( config.modelCache );
request.setModelCache( modelCache );
request.setTransformerContext( (TransformerContext) config.session.getData().get( TransformerContext.KEY ) );

return request;
Expand All @@ -330,7 +323,7 @@ public ProjectBuildingResult build( Artifact artifact, boolean allowStubModel, P
org.eclipse.aether.artifact.Artifact pomArtifact = RepositoryUtils.toArtifact( artifact );
pomArtifact = ArtifactDescriptorUtils.toPomArtifact( pomArtifact );

InternalConfig config = new InternalConfig( request, null, useGlobalModelCache() ? getModelCache() : null );
InternalConfig config = new InternalConfig( request, null );

boolean localProject;

Expand Down Expand Up @@ -427,8 +420,7 @@ public Model getRawModel( String groupId, String artifactId )
request.getRepositorySession().getData().set( TransformerContext.KEY, context );
}

InternalConfig config = new InternalConfig( request, modelPool,
useGlobalModelCache() ? getModelCache() : new ReactorModelCache() );
InternalConfig config = new InternalConfig( request, modelPool );

Map<String, MavenProject> projectIndex = new HashMap<>( 256 );

Expand Down Expand Up @@ -1110,13 +1102,10 @@ class InternalConfig

private final ReactorModelPool modelPool;

private final ReactorModelCache modelCache;

InternalConfig( ProjectBuildingRequest request, ReactorModelPool modelPool, ReactorModelCache modelCache )
InternalConfig( ProjectBuildingRequest request, ReactorModelPool modelPool )
{
this.request = request;
this.modelPool = modelPool;
this.modelCache = modelCache;
session =
LegacyLocalRepositoryManager.overlay( request.getLocalRepository(), request.getRepositorySession(),
repoSystem );
Expand All @@ -1125,9 +1114,4 @@ class InternalConfig

}

private ReactorModelCache getModelCache()
{
return this.modelCache;
}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ public void testDontResolveDependencies()
}

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

Expand All @@ -167,14 +166,6 @@ public void testReadModifiedPoms() throws Exception {
}
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() );
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
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;
Expand Down Expand Up @@ -977,6 +978,11 @@ private int execute( CliRequest cliRequest )
{
MavenExecutionRequest request = executionRequestPopulator.populateDefaults( cliRequest.request );

if ( cliRequest.request.getRepositoryCache() == null )
{
cliRequest.request.setRepositoryCache( new DefaultRepositoryCache() );
}

eventSpyDispatcher.onEvent( request );

MavenExecutionResult result = maven.execute( request );
Expand Down
Loading

0 comments on commit 331e402

Please sign in to comment.