gnodet-bot commented on code in PR #1144:
URL:
https://github.com/apache/maven-compiler-plugin/pull/1144#discussion_r4188239511
##########
src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java:
##########
@@ -682,6 +683,35 @@ final Charset charset() {
@Parameter(property = "maven.compiler.useIncrementalCompilation")
protected Boolean useIncrementalCompilation;
+ /**
+ * The strategy to use for incremental compilation.
+ * <ul>
+ * <li>{@code timestamp} (default) — the existing timestamp-based
strategy from
+ * {@link IncrementalBuild}. Detects changes by comparing source
file modification
+ * times and triggers full rebuilds when files are added/removed or
dependencies change.
+ * Respects {@link #incrementalCompilation} aspects, {@code
staleMillis}, and
+ * {@code incrementalExcludes}.</li>
+ * <li>{@code graph} — dependency-graph-based strategy. Tracks
class-level dependencies
+ * by analysing bytecode after each compilation pass. When a source
file changes, only
+ * the source files that transitively depend on any of its classes
are recompiled. Any
+ * change to a class (API or implementation) cascades to all its
consumers.
+ * Full JPMS support including {@code module-info.java} and
+ * {@code module-info-patch.maven} tracking.
+ * <p>Note: the graph strategy has its own change detection (content
hashing) and does
+ * not use {@code staleMillis}, {@code incrementalExcludes}, or the
+ * {@link #incrementalCompilation} aspects. Setting {@link
#useIncrementalCompilation}
+ * to {@code false} disables this strategy and forces a full
rebuild.</p></li>
+ * <li>{@code abi} — ABI-fingerprint-based strategy. Tracks class-level
dependencies
+ * and public API surface (ABI) fingerprints via bytecode analysis.
When a source file
+ * changes, only files whose ABI actually changed cascade to their
consumers; body-only
+ * changes recompile only the changed file. Accepted as an alias for
{@code graph}.</li>
+ * </ul>
+ *
+ * @since 4.0.0-beta-7
+ */
+ @Parameter(property = "maven.compiler.incrementalStrategy", defaultValue =
"timestamp")
Review Comment:
⚠️ **Warning (medium): `abi` strategy Javadoc is misleading.** The
description says:
> *only files whose ABI actually changed cascade to their consumers;
body-only changes recompile only the changed file*
But `processCompiledClasses()` cascades ALL compiled types regardless of
whether their ABI fingerprint changed (line ~2108: `changedTypes = new
TreeSet<>(results.keySet())`). The `abi` flag only controls whether ABI
fingerprints and fine-grained sig/impl deps are *stored* — it doesn't reduce
cascading based on fingerprint comparison.
The actual distinction between `graph` and `abi` is:
- `graph`: all deps go into `signatureConsumersIndex` → transitive cascade
for everything
- `abi`: separate sig/impl indexes → transitive cascade only through
signature consumers; implementation consumers cascade directly but not
transitively
Plus `abi` writes the `abi-fingerprints` manifest for cross-module tracking.
Either implement ABI-fingerprint-based cascade pruning in
`processCompiledClasses()` (compare new fingerprint vs stored fingerprint, skip
cascade if unchanged), or fix the Javadoc to accurately describe the current
behavior:
```suggestion
* <li>{@code abi} — ABI-aware dependency-graph strategy. Extends
{@code graph} with
* fine-grained signature/implementation dependency classification
and ABI fingerprints.
* Signature changes cascade transitively to consumers;
implementation-only dependency
* changes cascade directly but not transitively. Writes an ABI
manifest for cross-module
* incremental detection in reactor builds.</li>
```
##########
src/it/abi-incremental-basic/pom.xml:
##########
@@ -0,0 +1,66 @@
+<?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.compiler.it</groupId>
+ <artifactId>abi-incremental-basic</artifactId>
+ <version>1.0-SNAPSHOT</version>
+
+ <description>IT: ABI-based incremental compilation — body-only change should
only recompile the changed file.</description>
+
+ <properties>
Review Comment:
🔴 **Critical: IT will fail — manifest assertion vs strategy mismatch.** This
IT uses `incrementalStrategy=graph`, but `verify.groovy` (line 14 of that file)
asserts that `target/abi-fingerprints` exists. The manifest is only written
when `abiTracking=true`, which requires `incrementalStrategy=abi`.
`GraphIncrementalBuild.finish()` → `if (abiTracking) {
AbiManifest.write(...) }` — with `graph` strategy, `abiTracking=false`, so no
manifest is written.
Either change the strategy to `abi` to match the IT's name and assertions:
```suggestion
<maven.compiler.incrementalStrategy>abi</maven.compiler.incrementalStrategy>
```
Or remove the manifest assertions from `verify.groovy`.
##########
src/it/abi-incremental-cascade/pom.xml:
##########
@@ -0,0 +1,67 @@
+<?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.compiler.it</groupId>
+ <artifactId>abi-incremental-cascade</artifactId>
+ <version>1.0-SNAPSHOT</version>
+
+ <description>IT: ABI change should cascade to consumers (recompile Service
when Model API changes).</description>
+
+ <properties>
Review Comment:
🔴 **Same issue as `abi-incremental-basic`** — uses
`incrementalStrategy=graph` but `verify.groovy` asserts `graph cascade:
recompiling`. The cascade assertion is valid for `graph`, but the IT name
`abi-incremental-cascade` is misleading.
The cascade verify.groovy doesn't assert manifest existence (so it won't
fail for the same reason as the basic IT), but the name should match the
strategy. Consider renaming to `graph-incremental-cascade` or switching to
`incrementalStrategy=abi`.
##########
src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java:
##########
@@ -915,6 +919,244 @@ private static boolean removeFirsts(Deque<Path> paths,
Integer count) {
}
}
+ /**
+ * Compiles using the dependency-graph incremental strategy. This method
handles the full
+ * lifecycle: determining what to compile, running javac, cascading on
changed classes,
+ * and persisting state.
+ *
+ * @param compiler the compiler
+ * @param configuration the options to give to the Java compiler
+ * @param mojo the MOJO for configuration access
+ * @throws IOException if an error occurred while reading or writing a file
+ * @throws MojoException if the compilation failed
+ */
+ void compileWithGraphIncremental(JavaCompiler compiler, Options
configuration, AbstractCompilerMojo mojo)
+ throws IOException {
+ var graphBuild = new GraphIncrementalBuild(outputDirectory);
+
+ // Collect annotation processor path for processor classification
+ var processorPaths = new ArrayList<Path>();
+ for (var entry : dependencies.entrySet()) {
+ if (entry.getKey() instanceof JavaPathType type) {
+ var location = type.location();
+ if (location.isPresent()
+ && (location.get() ==
StandardLocation.ANNOTATION_PROCESSOR_PATH
+ || location.get() ==
StandardLocation.ANNOTATION_PROCESSOR_MODULE_PATH)) {
+ processorPaths.addAll(entry.getValue());
+ }
+ }
+ }
Review Comment:
💡 **Note (low): MODULE_PATH entries not collected for external ABI
tracking.** The `compileWithGraphIncremental` method collects entries from
`CLASS_PATH` and
`ANNOTATION_PROCESSOR_PATH`/`ANNOTATION_PROCESSOR_MODULE_PATH`, but does not
collect entries from `MODULE_PATH`. For JPMS reactor projects where
dependencies are on the module path rather than the classpath, external ABI
changes in those dependencies won't be detected.
This may be acceptable for an initial implementation (non-modular projects
are the common case), but worth tracking as a follow-up — a modular reactor
project where module A depends on module B via `requires` would not detect ABI
changes in B.
##########
src/main/java/org/apache/maven/plugin/compiler/incremental/GraphIncrementalBuild.java:
##########
@@ -0,0 +1,947 @@
+/*
+ * 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.
+ */
+package org.apache.maven.plugin.compiler.incremental;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+
+/**
+ * Dependency-graph-driven incremental build engine, designed for embedding
+ * in maven-compiler-plugin alongside the existing timestamp-based
+ * {@code IncrementalBuild}.
+ *
+ * <p>The plugin drives compilation; this class determines <em>what</em> to
+ * compile and performs post-compilation bytecode analysis to build the
+ * class-level dependency graph. Typical usage:
+ *
+ * {@snippet :
+ * var build = new GraphIncrementalBuild(outputDir);
+ * build.setProcessorPath(processorPath);
+ * build.setConfigHash(configHash);
+ *
+ * Set<Path> toCompile = build.initialize(allSourceFiles);
+ *
+ * while (!toCompile.isEmpty()) {
+ * compiler.compile(toCompile); // any compiler, any mode
+ * toCompile = build.processCompiledClasses(toCompile);
+ * }
+ *
+ * build.finish();
+ * }
+ *
+ * <p>After each compilation pass, {@link #processCompiledClasses(Set)} scans
+ * the freshly produced {@code .class} files, updates the dependency graph,
+ * and returns any additional source files that must be compiled in the next
+ * pass (cascade due to changed classes, or newly discovered dependencies).
+ * The loop converges in at most 2–3 passes in practice.
+ *
+ * <p>The engine persists its state as {@code incremental-state} in the
+ * {@code target/maven-status/maven-compiler-plugin/<outputDirName>/}
directory (outside the class
+ * output directory so it is not packaged into JARs).
+ *
+ * @see IncrementalState
+ */
+public class GraphIncrementalBuild {
+
+ /** Prefix used to distinguish module-info entries from regular type
entries in the state. */
+ static final String MODULE_PREFIX = BytecodeAnalyzer.MODULE_PREFIX;
+
+ private final Path outputDir;
+ private final Path buildDir;
+ private final Path stateFile;
+ private List<Path> classpathEntries;
+ private Set<Path> reactorModulePaths;
+ private boolean abiTracking;
+ private List<Path> processorPath;
+ private ProcessorClassification processorClassification;
+
+ private IncrementalState previousState;
+ private IncrementalState state;
+
+ /** Package-private accessor for tests. */
+ IncrementalState getState() {
+ return state;
+ }
+
+ private Map<String, String> sourceHashes;
+ private Map<String, Long> sourceMtimes;
+ private List<Path> allSourceFiles;
+ private Set<String> allCompiled;
+ private boolean fullBuild;
+ private boolean useModulePrefixedPaths;
+ private String configHash = "";
+ private String rebuildCause;
+ private int totalSources;
+ /** Lazily populated on full builds; maps each output class file to its
simple top-level class name. */
+ private Map<Path, String> outputClassIndex;
+
+ public GraphIncrementalBuild(Path outputDir) {
+ this.outputDir = outputDir.toAbsolutePath();
+ this.buildDir = this.outputDir.getParent() != null ?
this.outputDir.getParent() : this.outputDir;
+ // Store state outside the output directory so it is not included in
the JAR.
+ // Use the same maven-status convention as the timestamp-based
strategy.
+ // Include the output directory name (e.g. "classes", "test-classes")
to avoid
+ // collisions between compile and testCompile executions.
+ String outputDirName = outputDir.getFileName().toString();
+ Path mavenStatus =
buildDir.resolve("maven-status").resolve("maven-compiler-plugin");
+ this.stateFile =
mavenStatus.resolve(outputDirName).resolve("incremental-state");
+ }
+
+ /**
+ * Sets classpath entries for cross-module ABI tracking. Directory entries
+ * are checked for {@link AbiManifest} files; JAR entries use bytecode
+ * analysis as fallback.
+ */
+ public void setClasspathEntries(List<Path> entries) {
+ this.classpathEntries = entries;
+ }
+
+ /**
+ * Marks specific classpath entries as reactor modules. These are always
+ * checked for ABI changes (via manifest or bytecode).
+ */
+ public void setReactorModulePaths(Set<Path> paths) {
+ this.reactorModulePaths = paths;
+ }
+
+ /**
+ * Enables ABI tracking mode ({@code abi} strategy). When {@code true}, the
+ * engine computes ABI fingerprints and fine-grained {@code signatureDeps}/
+ * {@code implementationDeps} for each compiled type, and writes an
+ * {@link AbiManifest} on {@link #finish()}. When {@code false} (default,
+ * {@code graph} strategy), only class-level dependency references are
collected.
+ */
+ public void setAbiTracking(boolean abiTracking) {
+ this.abiTracking = abiTracking;
+ }
+
+ /**
+ * Sets the annotation processor classpath for processor classification.
+ * Entries are scanned for {@code
META-INF/maven/compiler/incremental.annotation.processors}
+ * and {@code META-INF/gradle/incremental.annotation.processors} to
determine
+ * whether each processor is {@link ProcessorType#ISOLATING},
+ * {@link ProcessorType#AGGREGATING}, or {@link ProcessorType#UNKNOWN}.
+ */
+ public void setProcessorPath(List<Path> processorPath) {
+ this.processorPath = processorPath;
+ this.processorClassification = new
ProcessorClassification(processorPath);
+ }
+
+ /**
+ * Indicates that class files are written under module-name subdirectories
+ * of the output directory (MODULE_SOURCE hierarchy). When set, stored
module
+ * names are used as path prefixes when deleting class files.
+ */
+ public void setUseModulePrefixedPaths(boolean useModulePrefixedPaths) {
+ this.useModulePrefixedPaths = useModulePrefixedPaths;
+ }
+
+ /**
+ * Sets a hash of compilation context configuration (e.g. compiler options
+ * and module-info-patch files). If this hash differs from the previous
+ * build, a full rebuild is triggered.
+ */
+ public void setConfigHash(String hash) {
+ this.configHash = hash != null ? hash : "";
+ }
+
+ /**
+ * Initializes the incremental build by scanning source files and comparing
+ * against the previous build's state.
+ *
+ * @param allSourceFiles all source files in this module
+ * @return the set of files that need compilation (may be all files for a
+ * full build, a subset for incremental, or empty if up-to-date)
+ */
+ public Set<Path> initialize(List<Path> allSourceFiles) throws IOException {
+ Files.createDirectories(outputDir);
+
+ this.allSourceFiles = allSourceFiles;
+ totalSources = allSourceFiles.size();
+ allCompiled = new TreeSet<>();
+ previousState = IncrementalState.load(stateFile);
+ sourceMtimes = new LinkedHashMap<>();
+ sourceHashes = hashSourceFiles(allSourceFiles, previousState,
sourceMtimes);
+
+ if (previousState == null) {
+ rebuildCause = "no previous build state";
+ return initFullBuild(allSourceFiles);
+ } else if (!configHash.equals(previousState.getConfigHash())) {
+ rebuildCause = "compilation configuration changed
(module-info-patch.maven or compiler options)";
+ return initFullBuild(allSourceFiles);
+ } else {
+ return initIncrementalBuild(allSourceFiles);
+ }
+ }
+
+ /**
+ * Processes the {@code .class} files produced by the last compilation
pass.
+ * Scans each class file to extract the dependency graph, records changes,
+ * and returns any additional source files that must be compiled in the
next pass.
+ *
+ * <p>The cascade logic: any compiled class whose content changed (new or
modified)
+ * triggers recompilation of all source files that depend on it (signature
or
+ * implementation consumers).
+ *
+ * @param compiledSourceFiles the source files that were passed to the
compiler in this round
+ * @return additional source files to compile (may be empty when fixpoint
is reached)
+ * @throws IOException if reading {@code .class} files fails
+ */
+ public Set<Path> processCompiledClasses(Set<Path> compiledSourceFiles)
throws IOException {
+ // Reset the class index so it is rebuilt fresh for each compilation
round
+ outputClassIndex = null;
+
+ // Scan .class files for the types produced from the compiled source
files
+ var results = new LinkedHashMap<String, SourceFileAnalysis>();
+ for (Path sourceFile : compiledSourceFiles) {
+ collectClassAnalyses(sourceFile, results);
+ }
+
+ // All compiled types cascade to their consumers (any change triggers
recompilation)
+ var changedTypes = new TreeSet<>(results.keySet());
+
+ // Update incremental state with this round's results
+ for (var entry : sourceHashes.entrySet()) {
+ if (allCompiled.contains(entry.getKey())) {
+ state.setSourceHash(entry.getKey(), entry.getValue());
+ }
+ }
+ for (var result : results.values()) {
+ String moduleName = useModulePrefixedPaths ? result.moduleName() :
"";
+ IncrementalState.TypeInfo typeInfo;
+ if (abiTracking) {
+ typeInfo = new IncrementalState.AbiTypeInfo(
+ result.sourceFile(),
+ result.classDeps(),
+ result.annotationTypes(),
+ moduleName,
+ result.signatureDeps(),
+ result.implementationDeps(),
+ result.abiFingerprint());
+ } else {
+ typeInfo = new IncrementalState.GraphTypeInfo(
+ result.sourceFile(), result.classDeps(),
result.annotationTypes(), moduleName);
+ }
+ state.setType(result.qualifiedName(), typeInfo);
+ }
+
+ if (fullBuild || changedTypes.isEmpty()) {
+ return Set.of();
+ }
+
+ // Detect module name changes — require a full rebuild
+ if (previousState != null && hasModuleNameChanged(state,
previousState)) {
+ return forceFullRebuild();
+ }
+
+ // Cascade: find all consumers of changed types (both signature and
implementation)
+ var cascade = new TreeSet<>(changedTypes);
+ for (String type : changedTypes) {
+ expandSignatureCascade(type, state, cascade);
+ }
+
+ var additionalFiles = new TreeSet<Path>();
+ for (String cascadedType : cascade) {
+ for (String consumer : state.getAllConsumers(cascadedType)) {
+ String sf = state.sourceFileFor(consumer);
+ if (sf != null && !allCompiled.contains(sf)) {
+ additionalFiles.add(Path.of(sf));
+ allCompiled.add(sf);
+ }
+ }
+ }
+
+ // Annotation processor cascade
+ additionalFiles.addAll(computeProcessorCascade());
+
+ return additionalFiles;
+ }
+
+ /**
+ * Scans the output directory for {@code .class} files produced from the
given source file,
+ * analyzes each one with {@link BytecodeAnalyzer}, and accumulates {@link
SourceFileAnalysis}
+ * records into {@code results}.
+ *
+ * <p>The source→class mapping is reconstructed by looking up types
previously recorded for
+ * this source file in the previous state, and by scanning the output
directory for class
+ * files whose name prefix matches the source file's simple name. This
covers both primary
+ * classes and inner/anonymous classes ({@code Foo$Bar.class}).
+ */
+ private void collectClassAnalyses(Path sourceFile, Map<String,
SourceFileAnalysis> results) throws IOException {
+ String sourceFilePath = sourceFile.toString();
+ String simpleSourceName = sourceFile.getFileName().toString(); // e.g.
"Model.java"
+
+ // Map from class file path → pre-computed analysis (null if not yet
analyzed).
+ // Reuses the analysis from the package-dir walk to avoid double
BytecodeAnalyzer.analyze calls.
+ var classFileCache = new LinkedHashMap<Path,
BytecodeAnalyzer.ClassAnalysis>();
+
+ // Types previously tracked for this source file — their .class files
may have moved
+ if (previousState != null) {
+ for (String type :
previousState.getTypesFromSource(sourceFilePath)) {
+ Path classFile = classFileFor(type,
previousState.getType(type));
+ if (Files.exists(classFile)) {
+ classFileCache.put(classFile, null);
+ }
+ // Also scan for inner classes (Foo$Bar.class etc.)
+ var innerFiles = new TreeSet<Path>();
+ addInnerClassFiles(classFile, innerFiles);
+ for (Path inner : innerFiles) {
+ classFileCache.putIfAbsent(inner, null);
+ }
+ }
+ }
+
+ // Walk output directory entries for this source's simple name
+ // (handles new types introduced in this compilation)
+ Path packageDir = inferPackageDir(sourceFile);
+ if (packageDir != null && Files.isDirectory(packageDir)) {
+ try (var stream = Files.list(packageDir)) {
+ stream.filter(p -> p.toString().endsWith(".class")).forEach(cf
-> {
+ try {
+ // Match by SourceFile attribute — covers primary
class, inner/anonymous
+ // classes (Foo$Bar.class), AND package-private
secondary top-level classes
+ // (FooHelper in Foo.java), without cross-package
simple-name collisions.
+ var a = BytecodeAnalyzer.analyze(cf);
+ if (simpleSourceName.equals(a.sourceFileName())) {
+ classFileCache.put(cf, a); // cache the analysis
for reuse below
+ }
+ } catch (IOException e) {
+ // Log at debug level — a corrupted or inaccessible
class file is silently
+ // skipped; the missing type entry will trigger a full
rebuild next time.
+ System.getLogger(GraphIncrementalBuild.class.getName())
+ .log(System.Logger.Level.DEBUG, "Failed to
analyze class file: {0}", cf);
+ }
+ });
+ }
+ } else {
+ // No previous state for this source (full build or new file in
incremental build):
+ // use the cached class index keyed by SourceFile attribute value
(simple source name,
+ // e.g. "Foo.java"). This correctly associates package-private
secondary types and
+ // avoids cross-package simple-name collisions.
+ for (var entry : getFullBuildClassIndex().entrySet()) {
+ if (simpleSourceName.equals(entry.getValue())) {
+ classFileCache.putIfAbsent(entry.getKey(), null);
+ }
+ }
+ }
+
+ // Special case: module-info.class — check both flat and
module-prefixed locations
+ if (sourceFile.getFileName().toString().equals("module-info.java")) {
+ // Flat layout: outputDir/module-info.class
+ Path flat = outputDir.resolve("module-info.class");
+ if (Files.exists(flat)) {
+ classFileCache.putIfAbsent(flat, null);
+ }
+ // MODULE_SOURCE layout: outputDir/<moduleName>/module-info.class
+ if (useModulePrefixedPaths && previousState != null) {
+ // Find the module name from previous state
+ for (String type :
previousState.getTypesFromSource(sourceFile.toString())) {
+ if (type.startsWith(MODULE_PREFIX)) {
+ String modName =
type.substring(MODULE_PREFIX.length());
+ Path modInfo =
outputDir.resolve(modName).resolve("module-info.class");
+ if (Files.exists(modInfo)) {
+ classFileCache.putIfAbsent(modInfo, null);
+ }
+ break;
+ }
+ }
+ }
+ }
+
+ // Final pass: analyze each class file (reusing cached analysis where
available)
+ for (var cacheEntry : classFileCache.entrySet()) {
+ Path classFile = cacheEntry.getKey();
+ if (!Files.exists(classFile)) {
+ continue;
+ }
+ try {
+ if (abiTracking) {
+ // Full ABI analysis: sig/impl deps + fingerprint
+ var analysis =
+ cacheEntry.getValue() != null ?
cacheEntry.getValue() : BytecodeAnalyzer.analyze(classFile);
+ String sourceFilePath2 = analysis.isModuleInfo()
+ ? sourceFilePath
+ : resolveSourceFile(analysis.className(),
sourceFilePath);
+ var sfa = new SourceFileAnalysis(
+ analysis.className(),
+ sourceFilePath2,
+ unionDeps(analysis.signatureTypes(),
analysis.implementationTypes()),
+ analysis.signatureTypes(),
+ analysis.implementationTypes(),
+ analysis.abiFingerprint(),
+ analysis.abiCanonical(),
+ analysis.annotationTypes(),
+ analysis.moduleName());
+ results.put(analysis.className(), sfa);
+ } else {
+ // Graph analysis: class-level deps only (constant pool
scan)
+ // We still need className, sourceFileName, moduleName,
annotationTypes
+ // from a lightweight analysis — use analyzeGraph for deps
but full analyze
+ // for metadata (still one parse of the classfile via the
classfile API).
+ var analysis =
+ cacheEntry.getValue() != null ?
cacheEntry.getValue() : BytecodeAnalyzer.analyze(classFile);
+ String sourceFilePath2 = analysis.isModuleInfo()
+ ? sourceFilePath
+ : resolveSourceFile(analysis.className(),
sourceFilePath);
+ Set<String> classDeps =
unionDeps(analysis.signatureTypes(), analysis.implementationTypes());
+ var sfa = new SourceFileAnalysis(
+ analysis.className(),
+ sourceFilePath2,
+ classDeps,
+ Set.of(),
Review Comment:
💡 **Note (low): `getFullBuildClassIndex()` walks the entire output tree.**
On a full build, this walks all `.class` files in the output directory and runs
`BytecodeAnalyzer.analyze()` on each one to read the `SourceFile` attribute.
This happens once per source file via `collectClassAnalyses()` when
`packageDir` is null (no previous state).
The index is cached (`outputClassIndex`), so the walk happens at most once
per round. But for large projects with thousands of class files, this
duplicates the analysis work since every class file is also individually
analyzed in the final pass of `collectClassAnalyses()`. The cached analysis
(`classFileCache.putIfAbsent(entry.getKey(), null)`) doesn't reuse the analysis
from the index walk — it stores `null` and re-analyzes.
Not a correctness issue, but a performance opportunity for large codebases.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
