Improve DeltaList behavior for large projects#335
Merged
gnodet merged 2 commits intoapache:maven-compiler-plugin-3.xfrom Jun 26, 2025
Merged
Improve DeltaList behavior for large projects#335gnodet merged 2 commits intoapache:maven-compiler-plugin-3.xfrom
gnodet merged 2 commits intoapache:maven-compiler-plugin-3.xfrom
Conversation
gsmet
commented
Jun 3, 2025
Comment on lines
36
to
37
| this.added = newList.stream().filter(i -> !oldList.contains(i)).collect(Collectors.toList()); | ||
| this.removed = oldList.stream().filter(i -> !newList.contains(i)).collect(Collectors.toList()); |
Author
There was a problem hiding this comment.
The idea is to use HashSets and avoid copying then removing a lot of elements.
Contributor
There was a problem hiding this comment.
Could it be better to use the old for loop instead of streams?
Author
|
hey @jorsol , any chance you could have a look at this PR? |
I have been experimenting with a large code base to try to improve Quarkus build time and I stumbled upon the fact that DeltaList was very inefficient in these cases. In this commit, I keep the ordering even if not actually important. I will push further optimizations in a second commit that we can discuss further. Fixes apache#334
Member
|
@desruisseaux @gnodet ping |
Author
|
I adjusted the commits:
|
cstamas
approved these changes
Jun 26, 2025
Contributor
|
It looks fine to me as well. |
gnodet
reviewed
Jun 26, 2025
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request targets the 3.x branch as I think this support has been rewritten in 4.
Here is the situation prior to this patch with a massive project (~40k source files):
The first commit keeps the ordering as is and make sure we write the file sorted and the outputs of
DeltaListare sorted.The behavior is exactly the same as before.
Here is the situation after the first commit:
The second commit is a further optimization, which might not be useful and only sorts when displaying the output. The file is written unsorted.
It doesn't change much tbh so we might as well not include the second commit.
I did the work to be comprehensive so I thought I would present it.
Here is the situation after the second commit:
Opening as a draft so that we decide what to do about the second commit: my advice would be to drop it but YMMV
BTW, it might be possible to optimize further but given this code is readable and give good results, I think it's good enough.
Fixes #334
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.