From 5232f080d683a22c490f4f19fd946d3bebb0fc89 Mon Sep 17 00:00:00 2001 From: Sergey Rostov Date: Mon, 15 Oct 2018 12:26:21 +0300 Subject: [PATCH] JPS: Fix updating cache format version for rebuilt targets by removing i/o optimization Problem: previously, format-version.txt file was deleted while cleaning target output on rebuild. Since directory was deleted, cache attributesDiff stored in memory was not updated. This causes that on saveExpectedStateIfNeeded does nothing. The right way to fix it is move cache version format diff into AbstractIncrementalCache, and override `clean()` method. But this is hard to do for lookup storage, since used compilers set (jvm/js) will known only after context init. This can be done by moving `expected` attributes out of manager, but this is huge change for small benefit. So, the optimal way for now is to write version for each build, even if it is not changed. #KT-27044 Fixed --- .../storage/version/CacheAttributesDiff.kt | 4 ---- .../storage/version/CacheAttributesManager.kt | 23 +++---------------- .../storage/version/CacheVersionManager.kt | 2 +- .../incremental/IncrementalCompilerRunner.kt | 3 +-- .../jetbrains/kotlin/jps/build/KotlinChunk.kt | 2 +- .../kotlin/jps/build/KotlinCompileContext.kt | 4 ++-- .../CompositeLookupsCacheAttributes.kt | 6 ++--- 7 files changed, 11 insertions(+), 33 deletions(-) diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesDiff.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesDiff.kt index fab3d10f29a..791ecf4301f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesDiff.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesDiff.kt @@ -27,10 +27,6 @@ data class CacheAttributesDiff( else CacheStatus.CLEARED } - fun saveExpectedIfNeeded() { - if (expected != actual) manager.writeActualVersion(expected) - } - override fun toString(): String { return "$status: actual=$actual -> expected=$expected" } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesManager.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesManager.kt index 54d176761cb..7f10891577f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesManager.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheAttributesManager.kt @@ -32,11 +32,8 @@ interface CacheAttributesManager { /** * Write [values] as cache attributes for next build execution. - * - * This is internal operation that should be implemented by particular implementation of CacheAttributesManager. - * Consider using `loadDiff().saveExpectedIfNeeded()` for saving attributes values for next build. */ - fun writeActualVersion(values: Attrs?) + fun writeVersion(values: Attrs? = expected) /** * Check if cache with [actual] attributes values can be used when [expected] attributes are required. @@ -52,28 +49,14 @@ fun CacheAttributesManager.loadDiff( fun CacheAttributesManager.loadAndCheckStatus() = loadDiff().status -/** - * This method is kept only for compatibility. - * Save [expected] cache attributes values if it is enabled and not equals to [actual]. - */ -@Deprecated( - message = "Consider using `this.loadDiff().saveExpectedIfNeeded()` and cache `loadDiff()` result.", - replaceWith = ReplaceWith("loadDiff().saveExpectedIfNeeded()") -) -fun CacheAttributesManager.saveIfNeeded( - actual: Attrs? = this.loadActual(), - expected: Attrs = this.expected - ?: error("To save disabled cache status [delete] should be called (this behavior is kept for compatibility)") -) = loadDiff(actual, expected).saveExpectedIfNeeded() - /** * This method is kept only for compatibility. * Delete actual cache attributes values if it existed. */ @Deprecated( message = "Consider using `this.loadDiff().saveExpectedIfNeeded()` and cache `loadDiff()` result.", - replaceWith = ReplaceWith("writeActualVersion(null)") + replaceWith = ReplaceWith("writeVersion(null)") ) fun CacheAttributesManager<*>.clean() { - writeActualVersion(null) + writeVersion(null) } \ No newline at end of file diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheVersionManager.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheVersionManager.kt index 2fdc91155e0..6b4d01f17a0 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheVersionManager.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/version/CacheVersionManager.kt @@ -44,7 +44,7 @@ class CacheVersionManager( null } - override fun writeActualVersion(values: CacheVersion?) { + override fun writeVersion(values: CacheVersion?) { if (values == null) versionFile.delete() else { versionFile.parentFile.mkdirs() 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 12affd8e27d..78443205fc5 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 @@ -30,7 +30,6 @@ import org.jetbrains.kotlin.incremental.components.ExpectActualTracker import org.jetbrains.kotlin.incremental.components.LookupTracker import org.jetbrains.kotlin.incremental.parsing.classesFqNames import org.jetbrains.kotlin.incremental.storage.version.CacheVersionManager -import org.jetbrains.kotlin.incremental.storage.version.saveIfNeeded import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.progress.CompilationCanceledStatus import java.io.File @@ -272,7 +271,7 @@ abstract class IncrementalCompilerRunner< processChangesAfterBuild(compilationMode, currentBuildInfo, dirtyData) if (exitCode == ExitCode.OK) { - cachesVersionManagers.forEach { it.saveIfNeeded() } + cachesVersionManagers.forEach { it.writeVersion() } } return exitCode diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinChunk.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinChunk.kt index 8280c4f2af3..626b78c4458 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinChunk.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinChunk.kt @@ -96,7 +96,7 @@ class KotlinChunk internal constructor(val context: KotlinCompileContext, val ta context.ensureLookupsCacheAttributesSaved() targets.forEach { - it.initialLocalCacheAttributesDiff.saveExpectedIfNeeded() + it.initialLocalCacheAttributesDiff.manager.writeVersion() } val serializedMetaInfo = representativeTarget.buildMetaInfoFactory.serializeToString(compilerArguments) diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinCompileContext.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinCompileContext.kt index 2abc23db574..1eaa8a049d6 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinCompileContext.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/build/KotlinCompileContext.kt @@ -149,7 +149,7 @@ class KotlinCompileContext(val jpsContext: CompileContext) { */ fun ensureLookupsCacheAttributesSaved() { if (lookupAttributesSaved.compareAndSet(false, true)) { - initialLookupsCacheStateDiff.saveExpectedIfNeeded() + initialLookupsCacheStateDiff.manager.writeVersion() } } @@ -207,7 +207,7 @@ class KotlinCompileContext(val jpsContext: CompileContext) { private fun clearLookupCache() { KotlinBuilder.LOG.info("Clearing lookup cache") dataManager.cleanLookupStorage(KotlinBuilder.LOG) - initialLookupsCacheStateDiff.saveExpectedIfNeeded() + initialLookupsCacheStateDiff.manager.writeVersion() } fun cleanupCaches() { diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/CompositeLookupsCacheAttributes.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/CompositeLookupsCacheAttributes.kt index e50c18e9471..5e9c6a9f16f 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/CompositeLookupsCacheAttributes.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/incremental/CompositeLookupsCacheAttributes.kt @@ -47,12 +47,12 @@ class CompositeLookupsCacheAttributesManager( return CompositeLookupsCacheAttributes(version.version, components) } - override fun writeActualVersion(values: CompositeLookupsCacheAttributes?) { + override fun writeVersion(values: CompositeLookupsCacheAttributes?) { if (values == null) { - versionManager.writeActualVersion(null) + versionManager.writeVersion(null) actualComponentsFile.delete() } else { - versionManager.writeActualVersion(CacheVersion(values.version)) + versionManager.writeVersion(CacheVersion(values.version)) actualComponentsFile.parentFile.mkdirs() actualComponentsFile.writeText(values.components.joinToString("\n"))