[improvement](build) Make the FE compilation incremental - #67824
Open
englefly wants to merge 1 commit into
Open
Conversation
### What problem does this solve?
Problem Summary:
A local edit anywhere in fe-core made `mvn compile` rebuild the whole module: 4487 java files,
about two minutes. Two independent causes were found.
1. The nereids pattern hierarchy (GeneratedPlanPatterns / GeneratedMemoPatterns /
GeneratedPlanRelations / GeneratedExpressionRelations) was produced by a javac annotation
processor which, on every build, parsed all 2679 sources of the nereids tree with antlr
(about 9s) and unconditionally rewrote the four generated files. The fresh timestamps kept
the whole module stale, and the generated content was not even reproducible: the file walk
order came from FileUtils.listFiles, PlanTypeMappingGenerator.findSuperPlan collected it into
a LinkedHashSet, and PlanPatternGeneratorAnalyzer read an IdentityHashMap.
2. maven-compiler-plugin is all-or-nothing by default: as soon as one source is newer than its
class file it recompiles every source of the module. That is independent of the generated
sources, so removing the annotation processor alone does not make an edit incremental.
Generation is now a plain program (PatternCodeGenerator) run by exec-maven-plugin from
process-sources: it caches the parsed ast of every source file, keyed by its content hash, so
only the modified files are re-parsed, and it rewrites an output file only when its content
really changed, so unchanged generated sources keep their timestamp. The emit order was made
deterministic. Measured: generation drops from about 9s to 0.8s, and a build with no source
change rewrites 0 of the 4 generated files.
For the second cause, an opt-in profile `-Pfast-fe` adds IncrementalSourceMarker, which indexes
the identifiers each source mentions and the type names it declares, then marks the changed
sources plus every source which mentions a type they declare. The compiler plugin's per-file
mode then compiles exactly that set. The set is a conservative superset, so no class can be left
referring to a type which changed, moved or disappeared underneath it.
Measured on fe-core, editing one file:
default build : Compiling 4487 source files, about 2:15
-Pfast-fe : Compiling 72 source files, 27s
The result was validated by comparing every class file against a forced full rebuild after
changing the inlined constant StringLikeLiteral.CHINESE_CHAR_BYTE_LENGTH: byte-identical.
### Release note
None
### Check List (For Author)
- Test: Manual test
- The incremental build was compared byte for byte against a forced full rebuild (all
~6000 class files identical) after changing an inlined constant, which a broken
dependency propagation would have left stale.
- build-support/tests/test-incremental-source-marker.sh covers the new build tool.
- `mvn compile -pl fe-core -am` and `mvn -Pfast-fe compile -pl fe-core -am` both pass with
0 checkstyle violations.
- Behavior changed: No, the default build is unchanged and -Pfast-fe is opt-in
- Does this need documentation: No
englefly
requested review from
924060929,
CalvinKirs,
morningman,
morrySnow and
starocean999
as code owners
September 10, 2026 15:49
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
TPC-H: Total hot run time: 17063 ms |
Contributor
TPC-DS: Total hot run time: 82977 ms |
Contributor
ClickBench: Total hot run time: 14.84 s |
924060929
reviewed
Sep 11, 2026
924060929
left a comment
924060929
left a comment
Contributor
There was a problem hiding this comment.
The standalone pattern-generation direction looks reasonable, but the incremental marker is not yet a safe conservative approximation: the cases below can leave stale class files in target/classes. I reproduced each case against this head. Please fix these correctness gaps before merging and add tests that validate the resulting bytecode/runtime behavior, not only which source timestamps were changed.
| } | ||
| affected.addAll(changed); | ||
|
|
||
| // Propagate one hop only: the files which mention a type declared by a changed file. A file |
Contributor
There was a problem hiding this comment.
[P1] One-hop propagation is not safe for Java inheritance. For example, let A declare public static final int VALUE = 2, let B extends A, and let C return B.VALUE. After changing A.VALUE to 3, this loop marks only A and B because C mentions B but not A. Recompiling the marked sources leaves C.class with the old inlined value 2; I reproduced this and confirmed iconst_2 with javap. A source that is recompiled without edits can expose a different effective API through its changed parent, so propagation must continue transitively unless ABI comparison proves that it can stop.
| } | ||
| } | ||
|
|
||
| private static boolean isUnder(File file, File directory) { |
Contributor
There was a problem hiding this comment.
[P1] touchRoot is canonicalized, but scanned files and cache keys use absolute paths. These differ whenever the checkout/root contains a symlink; on macOS the included test already triggers this because mktemp returns /var/folders/... while the canonical path is /private/var/folders/.... isUnder then returns false for every source, so nothing is touched and removed classes are not cleaned. Running test-incremental-source-marker.sh on macOS produced four failures for exactly this reason. Please normalize all roots/files/cache keys consistently and use Path.startsWith rather than string-prefix comparison.
| if (parent == null || !parent.isDirectory()) { | ||
| return; | ||
| } | ||
| String simpleName = new File(prefix).getName(); |
Contributor
There was a problem hiding this comment.
[P2] Class cleanup assumes every binary name starts with the source filename. A legal Removed.java containing public class Removed { static class Inner {} } plus package-private top-level class Extra {} produces Removed.class, Removed$Inner.class, and Extra.class; after deleting the source, this code removes the first two but leaves Extra.class. I reproduced that stale class. Since the index already records all declaredNames, cleanup should remove every top-level type declared by the removed source and each type's $ classes; please add this multi-top-level-class case to the test.
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
3 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.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
A local edit anywhere in
fe-coremademvn compilerebuild the whole module: 4473 javasources, about two minutes. Two independent causes were found and fixed.
1. The pattern hierarchy was regenerated on every build.
GeneratedPlanPatterns/GeneratedMemoPatterns/GeneratedPlanRelations/GeneratedExpressionRelationswereproduced by a javac annotation processor which, on every build, parsed all 2678 sources of the
nereids tree with antlr (about 9 s) and unconditionally rewrote the four generated files. The
fresh timestamps kept the whole module stale, and the generated content was not even
reproducible: the file walk order came from
FileUtils.listFiles,PlanTypeMappingGenerator.findSuperPlancollected it into aLinkedHashSet, andPlanPatternGeneratorAnalyzerread anIdentityHashMap.Generation is now a plain program (
PatternCodeGenerator) run byexec-maven-pluginfromprocess-sources. It caches the parsed ast of every source file, keyed by its content hash, soonly the modified files are re-parsed, and it rewrites an output file only when its content
really changed, so unchanged generated sources keep their timestamp. The emit order was made
deterministic. Measured: the generation step drops from about 9 s to 0.8 s, and a build with no
source change rewrites 0 of the 4 generated files. Two independent cold runs now produce
byte-identical output.
2.
maven-compiler-pluginis all-or-nothing. As soon as one source is newer than its classfile the plugin recompiles every source of the module (
Changes detected - recompiling the module!), which is independent of the generated sources: removing the annotation processoralone does not make an edit incremental.
An opt-in profile
-Pfast-fenow addsIncrementalSourceMarker. It indexes the identifiers eachsource mentions and the type names it declares, then marks (touches) the changed sources plus
every source which mentions a type they declare. The compiler plugin's per-file mode then
compiles exactly that set. The set is a conservative superset, so no class can be left referring
to a type which changed, moved or disappeared underneath it.
Measured on
fe-core, editing one file:Compiling 4473 source files-Pfast-feCompiling 72 source filesThe result was validated by comparing every class file against a forced full rebuild after
changing the inlined constant
StringLikeLiteral.CHINESE_CHAR_BYTE_LENGTH, which a brokendependency propagation would have left stale: all ~6000 class files byte-identical. Editing a
leaf operator marks 130 of 4527 sources, editing a widely used class such as
Planmarks 578.The default build is unchanged;
-Pfast-feis opt-in and intended for the local edit loop. Theonly coupling it cannot see is one which is not spelled out in the source at all (reflection,
ServiceLoader, resource lookups), so a release build should keep using the plain build.Release note
None
Check List (For Author)
Test
~6000 class files identical) after changing an inlined constant, which a broken
dependency propagation would have left stale.
build-support/tests/test-incremental-source-marker.shcovers the new build tool: nochange marks nothing, editing a type marks its referrers and not an unrelated file, a
mention inside a comment or a string literal is not a dependency, a new type marks only
itself, removing a source deletes its own classes and no others.
mvn compile -pl fe-core -amandmvn -Pfast-fe compile -pl fe-core -amboth pass with0 checkstyle violations.
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)