[IC] Fix fallback logic in IncrementalCompilerRunner
The current logic works as follows:
- Try either incremental compilation or non-incremental compilation
- If the above (or any of its surrounding work) fails, fall back to
non-incremental compilation
This means we may perform non-incremental compilation twice.
This commit will fix that logic so that we fall back to non-incremental
compilation only if *incremental compilation* fails.
A nice consequence of this change is that it also resolves the critical
bugs described at KT-52669 (which occur because the current logic is
flawed).
#KT-52669 Fixed
diff --git a/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt b/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt
index 9115e6d..bdfe01a 100644
--- a/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt
+++ b/build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt
@@ -18,8 +18,9 @@
enum class BuildAttribute(val kind: BuildAttributeKind, val readableString: String) : Serializable {
NO_BUILD_HISTORY(BuildAttributeKind.REBUILD_REASON, "Build history file not found"),
NO_ABI_SNAPSHOT(BuildAttributeKind.REBUILD_REASON, "ABI snapshot not found"),
+ INTERNAL_ERROR(BuildAttributeKind.REBUILD_REASON, "Internal error during preparation of IC round"),
CLASSPATH_SNAPSHOT_NOT_FOUND(BuildAttributeKind.REBUILD_REASON, "Classpath snapshot not found"),
- CACHE_CORRUPTION(BuildAttributeKind.REBUILD_REASON, "Cache corrupted"),
+ INCREMENTAL_COMPILATION_FAILED(BuildAttributeKind.REBUILD_REASON, "Incremental compilation failed"),
UNKNOWN_CHANGES_IN_GRADLE_INPUTS(BuildAttributeKind.REBUILD_REASON, "Unknown Gradle changes"),
JAVA_CHANGE_UNTRACKED_FILE_IS_REMOVED(BuildAttributeKind.REBUILD_REASON, "Untracked Java file is removed"),
JAVA_CHANGE_UNEXPECTED_PSI(BuildAttributeKind.REBUILD_REASON, "Java PSI file is expected"),
diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCachesManager.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCachesManager.kt
index b70c1a1..a6acd7d 100644
--- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCachesManager.kt
+++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCachesManager.kt
@@ -34,6 +34,7 @@
private val caches = arrayListOf<BasicMapsOwner>()
var isClosed = false
+ var isSuccessfulyClosed = false
@Synchronized
protected fun <T : BasicMapsOwner> T.registerCache() {
@@ -52,15 +53,15 @@
@Synchronized
fun close(flush: Boolean = false): Boolean {
if (isClosed) {
- return true
+ return isSuccessfulyClosed
}
- var successful = true
+ isSuccessfulyClosed = true
for (cache in caches) {
if (flush) {
try {
cache.flush(false)
} catch (e: Throwable) {
- successful = false
+ isSuccessfulyClosed = false
reporter.report { "Exception when flushing cache ${cache.javaClass}: $e" }
}
}
@@ -68,13 +69,13 @@
try {
cache.close()
} catch (e: Throwable) {
- successful = false
+ isSuccessfulyClosed = false
reporter.report { "Exception when closing cache ${cache.javaClass}: $e" }
}
}
isClosed = true
- return successful
+ return isSuccessfulyClosed
}
}
diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt
index eadf86c..dd9a0e8 100644
--- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt
+++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt
@@ -71,7 +71,53 @@
providedChangedFiles: ChangedFiles?,
projectDir: File? = null
): ExitCode = reporter.measure(BuildTime.INCREMENTAL_COMPILATION_DAEMON) {
- compileImpl(allSourceFiles, args, messageCollector, providedChangedFiles, projectDir)
+ try {
+ compileImpl(allSourceFiles, args, messageCollector, providedChangedFiles, projectDir)
+ } finally {
+ reporter.measure(BuildTime.CALCULATE_OUTPUT_SIZE) {
+ reporter.addMetric(
+ BuildPerformanceMetric.SNAPSHOT_SIZE,
+ buildHistoryFile.length() + lastBuildInfoFile.length() + abiSnapshotFile.length()
+ )
+ if (cacheDirectory.exists() && cacheDirectory.isDirectory()) {
+ cacheDirectory.walkTopDown().filter { it.isFile }.map { it.length() }.sum().let {
+ reporter.addMetric(BuildPerformanceMetric.CACHE_DIRECTORY_SIZE, it)
+ }
+ }
+ }
+ }
+ }
+
+ fun rebuild(
+ reason: BuildAttribute,
+ allSourceFiles: List<File>,
+ args: Args,
+ messageCollector: MessageCollector,
+ providedChangedFiles: ChangedFiles?,
+ projectDir: File? = null,
+ classpathAbiSnapshot: Map<String, AbiSnapshot>
+ ): ExitCode {
+ reporter.report { "Non-incremental compilation will be performed: $reason" }
+ reporter.measure(BuildTime.CLEAR_OUTPUT_ON_REBUILD) {
+ cleanOutputsAndLocalStateOnRebuild(args)
+ }
+ val caches = createCacheManager(args, projectDir)
+ try {
+ if (providedChangedFiles == null) {
+ caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
+ }
+ val allKotlinFiles = allSourceFiles.filter { it.isKotlinFile(kotlinSourceFilesExtensions) }
+ return compileIncrementally(
+ args, caches, allKotlinFiles, CompilationMode.Rebuild(reason), messageCollector, withAbiSnapshot,
+ classpathAbiSnapshot = classpathAbiSnapshot
+ ).also {
+ if (it == ExitCode.OK) {
+ performWorkAfterSuccessfulCompilation(caches)
+ }
+ }
+ } finally {
+ caches.close(true)
+ }
}
private fun compileImpl(
@@ -82,13 +128,11 @@
projectDir: File? = null
): ExitCode {
var caches = createCacheManager(args, projectDir)
+ var rebuildReason = BuildAttribute.INTERNAL_ERROR
- if (withAbiSnapshot) {
- reporter.report { "Incremental compilation with ABI snapshot enabled" }
- }
- //TODO if abi-snapshot is corrupted unable to rebuild. Should roll back to withSnapshot = false?
val classpathAbiSnapshot =
if (withAbiSnapshot) {
+ reporter.report { "Incremental compilation with ABI snapshot enabled" }
reporter.measure(BuildTime.SET_UP_ABI_SNAPSHOTS) {
setupJarDependencies(args, withAbiSnapshot, reporter)
}
@@ -96,27 +140,7 @@
emptyMap()
}
- fun rebuild(reason: BuildAttribute): ExitCode {
- reporter.report { "Non-incremental compilation will be performed: $reason" }
- caches.close(false)
- // todo: we can recompile all files incrementally (not cleaning caches), so rebuild won't propagate
- reporter.measure(BuildTime.CLEAR_OUTPUT_ON_REBUILD) {
- cleanOutputsAndLocalStateOnRebuild(args)
- }
- caches = createCacheManager(args, projectDir)
- if (providedChangedFiles == null) {
- caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
- }
- val allKotlinFiles = allSourceFiles.filter { it.isKotlinFile(kotlinSourceFilesExtensions) }
- return compileIncrementally(
- args, caches, allKotlinFiles, CompilationMode.Rebuild(reason), messageCollector, withAbiSnapshot,
- classpathAbiSnapshot = classpathAbiSnapshot
- )
- }
-
- // If compilation has crashed or we failed to close caches we have to clear them
- var cachesMayBeCorrupted = true
- return try {
+ try {
val changedFiles = when (providedChangedFiles) {
is ChangedFiles.Dependencies -> {
val changedSources = caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
@@ -129,75 +153,51 @@
else -> providedChangedFiles
}
- @Suppress("MoveVariableDeclarationIntoWhen")
- val compilationMode = sourcesToCompile(caches, changedFiles, args, messageCollector, classpathAbiSnapshot)
+ var compilationMode = sourcesToCompile(caches, changedFiles, args, messageCollector, classpathAbiSnapshot)
+ val abiSnapshot = if (compilationMode is CompilationMode.Incremental && withAbiSnapshot) {
+ AbiSnapshotImpl.read(abiSnapshotFile, reporter)
+ } else {
+ if (withAbiSnapshot) {
+ compilationMode = CompilationMode.Rebuild(BuildAttribute.NO_ABI_SNAPSHOT)
+ }
+ null
+ }
- val exitCode = when (compilationMode) {
+ when (compilationMode) {
is CompilationMode.Incremental -> {
- if (withAbiSnapshot) {
- val abiSnapshot = AbiSnapshotImpl.read(abiSnapshotFile, reporter)
- if (abiSnapshot != null) {
+ try {
+ val exitCode = if (withAbiSnapshot) {
compileIncrementally(
- args,
- caches,
- allSourceFiles,
- compilationMode,
- messageCollector,
- withAbiSnapshot,
- abiSnapshot,
- classpathAbiSnapshot
+ args, caches, allSourceFiles, compilationMode, messageCollector,
+ withAbiSnapshot, abiSnapshot!!, classpathAbiSnapshot
)
} else {
- rebuild(BuildAttribute.NO_ABI_SNAPSHOT)
+ compileIncrementally(args, caches, allSourceFiles, compilationMode, messageCollector, withAbiSnapshot)
}
- } else {
- compileIncrementally(
- args,
- caches,
- allSourceFiles,
- compilationMode,
- messageCollector,
- withAbiSnapshot
- )
+ if (exitCode == ExitCode.OK) {
+ performWorkAfterSuccessfulCompilation(caches)
+ }
+ return exitCode
+ } catch (e: Throwable) {
+ reporter.report {
+ "Incremental compilation failed: ${e.stackTraceToString()}.\nFalling back to non-incremental compilation."
+ }
+ rebuildReason = BuildAttribute.INCREMENTAL_COMPILATION_FAILED
}
}
- is CompilationMode.Rebuild -> {
- rebuild(compilationMode.reason)
- }
+ is CompilationMode.Rebuild -> rebuildReason = compilationMode.reason
}
-
- if (exitCode == ExitCode.OK) {
- performWorkAfterSuccessfulCompilation(caches)
- }
-
- if (!caches.close(flush = true)) throw RuntimeException("Could not flush caches")
- // Here we should analyze exit code of compiler. E.g. compiler failure should lead to caches rebuild,
- // but now JsKlib compiler reports invalid exit code.
- cachesMayBeCorrupted = false
-
- reporter.measure(BuildTime.CALCULATE_OUTPUT_SIZE) {
- reporter.addMetric(
- BuildPerformanceMetric.SNAPSHOT_SIZE,
- buildHistoryFile.length() + lastBuildInfoFile.length() + abiSnapshotFile.length()
- )
- if (cacheDirectory.exists() && cacheDirectory.isDirectory()) {
- cacheDirectory.walkTopDown().filter { it.isFile }.map { it.length() }.sum().let {
- reporter.addMetric(BuildPerformanceMetric.CACHE_DIRECTORY_SIZE, it)
- }
- }
- }
- return exitCode
- } catch (e: Exception) { // todo: catch only cache corruption
- // todo: warn?
- reporter.report { "Possible caches corruption: $e" }
- rebuild(BuildAttribute.CACHE_CORRUPTION).also {
- cachesMayBeCorrupted = false
+ } catch (e: Exception) {
+ reporter.report {
+ "Incremental compilation analysis failed: ${e.stackTraceToString()}.\nFalling back to non-incremental compilation."
}
} finally {
- if (cachesMayBeCorrupted) {
+ if (!caches.close()) {
+ reporter.report { "Unable to close IC caches. Cleaning internal state" }
cleanOutputsAndLocalStateOnRebuild(args)
}
}
+ return rebuild(rebuildReason, allSourceFiles, args, messageCollector, providedChangedFiles, projectDir, classpathAbiSnapshot)
}
/**
@@ -411,7 +411,10 @@
break
}
- val (dirtyLookupSymbols, dirtyClassFqNames, forceRecompile) = changesCollector.getDirtyData(listOf(caches.platformCache), reporter)
+ val (dirtyLookupSymbols, dirtyClassFqNames, forceRecompile) = changesCollector.getDirtyData(
+ listOf(caches.platformCache),
+ reporter
+ )
val compiledInThisIterationSet = sourcesToCompile.toHashSet()
val forceToRecompileFiles = mapClassesFqNamesToFiles(listOf(caches.platformCache), forceRecompile, reporter)
diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt
index 06ff14b..3adbdbe 100644
--- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt
+++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt
@@ -700,7 +700,7 @@
}
build("assemble") {
- assertOutputContains("Non-incremental compilation will be performed: CACHE_CORRUPTION")
+ assertOutputContains("Non-incremental compilation will be performed: INCREMENTAL_COMPILATION_FAILED")
}
val lookupFile = projectPath.resolve("lib/build/kotlin/${compileKotlinTaskName}/cacheable/${compileCacheFolderName}/lookups/file-to-id.tab")
