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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
