DefaultProjectBuilder enhancements #402
Conversation
|
On top of that, I'm going to check if caching of effective models can be done. This requires tracking which properties / profiles are used, but it looks doable. It may not be very interesting for stock maven, but it would certainly be for |
gnodet
force-pushed
the
dpb
branch
2 times, most recently
from
a8dbf87
to
96ea382
Compare
|
@rfscholte could you have a quick look and see if that's something that could be merged at some point ? I don't want to waste time if there's no interest at all. |
|
@rfscholte the commit I pointed you at and on which you commented aa285f2#r45727670 yesterday was outdated. I've rebased on top of the recent changes in master. The related commit is now 22582b6. The answer to the question should be in the commit comment: |
|
I'll investigate the integration test failures asap. |
|
Probably related is MNG-7063: if trying to read the same file multiple times (in this case {{dependency-reduced-pom.xml}}, it'll be picked up from cache. In this case the model shouldn't have been cached. |
|
I've written a unittest for MNG-7063. With this PR it succeeds again. This looks very promising. |
|
I'll get back on this PR next week. |
|
@rfscholte I've rebased and fix the integration tests. |
|
I just had another look at this. It looks to me that this commit has become too big. We should split it into 2 pieces:
|
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
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
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
The checks were previously done after the cache hit, but they are now a duplication of the checks performed in readParentLocally. Doing caching is useless as the first time, the version range will be resolved and stored in the Parent and caching happens during the call to readRawModel anyway. # Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
# Conflicts: # maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
|
ok, let me check that one first |
While initially working on MNG-7027, I came across several issues, so I'm raising this PR in order to gather feedback.
I've split the changes in small commits in order to make reviewing them easier.
There are 2 big important (and non trivial) commits though: 331e402 and aa285f2. Those have comments in the commit, but I can explain more if needed.
This PR is layered on top of #399 and #391 to avoid conflicts.