Skip to content

Commit 39d59cb

Browse files
hungvietnguyenTapchicoma
hungvietnguyen
authored and
Tapchicoma
committed
[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).
1 parent aab426c commit 39d59cb

File tree

4 files changed

+94
-89
lines changed

4 files changed

+94
-89
lines changed

‎build-common/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ enum class BuildAttributeKind : Serializable {
1818
enum class BuildAttribute(val kind: BuildAttributeKind, val readableString: String) : Serializable {
1919
NO_BUILD_HISTORY(BuildAttributeKind.REBUILD_REASON, "Build history file not found"),
2020
NO_ABI_SNAPSHOT(BuildAttributeKind.REBUILD_REASON, "ABI snapshot not found"),
21+
INTERNAL_ERROR(BuildAttributeKind.REBUILD_REASON, "Internal error during preparation of IC round"),
2122
CLASSPATH_SNAPSHOT_NOT_FOUND(BuildAttributeKind.REBUILD_REASON, "Classpath snapshot not found"),
22-
CACHE_CORRUPTION(BuildAttributeKind.REBUILD_REASON, "Cache corrupted"),
23+
INCREMENTAL_COMPILATION_FAILED(BuildAttributeKind.REBUILD_REASON, "Incremental compilation failed"),
2324
UNKNOWN_CHANGES_IN_GRADLE_INPUTS(BuildAttributeKind.REBUILD_REASON, "Unknown Gradle changes"),
2425
JAVA_CHANGE_UNTRACKED_FILE_IS_REMOVED(BuildAttributeKind.REBUILD_REASON, "Untracked Java file is removed"),
2526
JAVA_CHANGE_UNEXPECTED_PSI(BuildAttributeKind.REBUILD_REASON, "Java PSI file is expected"),

‎compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCachesManager.kt‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ abstract class IncrementalCachesManager<PlatformCache : AbstractIncrementalCache
3434
private val caches = arrayListOf<BasicMapsOwner>()
3535

3636
var isClosed = false
37+
var isSuccessfulyClosed = false
3738

3839
@Synchronized
3940
protected fun <T : BasicMapsOwner> T.registerCache() {
@@ -52,29 +53,29 @@ abstract class IncrementalCachesManager<PlatformCache : AbstractIncrementalCache
5253
@Synchronized
5354
fun close(flush: Boolean = false): Boolean {
5455
if (isClosed) {
55-
return true
56+
return isSuccessfulyClosed
5657
}
57-
var successful = true
58+
isSuccessfulyClosed = true
5859
for (cache in caches) {
5960
if (flush) {
6061
try {
6162
cache.flush(false)
6263
} catch (e: Throwable) {
63-
successful = false
64+
isSuccessfulyClosed = false
6465
reporter.report { "Exception when flushing cache ${cache.javaClass}: $e" }
6566
}
6667
}
6768

6869
try {
6970
cache.close()
7071
} catch (e: Throwable) {
71-
successful = false
72+
isSuccessfulyClosed = false
7273
reporter.report { "Exception when closing cache ${cache.javaClass}: $e" }
7374
}
7475
}
7576

7677
isClosed = true
77-
return successful
78+
return isSuccessfulyClosed
7879
}
7980
}
8081

‎compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt‎

Lines changed: 85 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,53 @@ abstract class IncrementalCompilerRunner<
7171
providedChangedFiles: ChangedFiles?,
7272
projectDir: File? = null
7373
): ExitCode = reporter.measure(BuildTime.INCREMENTAL_COMPILATION_DAEMON) {
74-
compileImpl(allSourceFiles, args, messageCollector, providedChangedFiles, projectDir)
74+
try {
75+
compileImpl(allSourceFiles, args, messageCollector, providedChangedFiles, projectDir)
76+
} finally {
77+
reporter.measure(BuildTime.CALCULATE_OUTPUT_SIZE) {
78+
reporter.addMetric(
79+
BuildPerformanceMetric.SNAPSHOT_SIZE,
80+
buildHistoryFile.length() + lastBuildInfoFile.length() + abiSnapshotFile.length()
81+
)
82+
if (cacheDirectory.exists() && cacheDirectory.isDirectory()) {
83+
cacheDirectory.walkTopDown().filter { it.isFile }.map { it.length() }.sum().let {
84+
reporter.addMetric(BuildPerformanceMetric.CACHE_DIRECTORY_SIZE, it)
85+
}
86+
}
87+
}
88+
}
89+
}
90+
91+
fun rebuild(
92+
reason: BuildAttribute,
93+
allSourceFiles: List<File>,
94+
args: Args,
95+
messageCollector: MessageCollector,
96+
providedChangedFiles: ChangedFiles?,
97+
projectDir: File? = null,
98+
classpathAbiSnapshot: Map<String, AbiSnapshot>
99+
): ExitCode {
100+
reporter.report { "Non-incremental compilation will be performed: $reason" }
101+
reporter.measure(BuildTime.CLEAR_OUTPUT_ON_REBUILD) {
102+
cleanOutputsAndLocalStateOnRebuild(args)
103+
}
104+
val caches = createCacheManager(args, projectDir)
105+
try {
106+
if (providedChangedFiles == null) {
107+
caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
108+
}
109+
val allKotlinFiles = allSourceFiles.filter { it.isKotlinFile(kotlinSourceFilesExtensions) }
110+
return compileIncrementally(
111+
args, caches, allKotlinFiles, CompilationMode.Rebuild(reason), messageCollector, withAbiSnapshot,
112+
classpathAbiSnapshot = classpathAbiSnapshot
113+
).also {
114+
if (it == ExitCode.OK) {
115+
performWorkAfterSuccessfulCompilation(caches)
116+
}
117+
}
118+
} finally {
119+
caches.close(true)
120+
}
75121
}
76122

77123
private fun compileImpl(
@@ -82,41 +128,19 @@ abstract class IncrementalCompilerRunner<
82128
projectDir: File? = null
83129
): ExitCode {
84130
var caches = createCacheManager(args, projectDir)
131+
var rebuildReason = BuildAttribute.INTERNAL_ERROR
85132

86-
if (withAbiSnapshot) {
87-
reporter.report { "Incremental compilation with ABI snapshot enabled" }
88-
}
89-
//TODO if abi-snapshot is corrupted unable to rebuild. Should roll back to withSnapshot = false?
90133
val classpathAbiSnapshot =
91134
if (withAbiSnapshot) {
135+
reporter.report { "Incremental compilation with ABI snapshot enabled" }
92136
reporter.measure(BuildTime.SET_UP_ABI_SNAPSHOTS) {
93137
setupJarDependencies(args, withAbiSnapshot, reporter)
94138
}
95139
} else {
96140
emptyMap()
97141
}
98142

99-
fun rebuild(reason: BuildAttribute): ExitCode {
100-
reporter.report { "Non-incremental compilation will be performed: $reason" }
101-
caches.close(false)
102-
// todo: we can recompile all files incrementally (not cleaning caches), so rebuild won't propagate
103-
reporter.measure(BuildTime.CLEAR_OUTPUT_ON_REBUILD) {
104-
cleanOutputsAndLocalStateOnRebuild(args)
105-
}
106-
caches = createCacheManager(args, projectDir)
107-
if (providedChangedFiles == null) {
108-
caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
109-
}
110-
val allKotlinFiles = allSourceFiles.filter { it.isKotlinFile(kotlinSourceFilesExtensions) }
111-
return compileIncrementally(
112-
args, caches, allKotlinFiles, CompilationMode.Rebuild(reason), messageCollector, withAbiSnapshot,
113-
classpathAbiSnapshot = classpathAbiSnapshot
114-
)
115-
}
116-
117-
// If compilation has crashed or we failed to close caches we have to clear them
118-
var cachesMayBeCorrupted = true
119-
return try {
143+
try {
120144
val changedFiles = when (providedChangedFiles) {
121145
is ChangedFiles.Dependencies -> {
122146
val changedSources = caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
@@ -129,75 +153,51 @@ abstract class IncrementalCompilerRunner<
129153
else -> providedChangedFiles
130154
}
131155

132-
@Suppress("MoveVariableDeclarationIntoWhen")
133-
val compilationMode = sourcesToCompile(caches, changedFiles, args, messageCollector, classpathAbiSnapshot)
156+
var compilationMode = sourcesToCompile(caches, changedFiles, args, messageCollector, classpathAbiSnapshot)
157+
val abiSnapshot = if (compilationMode is CompilationMode.Incremental && withAbiSnapshot) {
158+
AbiSnapshotImpl.read(abiSnapshotFile, reporter)
159+
} else {
160+
if (withAbiSnapshot) {
161+
compilationMode = CompilationMode.Rebuild(BuildAttribute.NO_ABI_SNAPSHOT)
162+
}
163+
null
164+
}
134165

135-
val exitCode = when (compilationMode) {
166+
when (compilationMode) {
136167
is CompilationMode.Incremental -> {
137-
if (withAbiSnapshot) {
138-
val abiSnapshot = AbiSnapshotImpl.read(abiSnapshotFile, reporter)
139-
if (abiSnapshot != null) {
168+
try {
169+
val exitCode = if (withAbiSnapshot) {
140170
compileIncrementally(
141-
args,
142-
caches,
143-
allSourceFiles,
144-
compilationMode,
145-
messageCollector,
146-
withAbiSnapshot,
147-
abiSnapshot,
148-
classpathAbiSnapshot
171+
args, caches, allSourceFiles, compilationMode, messageCollector,
172+
withAbiSnapshot, abiSnapshot!!, classpathAbiSnapshot
149173
)
150174
} else {
151-
rebuild(BuildAttribute.NO_ABI_SNAPSHOT)
175+
compileIncrementally(args, caches, allSourceFiles, compilationMode, messageCollector, withAbiSnapshot)
152176
}
153-
} else {
154-
compileIncrementally(
155-
args,
156-
caches,
157-
allSourceFiles,
158-
compilationMode,
159-
messageCollector,
160-
withAbiSnapshot
161-
)
162-
}
163-
}
164-
is CompilationMode.Rebuild -> {
165-
rebuild(compilationMode.reason)
166-
}
167-
}
168-
169-
if (exitCode == ExitCode.OK) {
170-
performWorkAfterSuccessfulCompilation(caches)
171-
}
172-
173-
if (!caches.close(flush = true)) throw RuntimeException("Could not flush caches")
174-
// Here we should analyze exit code of compiler. E.g. compiler failure should lead to caches rebuild,
175-
// but now JsKlib compiler reports invalid exit code.
176-
cachesMayBeCorrupted = false
177-
178-
reporter.measure(BuildTime.CALCULATE_OUTPUT_SIZE) {
179-
reporter.addMetric(
180-
BuildPerformanceMetric.SNAPSHOT_SIZE,
181-
buildHistoryFile.length() + lastBuildInfoFile.length() + abiSnapshotFile.length()
182-
)
183-
if (cacheDirectory.exists() && cacheDirectory.isDirectory()) {
184-
cacheDirectory.walkTopDown().filter { it.isFile }.map { it.length() }.sum().let {
185-
reporter.addMetric(BuildPerformanceMetric.CACHE_DIRECTORY_SIZE, it)
177+
if (exitCode == ExitCode.OK) {
178+
performWorkAfterSuccessfulCompilation(caches)
179+
}
180+
return exitCode
181+
} catch (e: Throwable) {
182+
reporter.report {
183+
"Incremental compilation failed: ${e.stackTraceToString()}.\nFalling back to non-incremental compilation."
184+
}
185+
rebuildReason = BuildAttribute.INCREMENTAL_COMPILATION_FAILED
186186
}
187187
}
188+
is CompilationMode.Rebuild -> rebuildReason = compilationMode.reason
188189
}
189-
return exitCode
190-
} catch (e: Exception) { // todo: catch only cache corruption
191-
// todo: warn?
192-
reporter.report { "Possible caches corruption: $e" }
193-
rebuild(BuildAttribute.CACHE_CORRUPTION).also {
194-
cachesMayBeCorrupted = false
190+
} catch (e: Exception) {
191+
reporter.report {
192+
"Incremental compilation analysis failed: ${e.stackTraceToString()}.\nFalling back to non-incremental compilation."
195193
}
196194
} finally {
197-
if (cachesMayBeCorrupted) {
195+
if (!caches.close()) {
196+
reporter.report { "Unable to close IC caches. Cleaning internal state" }
198197
cleanOutputsAndLocalStateOnRebuild(args)
199198
}
200199
}
200+
return rebuild(rebuildReason, allSourceFiles, args, messageCollector, providedChangedFiles, projectDir, classpathAbiSnapshot)
201201
}
202202

203203
/**
@@ -411,7 +411,10 @@ abstract class IncrementalCompilerRunner<
411411
break
412412
}
413413

414-
val (dirtyLookupSymbols, dirtyClassFqNames, forceRecompile) = changesCollector.getDirtyData(listOf(caches.platformCache), reporter)
414+
val (dirtyLookupSymbols, dirtyClassFqNames, forceRecompile) = changesCollector.getDirtyData(
415+
listOf(caches.platformCache),
416+
reporter
417+
)
415418
val compiledInThisIterationSet = sourcesToCompile.toHashSet()
416419

417420
val forceToRecompileFiles = mapClassesFqNamesToFiles(listOf(caches.platformCache), forceRecompile, reporter)

‎libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/IncrementalCompilationMultiProjectIT.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ abstract class BaseIncrementalCompilationMultiProjectIT : IncrementalCompilation
675675
}
676676

677677
build("assemble") {
678-
assertOutputContains("Non-incremental compilation will be performed: CACHE_CORRUPTION")
678+
assertOutputContains("Non-incremental compilation will be performed: INCREMENTAL_COMPILATION_FAILED")
679679
}
680680

681681
val lookupFile = projectPath.resolve("lib/build/kotlin/${compileKotlinTaskName}/cacheable/${compileCacheFolderName}/lookups/file-to-id.tab")

0 commit comments

Comments
 (0)