From ca19d71a004fa2261c4af41383c9135adb6483b5 Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Wed, 16 Nov 2022 22:40:23 +0100 Subject: [PATCH] [JS IR] Commit cache header after lowering Commiting cache header before lowering may break caches in case of lowering errors. ^KT-54912 Fixed --- .../jetbrains/kotlin/cli/js/K2JsIrCompiler.kt | 5 +- .../kotlin/ir/backend/js/ic/CacheUpdater.kt | 22 ++- .../ir/backend/js/ic/IncrementalCache.kt | 129 +++++++++--------- .../incremental/AbstractInvalidationTest.kt | 2 +- .../js/test/ir/AbstractJsKLibABITestCase.kt | 2 +- .../invalidation/enum/lib1/module.info | 2 +- .../invalidation/enum/main/module.info | 2 +- .../kotlin/gradle/Kotlin2JsGradlePluginIT.kt | 19 ++- 8 files changed, 92 insertions(+), 91 deletions(-) diff --git a/compiler/cli/cli-js/src/org/jetbrains/kotlin/cli/js/K2JsIrCompiler.kt b/compiler/cli/cli-js/src/org/jetbrains/kotlin/cli/js/K2JsIrCompiler.kt index cf149dcf5cc..f2b79ae2026 100644 --- a/compiler/cli/cli-js/src/org/jetbrains/kotlin/cli/js/K2JsIrCompiler.kt +++ b/compiler/cli/cli-js/src/org/jetbrains/kotlin/cli/js/K2JsIrCompiler.kt @@ -650,7 +650,7 @@ class K2JsIrCompiler : CLICompiler() { val cacheUpdater = CacheUpdater( mainModule = arguments.includes!!, allModules = libraries, - icCacheRootDir = cacheDirectories.last(), + cacheDir = cacheDirectories.last(), compilerConfiguration = configurationJs, irFactory = { IrFactoryImplForJsIC(WholeWorldStageController()) }, mainArguments = mainCallArguments, @@ -666,8 +666,7 @@ class K2JsIrCompiler : CLICompiler() { var libIndex = 0 for ((libFile, srcFiles) in cacheUpdater.getDirtyFileLastStats()) { val (msg, showFiles) = when { - srcFiles.values.all { it.contains(DirtyFileState.ADDED_FILE) } -> "fully rebuilt due to clean build" to false - srcFiles.values.all { it.contains(DirtyFileState.MODIFIED_CONFIG) } -> "fully rebuilt due to config modification" to false + srcFiles.values.all { it.contains(DirtyFileState.ADDED_FILE) } -> "fully rebuilt" to false else -> "partially rebuilt" to true } messageCollector.report(INFO, "${++libIndex}) module [${File(libFile.path).name}] was $msg") diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/CacheUpdater.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/CacheUpdater.kt index cd7a2343261..06ed9418d36 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/CacheUpdater.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/CacheUpdater.kt @@ -37,7 +37,6 @@ fun interface JsIrCompilerICInterfaceFactory { enum class DirtyFileState(val str: String) { ADDED_FILE("added file"), - MODIFIED_CONFIG("modified config"), MODIFIED_IR("modified ir"), UPDATED_EXPORTS("updated exports"), UPDATED_IMPORTS("updated imports"), @@ -49,7 +48,7 @@ enum class DirtyFileState(val str: String) { class CacheUpdater( mainModule: String, private val allModules: Collection, - private val icCacheRootDir: String, + cacheDir: String, private val compilerConfiguration: CompilerConfiguration, private val irFactory: () -> IrFactory, private val mainArguments: List?, @@ -59,10 +58,10 @@ class CacheUpdater( private val dirtyFileStats = KotlinSourceFileMutableMap>() - private val configHash = compilerConfiguration.configHashForIC() - private val mainLibraryFile = KotlinLibraryFile(File(mainModule).canonicalPath) + private val cacheRootDir = File(cacheDir, "version.${compilerConfiguration.configHashForIC()}") + fun getDirtyFileLastStats(): KotlinSourceFileMap> = dirtyFileStats fun getStopwatchLastLaps() = stopwatch.laps @@ -104,12 +103,10 @@ class CacheUpdater( } } - private val incrementalCaches = stopwatch.measure("Incremental cache - initializing") { - libraryDependencies.keys.associate { lib -> - val libFile = KotlinLibraryFile(lib) - val file = File(libFile.path) - libFile to IncrementalCache(lib, File(icCacheRootDir, "${file.absolutePath.stringHashForIC()}.${file.name}")) - } + private val incrementalCaches = libraryDependencies.keys.associate { lib -> + val libFile = KotlinLibraryFile(lib) + val file = File(libFile.path) + libFile to IncrementalCache(lib, File(cacheRootDir, "${file.name}.${file.absolutePath.stringHashForIC()}")) } private fun getLibIncrementalCache(libFile: KotlinLibraryFile) = @@ -159,14 +156,13 @@ class CacheUpdater( val removedFilesMetadata = hashMapOf>() val modifiedFiles = KotlinSourceFileMap(incrementalCaches.entries.associate { (lib, cache) -> - val (dirtyFiles, removedFiles, newFiles, modifiedConfigFiles) = cache.collectModifiedFiles(configHash) + val (dirtyFiles, removedFiles, newFiles) = cache.collectModifiedFiles() val fileStats by lazy(LazyThreadSafetyMode.NONE) { dirtyFileStats.getOrPutFiles(lib) } newFiles.forEach { fileStats.addDirtFileStat(it, DirtyFileState.ADDED_FILE) } - modifiedConfigFiles.forEach { fileStats.addDirtFileStat(it, DirtyFileState.MODIFIED_CONFIG) } removedFiles.forEach { fileStats.addDirtFileStat(it.key, DirtyFileState.REMOVED_FILE) } dirtyFiles.forEach { - if (it.key !in newFiles && it.key !in modifiedConfigFiles) { + if (it.key !in newFiles) { fileStats.addDirtFileStat(it.key, DirtyFileState.MODIFIED_IR) } } diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/IncrementalCache.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/IncrementalCache.kt index 8967be19877..9a6f33b3ebd 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/IncrementalCache.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/ic/IncrementalCache.kt @@ -12,10 +12,11 @@ import org.jetbrains.kotlin.library.KotlinLibrary import org.jetbrains.kotlin.library.impl.javaFile import org.jetbrains.kotlin.protobuf.CodedInputStream import org.jetbrains.kotlin.protobuf.CodedOutputStream +import org.jetbrains.kotlin.utils.addToStdlib.ifTrue import java.io.File -internal class IncrementalCache(private val library: KotlinLibrary, private val cacheDir: File) { +internal class IncrementalCache(private val library: KotlinLibrary, val cacheDir: File) { companion object { private const val CACHE_HEADER = "ic.header.bin" @@ -24,40 +25,58 @@ internal class IncrementalCache(private val library: KotlinLibrary, private val private const val METADATA_TMP_SUFFIX = "metadata.tmp.bin" } - private var forceRebuildJs = false private val signatureToIndexMappingFromMetadata = hashMapOf>() private val libraryFile = KotlinLibraryFile(library) - class CacheHeader(val klibFileHash: ICHash = ICHash(), val configHash: ICHash = ICHash()) { + private val cacheHeaderFile = File(cacheDir, CACHE_HEADER) + + private var newCacheHeader: CacheHeader? = null + + private var removedSrcFiles: Collection = emptyList() + + private val kotlinLibraryHeader: KotlinLibraryHeader by lazy { KotlinLibraryHeader(library) } + + private class CacheHeader(val klibFileHash: ICHash, val klibFile: KotlinLibraryFile, val fingerprints: Map?) { fun toProtoStream(out: CodedOutputStream) { + klibFile.toProtoStream(out) klibFileHash.toProtoStream(out) - configHash.toProtoStream(out) + + if (fingerprints == null) { + notFoundIcError("fingerprints", klibFile) + } + out.writeInt32NoTag(fingerprints.size) + for ((srcFile, fingerprint) in fingerprints) { + srcFile.toProtoStream(out) + fingerprint.toProtoStream(out) + } } companion object { - fun fromProtoStream(input: CodedInputStream): CacheHeader { - val klibFileHash = ICHash.fromProtoStream(input) - val configHash = ICHash.fromProtoStream(input) - return CacheHeader(klibFileHash, configHash) + fun fromProtoStream(input: CodedInputStream, newKlibFileHash: ICHash): CacheHeader { + val klibFile = KotlinLibraryFile.fromProtoStream(input) + val oldKlibFileHash = ICHash.fromProtoStream(input) + + val fingerprints = (oldKlibFileHash != newKlibFileHash).ifTrue { + buildMapUntil(input.readInt32()) { + val file = KotlinSourceFile.fromProtoStream(input) + put(file, ICHash.fromProtoStream(input)) + } + } + return CacheHeader(oldKlibFileHash, klibFile, fingerprints) } } } - private var cacheHeader = File(cacheDir, CACHE_HEADER).useCodedInputIfExists { - CacheHeader.fromProtoStream(this) - } ?: CacheHeader() - - private fun loadCachedFingerprints() = File(cacheDir, CACHE_HEADER).useCodedInputIfExists { - // skip cache header - CacheHeader.fromProtoStream(this@useCodedInputIfExists) - buildMapUntil(readInt32()) { - val file = KotlinSourceFile.fromProtoStream(this@useCodedInputIfExists) - put(file, ICHash.fromProtoStream(this@useCodedInputIfExists)) + private fun loadCacheHeader(newKlibFileHash: ICHash): CacheHeader? { + return cacheHeaderFile.useCodedInputIfExists { + val cacheHeader = CacheHeader.fromProtoStream(this, newKlibFileHash) + if (cacheHeader.klibFile != libraryFile) { + icError("broken incremental cache header $cacheHeaderFile; expected $libraryFile, got ${cacheHeader.klibFile}") + } + cacheHeader } - } ?: emptyMap() - - private val kotlinLibraryHeader: KotlinLibraryHeader by lazy { KotlinLibraryHeader(library) } + } private class KotlinSourceFileMetadataFromDisk( override val inverseDependencies: KotlinSourceFileMap>, @@ -73,53 +92,46 @@ internal class IncrementalCache(private val library: KotlinLibrary, private val private fun KotlinSourceFile.getCacheFile(suffix: String) = File(cacheDir, "${File(path).name}.${path.stringHashForIC()}.$suffix") - private fun commitCacheHeader(fingerprints: List>) = File(cacheDir, CACHE_HEADER).useCodedOutput { - cacheHeader.toProtoStream(this) - writeInt32NoTag(fingerprints.size) - for ((srcFile, fingerprint) in fingerprints) { - srcFile.toProtoStream(this) - fingerprint.toProtoStream(this) - } - } - fun buildIncrementalCacheArtifact(signatureToIndexMapping: Map>): IncrementalCacheArtifact { + newCacheHeader?.let { cacheHeader -> + cacheHeaderFile.useCodedOutput { cacheHeader.toProtoStream(this) } + } + + for (removedFile in removedSrcFiles) { + removedFile.getCacheFile(BINARY_AST_SUFFIX).delete() + removedFile.getCacheFile(METADATA_SUFFIX).delete() + } + val fileArtifacts = kotlinLibraryHeader.sourceFiles.map { srcFile -> commitSourceFileMetadata(srcFile.getCacheFile(BINARY_AST_SUFFIX), srcFile, signatureToIndexMapping[srcFile] ?: emptyMap()) } - return IncrementalCacheArtifact(cacheDir, forceRebuildJs, fileArtifacts, library.jsOutputName) + return IncrementalCacheArtifact(cacheDir, removedSrcFiles.isNotEmpty(), fileArtifacts, library.jsOutputName) } data class ModifiedFiles( val dirtyFiles: Map = emptyMap(), val removedFiles: Map = emptyMap(), val newFiles: Set = emptySet(), - val modifiedConfigFiles: Set = emptySet(), ) - fun collectModifiedFiles(configHash: ICHash): ModifiedFiles { - var isConfigModified = false + fun collectModifiedFiles(): ModifiedFiles { val klibFileHash = library.libraryFile.javaFile().fileHashForIC() - cacheHeader = when { - cacheHeader.configHash != configHash -> { - cacheDir.deleteRecursively() - isConfigModified = cacheHeader.configHash != ICHash() - CacheHeader(klibFileHash, configHash) - } - cacheHeader.klibFileHash != klibFileHash -> CacheHeader(klibFileHash, configHash) - else -> return ModifiedFiles() - } + val cacheHeader = loadCacheHeader(klibFileHash) + + val cachedFingerprints = cacheHeader?.let { it.fingerprints ?: return ModifiedFiles() } ?: emptyMap() - val cachedFingerprints = loadCachedFingerprints() val deletedFiles = HashSet(cachedFingerprints.keys) - val unknownFiles = mutableSetOf() + val newFiles = mutableSetOf() + + val newFingerprints = HashMap(kotlinLibraryHeader.sourceFiles.size) + kotlinLibraryHeader.sourceFiles.forEachIndexed { index, file -> newFingerprints[file] = library.fingerprint(index) } - val newFingerprints = kotlinLibraryHeader.sourceFiles.mapIndexed { index, file -> file to library.fingerprint(index) } val modifiedFiles = HashMap(newFingerprints.size).apply { for ((file, fileNewFingerprint) in newFingerprints) { val oldFingerprint = cachedFingerprints[file] if (oldFingerprint == null) { - unknownFiles += file + newFiles += file } if (oldFingerprint != fileNewFingerprint) { val metadata = fetchSourceFileMetadata(file, false) @@ -129,23 +141,12 @@ internal class IncrementalCache(private val library: KotlinLibrary, private val } } - val removedFilesMetadata = deletedFiles.associateWith { - val metadata = fetchSourceFileMetadata(it, false) - it.getCacheFile(BINARY_AST_SUFFIX).delete() - it.getCacheFile(METADATA_SUFFIX).delete() - metadata - } + val removedFiles = deletedFiles.associateWith { fetchSourceFileMetadata(it, false) } - forceRebuildJs = deletedFiles.isNotEmpty() - commitCacheHeader(newFingerprints) + removedSrcFiles = deletedFiles + newCacheHeader = CacheHeader(klibFileHash, libraryFile, newFingerprints) - val (newFiles, modifiedConfigFiles) = if (isConfigModified) { - emptySet() to unknownFiles - } else { - unknownFiles to emptySet() - } - - return ModifiedFiles(modifiedFiles, removedFilesMetadata, newFiles, modifiedConfigFiles) + return ModifiedFiles(modifiedFiles, removedFiles, newFiles) } fun fetchSourceFileFullMetadata(srcFile: KotlinSourceFile): KotlinSourceFileMetadata { @@ -170,12 +171,12 @@ internal class IncrementalCache(private val library: KotlinLibrary, private val } fun CodedInputStream.readDependencies(signaturesReader: () -> T) = buildMapUntil(readInt32()) { - val libraryFile = KotlinLibraryFile.fromProtoStream(this@readDependencies) + val libFile = KotlinLibraryFile.fromProtoStream(this@readDependencies) val depends = buildMapUntil(readInt32()) { val dependencySrcFile = KotlinSourceFile.fromProtoStream(this@readDependencies) put(dependencySrcFile, signaturesReader()) } - put(libraryFile, depends) + put(libFile, depends) } fun CodedInputStream.readDirectDependencies() = readDependencies { diff --git a/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt b/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt index 9b98544afc3..bab14adc614 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt @@ -229,7 +229,7 @@ abstract class AbstractInvalidationTest : KotlinTestWithEnvironment() { val cacheUpdater = CacheUpdater( mainModule = testInfo.last().modulePath, allModules = testInfo.mapTo(mutableListOf(STDLIB_KLIB)) { it.modulePath }, - icCacheRootDir = buildDir.resolve("incremental-cache").absolutePath, + cacheDir = buildDir.resolve("incremental-cache").absolutePath, compilerConfiguration = configuration, irFactory = { IrFactoryImplForJsIC(WholeWorldStageController()) }, mainArguments = null, diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt index 4ba6b9f267b..3d6a45721ec 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/ir/AbstractJsKLibABITestCase.kt @@ -193,7 +193,7 @@ abstract class AbstractJsKLibABITestCase : KtUsefulTestCase() { val cacheUpdater = CacheUpdater( mainModule = mainModuleKlibFile.absolutePath, allModules = allDependencies.regularDependencies.map { it.path }, - icCacheRootDir = buildDir.resolve("libs-cache").absolutePath, + cacheDir = buildDir.resolve("libs-cache").absolutePath, compilerConfiguration = configuration, irFactory = { IrFactoryImplForJsIC(WholeWorldStageController()) }, mainArguments = null, diff --git a/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info b/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info index 576ac253fac..4b44dcc7dd3 100644 --- a/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info +++ b/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info @@ -7,6 +7,6 @@ STEP 1: U : l1.1.kt -> l1.kt modified ir: l1.kt STEP 2: - modified config: l1.kt + added file: l1.kt STEP 3: updated exports: l1.kt diff --git a/js/js.translator/testData/incremental/invalidation/enum/main/module.info b/js/js.translator/testData/incremental/invalidation/enum/main/module.info index 5ddc8d4fbf2..084c80926b4 100644 --- a/js/js.translator/testData/incremental/invalidation/enum/main/module.info +++ b/js/js.translator/testData/incremental/invalidation/enum/main/module.info @@ -7,7 +7,7 @@ STEP 1: dependencies: lib1 STEP 2: dependencies: lib1 - modified config: m.kt, testEnumEntries.kt, testEnumValues.kt + added file: m.kt, testEnumEntries.kt, testEnumValues.kt STEP 3: modifications: U : testEnumEntries.3.kt -> testEnumEntries.kt diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt index b890761b634..8ace11002e2 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt @@ -176,18 +176,23 @@ class Kotlin2JsIrGradlePluginIT : AbstractKotlin2JsGradlePluginIT(true) { fun testJsIrIncrementalMultipleArtifacts(gradleVersion: GradleVersion) { project("kotlin-js-ir-ic-multiple-artifacts", gradleVersion) { build("compileDevelopmentExecutableKotlinJs") { - val cacheDir = projectPath.resolve("app/build/klib/cache/lib/") - .toFile() - assertTrue("Lib cache size should be 2") { - cacheDir - .list() - ?.size == 2 + val cacheDir = projectPath.resolve("app/build/klib/cache/").toFile() + val cacheRootDirName = cacheDir.list()?.singleOrNull() + assertTrue("Lib cache root dir should contain 1 element 'version.hash'") { + cacheRootDirName?.startsWith("version.") ?: false } + val cacheRootDir = cacheDir.resolve(cacheRootDirName!!) + val klibCacheDirs = cacheRootDir.list() + // 2 for lib.klib + 1 for stdlib + 1 for main + assertEquals(4, klibCacheDirs?.size, "cache should contain 4 dirs") + + val libKlibCacheDirs = klibCacheDirs?.filter { dir -> dir.startsWith("lib.klib.") } + assertEquals(2, libKlibCacheDirs?.size, "cache should contain 2 dirs for lib.klib") var lib = false var libOther = false - cacheDir.listFiles()!! + cacheRootDir.listFiles()!! .forEach { it.listFiles()!! .filter { it.isFile }