From 39d59cbdf4c25902ca38b27ebd31e9277dc235d5 Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Wed, 8 Jun 2022 15:13:37 +0100 Subject: [PATCH] [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). --- .../build/report/metrics/BuildAttribute.kt | 3 +- .../incremental/IncrementalCachesManager.kt | 11 +- .../incremental/IncrementalCompilerRunner.kt | 167 +++++++++--------- .../IncrementalCompilationMultiProjectIT.kt | 2 +- 4 files changed, 94 insertions(+), 89 deletions(-) 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 9115e6daa350c..bdfe01a4135b9 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 BuildAttributeKind : Serializable { 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 b70c1a1a740e0..a6acd7dd3b59c 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 @@ abstract class IncrementalCachesManager() var isClosed = false + var isSuccessfulyClosed = false @Synchronized protected fun T.registerCache() { @@ -52,15 +53,15 @@ abstract class IncrementalCachesManager, + args: Args, + messageCollector: MessageCollector, + providedChangedFiles: ChangedFiles?, + projectDir: File? = null, + classpathAbiSnapshot: Map + ): 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 @@ abstract class IncrementalCompilerRunner< 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 @@ abstract class IncrementalCompilerRunner< 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 @@ abstract class IncrementalCompilerRunner< 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 - ) - } - } - is CompilationMode.Rebuild -> { - rebuild(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) + 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 -> rebuildReason = compilationMode.reason } - 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 @@ abstract class IncrementalCompilerRunner< 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 c65442dde88cb..9d5a082ad9cf1 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 @@ -675,7 +675,7 @@ abstract class BaseIncrementalCompilationMultiProjectIT : IncrementalCompilation } 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")