You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "jorsol (via GitHub)" <gi...@apache.org> on 2023/01/29 03:25:31 UTC
[GitHub] [maven-compiler-plugin] jorsol opened a new pull request, #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
jorsol opened a new pull request, #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172
This [fix a bug](https://issues.apache.org/jira/browse/MCOMPILER-525) with the incremental compilation related to the incorrect detection of the project classes as dependency and also partially fix [MCOMPILER-381](https://issues.apache.org/jira/browse/MCOMPILER-38).
Following this checklist to help us incorporate your
contribution quickly and easily:
- [X] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MCOMPILER) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [X] Each commit in the pull request should have a meaningful subject line and body.
- [X] Format the pull request title like `[MCOMPILER-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MCOMPILER-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [X] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [X] You have run the integration tests successfully (`mvn -Prun-its clean verify`).
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [X] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1096142246
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1917,7 +1965,7 @@ private DirectoryScanResult computeInputFileTreeChanges( IncrementalBuildHelper
}
catch ( IOException e )
{
- throw new MojoExecutionException( "Error reading old mojo status " + mojoConfigFile, e );
+ throw new UncheckedIOException( "Error reading old mojo status " + mojoConfigFile, e );
Review Comment:
Ok, I have changed it to not throw an exception, but to log a warning in the case is not possible to read/write the status.
As for the why it's because as part of the refactoring of the code it's being used as a lambda and lambdas don't play well with Checked Exceptions, that's the reasoning for not throwing an exception and fallback to just skip the check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] pzygielo commented on pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "pzygielo (via GitHub)" <gi...@apache.org>.
pzygielo commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1410213100
I believe https://github.com/apache/maven-compiler-plugin/blob/3f95d39f166809bc1a2bb0b2fa62c190e8d2a9f7/src/it/MCOMPILER-275_separate-moduleinfo/verify.groovy#L23 should be updated as well.
It's skipped on GitHub, but it fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1408624537
Hi @olamy, the issue is not fixed with the PR https://github.com/apache/maven-compiler-plugin/pull/163.
I haven't done any performance testing, but I'm sure this will be more performant than the current code, yet I could check and improve the code further if needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on pull request #172: [MCOMPILER-525] Incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1428448028
Thanks @gnodet , I will open the refactor PR for [MCOMPILER-381](https://issues.apache.org/jira/browse/MCOMPILER-381) that was previously done here _ASAP_ and then later I could check if I could use the `IncrementalBuildHelper` mentioned if time allows me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] olamy commented on pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "olamy (via GitHub)" <gi...@apache.org>.
olamy commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1407773356
@jorsol thanks for your PR. is your issue fixed if you try this PR https://github.com/apache/maven-compiler-plugin/pull/172/files#r1090053416?
because I'm a bit scared of the performance impact of some changes here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] pzygielo commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "pzygielo (via GitHub)" <gi...@apache.org>.
pzygielo commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1089900853
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -902,7 +906,7 @@ else if ( CompilerConfiguration.CompilerReuseStrategy.ReuseSame.getStrategy().eq
{
String cause = idk ? "idk" : ( dependencyChanged ? "dependency"
: ( sourceChanged ? "source" : "input tree" ) );
- getLog().info( "Changes detected - recompiling the module! :" + cause );
+ getLog().info( "Changes detected! - recompiling the module due to: " + cause );
Review Comment:
If this message is going to change - could we drop exclamation point completely?
(I'm no position to *request* this to happen, just asking.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] olamy commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "olamy (via GitHub)" <gi...@apache.org>.
olamy commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1090053416
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1733,30 +1740,46 @@ protected boolean isDependencyChanged()
fileExtensions = Collections.unmodifiableList( Arrays.asList( "class", "jar" ) );
}
- Date buildStartTime = getBuildStartTime();
+ Instant buildStartTime = getBuildStartTime();
List<String> pathElements = new ArrayList<>();
pathElements.addAll( getClasspathElements() );
pathElements.addAll( getModulepathElements() );
for ( String pathElement : pathElements )
{
- File artifactPath = new File( pathElement );
- if ( artifactPath.isDirectory() || artifactPath.isFile() )
+ Path artifactPath = Paths.get( pathElement );
+
+ // Search files only on dependencies (other modules), not the current project.
+ if ( Files.isDirectory( artifactPath ) && !artifactPath.equals( getOutputDirectory().toPath() ) )
{
- if ( hasNewFile( artifactPath, buildStartTime ) )
+ try ( Stream<Path> walk = Files.walk( artifactPath ) )
Review Comment:
I need to review more this. At this may have bad performance impact on large projects with a lot of modules and large source code (e.g source files number).
this PR https://github.com/apache/maven-compiler-plugin/pull/163 use another path by storing some information rather than walking again and again the same source tree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] pzygielo commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "pzygielo (via GitHub)" <gi...@apache.org>.
pzygielo commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1089900853
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -902,7 +906,7 @@ else if ( CompilerConfiguration.CompilerReuseStrategy.ReuseSame.getStrategy().eq
{
String cause = idk ? "idk" : ( dependencyChanged ? "dependency"
: ( sourceChanged ? "source" : "input tree" ) );
- getLog().info( "Changes detected - recompiling the module! :" + cause );
+ getLog().info( "Changes detected! - recompiling the module due to: " + cause );
Review Comment:
If this message is going to change - could we drop exclamation point completely?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] slawekjaranowski commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1093738580
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -531,12 +541,12 @@
/**
* File extensions to check timestamp for incremental build.
- * Default contains only <code>class</code> and <code>jar</code>.
+ * Default contains only {@code class} and {@code jar}.
*
* @since 3.1
*/
- @Parameter
- private List<String> fileExtensions;
+ @Parameter( defaultValue = "class,jar" )
Review Comment:
When you add `defaultValue` the information about it will added by plugin tools, so we can remove manual description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] olamy commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "olamy (via GitHub)" <gi...@apache.org>.
olamy commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1095293400
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1917,7 +1965,7 @@ private DirectoryScanResult computeInputFileTreeChanges( IncrementalBuildHelper
}
catch ( IOException e )
{
- throw new MojoExecutionException( "Error reading old mojo status " + mojoConfigFile, e );
+ throw new UncheckedIOException( "Error reading old mojo status " + mojoConfigFile, e );
Review Comment:
why changing that?
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1733,30 +1740,46 @@ protected boolean isDependencyChanged()
fileExtensions = Collections.unmodifiableList( Arrays.asList( "class", "jar" ) );
}
- Date buildStartTime = getBuildStartTime();
+ Instant buildStartTime = getBuildStartTime();
List<String> pathElements = new ArrayList<>();
pathElements.addAll( getClasspathElements() );
pathElements.addAll( getModulepathElements() );
for ( String pathElement : pathElements )
{
- File artifactPath = new File( pathElement );
- if ( artifactPath.isDirectory() || artifactPath.isFile() )
+ Path artifactPath = Paths.get( pathElement );
+
+ // Search files only on dependencies (other modules), not the current project.
+ if ( Files.isDirectory( artifactPath ) && !artifactPath.equals( getOutputDirectory().toPath() ) )
{
- if ( hasNewFile( artifactPath, buildStartTime ) )
+ try ( Stream<Path> walk = Files.walk( artifactPath ) )
Review Comment:
my concern is the method `computeInputFileTreeChanges` already scanning files and here you add another source tree scanning so maybe `IncrementalBuildHelper` could be improved to store the information you need? (in a similar logic as this PR https://github.com/apache/maven-compiler-plugin/pull/163 ).
?
regarding `large project` what sort of figures? because this `large` can be relative ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] gnodet merged pull request #172: [MCOMPILER-525] Incorrect detection of dependency change
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1093502717
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1733,30 +1740,46 @@ protected boolean isDependencyChanged()
fileExtensions = Collections.unmodifiableList( Arrays.asList( "class", "jar" ) );
}
- Date buildStartTime = getBuildStartTime();
+ Instant buildStartTime = getBuildStartTime();
List<String> pathElements = new ArrayList<>();
pathElements.addAll( getClasspathElements() );
pathElements.addAll( getModulepathElements() );
for ( String pathElement : pathElements )
{
- File artifactPath = new File( pathElement );
- if ( artifactPath.isDirectory() || artifactPath.isFile() )
+ Path artifactPath = Paths.get( pathElement );
+
+ // Search files only on dependencies (other modules), not the current project.
+ if ( Files.isDirectory( artifactPath ) && !artifactPath.equals( getOutputDirectory().toPath() ) )
{
- if ( hasNewFile( artifactPath, buildStartTime ) )
+ try ( Stream<Path> walk = Files.walk( artifactPath ) )
Review Comment:
Hi @olamy, now you can review this, I don't expect any bad performance even on large projects, I have tested this vs the 3.10.1 version on a large project without any drawback.
Additionally, I have included a small perf improvement for the evaluation of the cause of recompilation.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] slawekjaranowski commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1093744196
##########
src/it/MCOMPILER-525/pom.xml:
##########
@@ -0,0 +1,64 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>MJAR-70-no-recreation</artifactId>
Review Comment:
`MJAR-70` -> `MCOMPILER-525` - what do yo think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] gnodet commented on pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1424727847
@jorsol I have a hard time understanding the exact change that fixes the problem. Would it be possible to have a first commit that refactors the code using the nio api and other improvements, and another commit with the actual fix for the problem ?
Note that we could also embeds [the trimmed down version](https://github.com/apache/maven-compiler-plugin/blob/7d9b2f7dbfd51d8400c332f6f50388d5a3eaf100/src/main/java/org/apache/maven/plugin/compiler/IncrementalBuildHelper.java) of the `IncrementalBuildHelper`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1425621531
> I have a hard time understanding the exact change that fixes the problem.
Ok, sure, the fix for this issue is trivial, so I undo all the refactoring and only provide the actual fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#issuecomment-1413594960
@slawekjaranowski @olamy I think I have addressed all the comments, I think I end up basically fixing the issue MCOMPILER-381 too.
There are a lot of other improvements that can be made, refactor legacy code, and more, but for now I don't want to keep touching and touching this (every time I look I end up making some small refactor), this should be a good improvement so far, as of performance concerns this should be at least equally performant than the previous code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1096151839
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1917,7 +1965,7 @@ private DirectoryScanResult computeInputFileTreeChanges( IncrementalBuildHelper
}
catch ( IOException e )
{
- throw new MojoExecutionException( "Error reading old mojo status " + mojoConfigFile, e );
+ throw new UncheckedIOException( "Error reading old mojo status " + mojoConfigFile, e );
Review Comment:
I honestly don't like the current status of that code and there is a lot of legacy code that's even deprecated.
I could keep refactoring more and more code and improve it with modern Java, but that would require more effort and I already put a lot of time into this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-compiler-plugin] jorsol commented on a diff in pull request #172: [MCOMPILER-525] - Incremental recompile incorrect detection of dependency change
Posted by "jorsol (via GitHub)" <gi...@apache.org>.
jorsol commented on code in PR #172:
URL: https://github.com/apache/maven-compiler-plugin/pull/172#discussion_r1096226503
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -1733,30 +1740,46 @@ protected boolean isDependencyChanged()
fileExtensions = Collections.unmodifiableList( Arrays.asList( "class", "jar" ) );
}
- Date buildStartTime = getBuildStartTime();
+ Instant buildStartTime = getBuildStartTime();
List<String> pathElements = new ArrayList<>();
pathElements.addAll( getClasspathElements() );
pathElements.addAll( getModulepathElements() );
for ( String pathElement : pathElements )
{
- File artifactPath = new File( pathElement );
- if ( artifactPath.isDirectory() || artifactPath.isFile() )
+ Path artifactPath = Paths.get( pathElement );
+
+ // Search files only on dependencies (other modules), not the current project.
+ if ( Files.isDirectory( artifactPath ) && !artifactPath.equals( getOutputDirectory().toPath() ) )
{
- if ( hasNewFile( artifactPath, buildStartTime ) )
+ try ( Stream<Path> walk = Files.walk( artifactPath ) )
Review Comment:
Now I understand your concern, but actually, right now that's exactly what is happening, I'm NOT adding another source tree scanning, it was already there, but it's written in a bit weird way with a recursive pattern:
https://github.com/apache/maven-compiler-plugin/blob/3f95d39f166809bc1a2bb0b2fa62c190e8d2a9f7/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java#L1784-L1792
I'm following a modern approach with the NIO2 Path API that should be more efficient, so this refactor makes it obvious that it walks the generated classes, now it should be clear and easier to understand (possibly fixing [MCOMPILER-381](https://issues.apache.org/jira/browse/MCOMPILER-381) )
And regarding your concern, I make the refactoring in such a way that it no longer needs to do the scanning for each step, previously the methods `computeInputFileTreeChanges`, `isDependencyChanged`, `isSourceChanged`, `hasInputFileTreeChanged` were executed eagerly every time, even if you detect that there was a change in the previous step, now the evaluation of each method is lazy, so if a dependency is changed it will not continue scanning for stale sources or run the `hasInputFileTreeChanged` method.
I haven't really checked the `maven-shared-incremental` project, and it's possible that all this logic should be moved and improved there, but as of today I'm mostly refactoring the code that it's already here, yet I agree that all of this incremental logic should be rewritten/refactored in such a way that makes it easier to maintain than is right now.
Regarding the `large project` I have tested with the Quarkus project using the following command changing only the `clean` goal and the `version.compiler.plugin` version:
```bash
LANG=en_US.UTF-8 ./mvnw -q clean install -Dversion.compiler.plugin=3.11.0-SNAPSHOT -DskipTests=true -Dasciidoctor.skip=true -Dexec.skip=true -Dinvoker.skip=true -Denforcer.skip=true -Dformatter.skip=true -Dimpsort.skip=true -Dforbiddenapis.skip=true -Dprofile -DprofileFormat=HTML -DhideParameters=true
```
I'm not claiming that this change will be faster, but it should not have bad performance either, this are some profiler reports: [profile-report.zip](https://github.com/apache/maven-compiler-plugin/files/10582313/profile-report.zip) yet this should be taken with a grain of salt, and in any case, you can do some tests and check if you found any regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org