From 4e89dcf031c75c538dcff9f4b52e5823a9ea8759 Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Fri, 25 Aug 2023 20:12:52 +0100 Subject: [PATCH] [IC] Refactor IC maps to reuse code and ensure consistency The key changes include: - Top interface: `PersistentStorage` - Implementation: + `LazyStorage` + `InMemoryStorage` - Extended functionality: + `BasicMap` + `PersistentStorageAdapter` The remaining changes are small cleanups to adapt to the above key changes. Test: Existing tests (refactoring change) Bug: N/A (code cleanup) --- .../incremental/CompilationTransaction.kt | 10 +- .../IncrementalCompilationContext.kt | 6 +- .../kotlin/incremental/IncrementalJsCache.kt | 8 +- .../kotlin/incremental/IncrementalJvmCache.kt | 17 +- .../kotlin/incremental/LookupStorage.kt | 14 +- .../kotlin/incremental/storage/BasicMap.kt | 146 ++++++++------- .../incremental/storage/BasicMapsOwner.kt | 28 +-- .../incremental/storage/CachingLazyStorage.kt | 158 ---------------- .../incremental/storage/ClassOneToManyMap.kt | 9 +- .../storage/ComplementarySourceFilesMap.kt | 10 +- .../kotlin/incremental/storage/IdToFileMap.kt | 9 +- .../incremental/storage/InMemoryStorage.kt | 162 +++++++++++++++++ .../storage/InMemoryStorageWrapper.kt | 171 ------------------ .../kotlin/incremental/storage/LazyStorage.kt | 169 +++++++++++++++-- .../kotlin/incremental/storage/LookupMap.kt | 18 +- .../incremental/storage/PersistentStorage.kt | 116 ++++++++++++ .../storage/PersistentStorageAdapter.kt | 76 ++++++++ .../storage/SourceToJsOutputMap.kt | 9 +- .../incremental/storage/SourceToOutputMaps.kt | 17 +- .../incremental/storage/externalizers.kt | 119 ++++++------ .../incremental/CompilationTransactionTest.kt | 21 +-- ...eWrapperTest.kt => InMemoryStorageTest.kt} | 7 +- .../incremental/IncrementalCachesManager.kt | 2 - .../storage/SourceToOutputFilesMap.kt | 3 +- .../snapshots/FileSnapshotMapTest.kt | 3 +- .../storage/SourceToOutputFilesMapTest.kt | 3 +- 26 files changed, 730 insertions(+), 581 deletions(-) delete mode 100644 build-common/src/org/jetbrains/kotlin/incremental/storage/CachingLazyStorage.kt create mode 100644 build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt delete mode 100644 build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt create mode 100644 build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt create mode 100644 build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorageAdapter.kt rename build-common/test/org/jetbrains/kotlin/incremental/storage/{InMemoryStorageWrapperTest.kt => InMemoryStorageTest.kt} (97%) diff --git a/build-common/src/org/jetbrains/kotlin/incremental/CompilationTransaction.kt b/build-common/src/org/jetbrains/kotlin/incremental/CompilationTransaction.kt index 83fb6e8fd4e..63503e0052f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/CompilationTransaction.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/CompilationTransaction.kt @@ -11,7 +11,7 @@ import org.jetbrains.kotlin.build.report.metrics.GradleBuildPerformanceMetric import org.jetbrains.kotlin.build.report.metrics.GradleBuildTime import org.jetbrains.kotlin.build.report.metrics.measure import org.jetbrains.kotlin.compilerRunner.OutputItemsCollector -import org.jetbrains.kotlin.incremental.storage.InMemoryStorageWrapper +import org.jetbrains.kotlin.incremental.storage.InMemoryStorageInterface import org.jetbrains.kotlin.konan.file.use import java.io.Closeable import java.io.File @@ -48,7 +48,7 @@ interface CompilationTransaction : Closeable { */ var executionThrowable: Throwable? - fun registerInMemoryStorageWrapper(inMemoryStorageWrapper: InMemoryStorageWrapper<*, *>) + fun registerInMemoryStorageWrapper(inMemoryStorageWrapper: InMemoryStorageInterface<*, *>) } fun CompilationTransaction.write(file: Path, writeAction: () -> Unit) { @@ -86,7 +86,7 @@ inline fun CompilationTransaction.runWithin( } abstract class BaseCompilationTransaction : CompilationTransaction { - private val inMemoryStorageWrappers = hashSetOf>() + private val inMemoryStorageWrappers = hashSetOf>() protected var isSuccessful: Boolean = false @@ -94,7 +94,7 @@ abstract class BaseCompilationTransaction : CompilationTransaction { isSuccessful = true } - override fun registerInMemoryStorageWrapper(inMemoryStorageWrapper: InMemoryStorageWrapper<*, *>) { + override fun registerInMemoryStorageWrapper(inMemoryStorageWrapper: InMemoryStorageInterface<*, *>) { inMemoryStorageWrappers.add(inMemoryStorageWrapper) } @@ -117,7 +117,7 @@ abstract class BaseCompilationTransaction : CompilationTransaction { protected fun closeCachesManager() = runCatching { if (!isSuccessful) { for (wrapper in inMemoryStorageWrappers) { - wrapper.resetInMemoryChanges() + wrapper.clearChanges() } } cachesManager?.close() diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt index 5487d24abf7..65491d901b4 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt @@ -11,7 +11,7 @@ import org.jetbrains.kotlin.incremental.storage.FileToAbsolutePathConverter import org.jetbrains.kotlin.incremental.storage.FileToPathConverter class IncrementalCompilationContext( - // The root directories of source files and class files are different, so we may need different `FileToPathConverter`s + // The root directories of source files and output files are different, so we may need different `FileToPathConverter`s val pathConverterForSourceFiles: FileToPathConverter = FileToAbsolutePathConverter, val pathConverterForOutputFiles: FileToPathConverter = FileToAbsolutePathConverter, val storeFullFqNamesInLookupCache: Boolean = false, @@ -47,6 +47,6 @@ class IncrementalCompilationContext( ) // FIXME: Remove `pathConverter` and require its users to decide whether to use `pathConverterForSourceFiles` or - // `pathConverterForClassFiles` + // `pathConverterForOutputFiles` val pathConverter = pathConverterForSourceFiles -} \ No newline at end of file +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt index 70198c56b75..89df06d1af8 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt @@ -154,7 +154,7 @@ open class IncrementalJsCache( } removeAllFromClassStorage(dirtyOutputClassesMap.getDirtyOutputClasses(), changesCollector) dirtySources.clear() - dirtyOutputClassesMap.clean() + dirtyOutputClassesMap.clear() } fun nonDirtyPackageParts(): Map = @@ -447,13 +447,7 @@ private class PackageMetadataMap( storage[packageName] = newMetadata } - fun remove(packageName: String) { - storage.remove(packageName) - } - fun keys() = storage.keys - operator fun get(packageName: String) = storage[packageName] - override fun dumpValue(value: ByteArray): String = "Package metadata: ${value.md5()}" } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt index 598942de613..ada7dd9a3e8 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt @@ -94,7 +94,7 @@ open class IncrementalJvmCache( sourceToClassesMap.contains(file) fun sourcesByInternalName(internalName: String): Collection = - internalNameToSource[internalName] + internalNameToSource.getFiles(internalName) fun getAllPartsOfMultifileFacade(facade: JvmClassName): Collection? { return multifileFacadeToParts[facade] @@ -293,7 +293,7 @@ open class IncrementalJvmCache( } removeAllFromClassStorage(dirtyClasses.map { it.fqNameForClassNameWithoutDollars }, changesCollector) - dirtyOutputClassesMap.clean() + dirtyOutputClassesMap.clear() } override fun getObsoletePackageParts(): Collection { @@ -542,11 +542,6 @@ open class IncrementalJvmCache( ) : BasicStringMap(storageFile, EnumeratorStringDescriptor.INSTANCE, icContext) { - @Synchronized - fun set(partName: String, facadeName: String) { - storage[partName] = facadeName - } - fun get(partName: JvmClassName): String? = storage[partName.internalName] @@ -566,13 +561,9 @@ open class IncrementalJvmCache( storage[internalName] = pathConverter.toPaths(sourceFiles) } - operator fun get(internalName: String): Collection = + fun getFiles(internalName: String): Collection = pathConverter.toFiles(storage[internalName] ?: emptyList()) - fun remove(internalName: String) { - storage.remove(internalName) - } - override fun dumpValue(value: Collection): String = value.dumpCollection() } @@ -638,7 +629,7 @@ open class IncrementalJvmCache( } private object PathCollectionExternalizer : - CollectionExternalizer(PathStringDescriptor, { THashSet(CollectionFactory.createFilePathSet()) }) + CollectionExternalizerV2>(PathStringDescriptor, { THashSet(CollectionFactory.createFilePathSet()) }) sealed class ChangeInfo(val fqName: FqName) { open class MembersChanged(fqName: FqName, val names: Collection) : ChangeInfo(fqName) { diff --git a/build-common/src/org/jetbrains/kotlin/incremental/LookupStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/LookupStorage.kt index 9ce51d77fbc..97b4a2e37e4 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/LookupStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/LookupStorage.kt @@ -96,7 +96,7 @@ open class LookupStorage( val filtered = mutableSetOf() for (fileId in fileIds) { - val path = idToFile[fileId]?.path + val path = idToFile.getFile(fileId)?.path if (path != null) { paths.add(path) @@ -135,16 +135,16 @@ open class LookupStorage( } @Synchronized - override fun clean() { + override fun deleteStorageFiles() { icContext.transaction.deleteFile(countersFile.toPath()) size = 0 - super.clean() + super.deleteStorageFiles() } @Synchronized - override fun flush(memoryCachesOnly: Boolean) { + override fun close() { try { if (size != oldSize) { if (size > 0) { @@ -152,7 +152,7 @@ open class LookupStorage( } } } finally { - super.flush(memoryCachesOnly) + super.close() } } @@ -173,8 +173,8 @@ open class LookupStorage( val oldFileToId = fileToId.toMap() val oldIdToNewId = HashMap(oldFileToId.size) - idToFile.clean() - fileToId.clean() + idToFile.clear() + fileToId.clear() size = 0 for ((file, oldId) in oldFileToId.entries.sortedBy { it.key.path }) { diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt index aa1dd379afb..6bf77b461fe 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt @@ -18,111 +18,127 @@ package org.jetbrains.kotlin.incremental.storage import com.intellij.util.io.DataExternalizer import com.intellij.util.io.EnumeratorStringDescriptor +import com.intellij.util.io.IOUtil import com.intellij.util.io.KeyDescriptor import org.jetbrains.annotations.TestOnly import org.jetbrains.kotlin.incremental.IncrementalCompilationContext import org.jetbrains.kotlin.utils.Printer import java.io.File -abstract class BasicMap, V, StorageType : LazyStorage>( - internal val storageFile: File, - protected val storage: StorageType, - protected val icContext: IncrementalCompilationContext, -) { - protected val pathConverter - get() = icContext.pathConverter +/** [PersistentStorage] that provides a few extra utility methods. */ +interface BasicMap : PersistentStorage { - fun clean() { - storage.clean() + /** Removes all entries. */ + fun clear() { + keys.forEach { remove(it) } } + /** + * DEPRECATED: This method should be removed because + * - The `memoryCachesOnly` parameter is not used (it is always `false`). + * - There is currently no good use case for flushing. In fact, the current implementation of this method does nothing. + */ fun flush(memoryCachesOnly: Boolean) { - storage.flush(memoryCachesOnly) + check(!memoryCachesOnly) { "Expected memoryCachesOnly = false but it is `true`" } } - // avoid unsynchronized close - fun close() { - storage.close() + /** + * Deletes [storageFile] or a group of files associated with [storageFile] (e.g., an implementation of [PersistentStorage] may use a + * [com.intellij.util.io.PersistentHashMap], which creates files such as "storageFile.tab", "storageFile.tab.len", etc.). + * + * Make sure the storage has been closed first before calling this method. + */ + fun deleteStorageFiles() { + check(IOUtil.deleteAllFilesStartingWith(storageFile)) { + "Unable to delete storage file(s) with name prefix: ${storageFile.path}" + } } - @TestOnly - fun closeForTest() { + /** + * DEPRECATED: This method should be removed because: + * - The method name is ambiguous (it may be confused with [clear], and it does not exactly describe the current implementation). + * - The method currently calls close(). However, close() is often already called separately and automatically, so calling this method + * means that close() will likely be called twice. + * Instead, just inline this method or call either close() or deleteStorageFiles() directly. + */ + fun clean() { close() + deleteStorageFiles() } @TestOnly fun dump(): String { - return with(StringBuilder()) { - with(Printer(this)) { + return StringBuilder().apply { + Printer(this).apply { println("${storageFile.name.substringBefore(".tab")} (${this@BasicMap::class.java.simpleName})") pushIndent() - for (key in storage.keys.sorted()) { - println("${dumpKey(key)} -> ${dumpValue(storage[key]!!)}") + for (key in keys.sortedBy { dumpKey(it) }) { + println("${dumpKey(key)} -> ${dumpValue(this@BasicMap[key]!!)}") } popIndent() } - - this }.toString() } @TestOnly - protected abstract fun dumpKey(key: K): String + fun dumpKey(key: KEY): String = key.toString() @TestOnly - protected abstract fun dumpValue(value: V): String + fun dumpValue(value: VALUE): String = value.toString() } -abstract class NonAppendableBasicMap, V>( - storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: DataExternalizer, - icContext: IncrementalCompilationContext, -) : BasicMap>( - storageFile, - createLazyStorage(storageFile, keyDescriptor, valueExternalizer, icContext), - icContext -) +abstract class BasicMapBase( + protected val storage: PersistentStorage, +) : PersistentStorageWrapper(storage), BasicMap -abstract class AppendableBasicMap, V>( - storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: AppendableDataExternalizer, - icContext: IncrementalCompilationContext, -) : BasicMap>( - storageFile, - createLazyStorage(storageFile, keyDescriptor, valueExternalizer, icContext), - icContext -) +abstract class AppendableBasicMapBase>( + protected val storage: AppendablePersistentStorage, +) : AppendablePersistentStorageWrapper(storage), BasicMap -abstract class BasicStringMap( +abstract class AbstractBasicMap( storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: DataExternalizer, - icContext: IncrementalCompilationContext, -) : NonAppendableBasicMap(storageFile, keyDescriptor, valueExternalizer, icContext) { - constructor( - storageFile: File, - valueExternalizer: DataExternalizer, - icContext: IncrementalCompilationContext, - ) : this(storageFile, EnumeratorStringDescriptor.INSTANCE, valueExternalizer, icContext) - - override fun dumpKey(key: String): String = key + keyDescriptor: KeyDescriptor, + valueExternalizer: DataExternalizer, + protected val icContext: IncrementalCompilationContext, +) : BasicMapBase( + createPersistentStorage(storageFile, keyDescriptor, valueExternalizer, icContext) +) { + protected val pathConverter + get() = icContext.pathConverter } -abstract class AppendableBasicStringMap( +abstract class AppendableAbstractBasicMap>( + storageFile: File, + keyDescriptor: KeyDescriptor, + elementExternalizer: DataExternalizer, + protected val icContext: IncrementalCompilationContext, +) : AppendableBasicMapBase( + createAppendablePersistentStorage(storageFile, keyDescriptor, elementExternalizer, icContext) +) { + protected val pathConverter + get() = icContext.pathConverter +} + +abstract class BasicStringMap( storageFile: File, keyDescriptor: KeyDescriptor, - valueExternalizer: AppendableDataExternalizer, + valueExternalizer: DataExternalizer, icContext: IncrementalCompilationContext, -) : AppendableBasicMap(storageFile, keyDescriptor, valueExternalizer, icContext) { - constructor( - storageFile: File, - valueExternalizer: AppendableDataExternalizer, - icContext: IncrementalCompilationContext, - ) : this(storageFile, EnumeratorStringDescriptor.INSTANCE, valueExternalizer, icContext) +) : AbstractBasicMap(storageFile, keyDescriptor, valueExternalizer, icContext) { - override fun dumpKey(key: String): String = key -} \ No newline at end of file + constructor(storageFile: File, valueExternalizer: DataExternalizer, icContext: IncrementalCompilationContext) : + this(storageFile, EnumeratorStringDescriptor.INSTANCE, valueExternalizer, icContext) +} + +abstract class AppendableBasicStringMap>( + storageFile: File, + keyDescriptor: KeyDescriptor, + elementExternalizer: DataExternalizer, + icContext: IncrementalCompilationContext, +) : AppendableAbstractBasicMap(storageFile, keyDescriptor, elementExternalizer, icContext) { + + constructor(storageFile: File, elementExternalizer: DataExternalizer, icContext: IncrementalCompilationContext) : + this(storageFile, EnumeratorStringDescriptor.INSTANCE, elementExternalizer, icContext) +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt index 25ec537fd02..ecb6f052f1f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMapsOwner.kt @@ -21,7 +21,7 @@ import java.io.File import java.io.IOException open class BasicMapsOwner(val cachesDir: File) { - private val maps = arrayListOf>() + private val maps = arrayListOf>() companion object { val CACHE_EXTENSION = "tab" @@ -31,25 +31,29 @@ open class BasicMapsOwner(val cachesDir: File) { get() = File(cachesDir, this + "." + CACHE_EXTENSION) @Synchronized - protected fun > registerMap(map: M): M { + protected fun > registerMap(map: M): M { maps.add(map) return map } - open fun clean() { - forEachMapSafe("clean", BasicMap<*, *, *>::clean) - } - - open fun close() { - forEachMapSafe("close", BasicMap<*, *, *>::close) - } - - open fun flush(memoryCachesOnly: Boolean) { + fun flush(memoryCachesOnly: Boolean) { forEachMapSafe("flush") { it.flush(memoryCachesOnly) } } + open fun close() { + forEachMapSafe("close", BasicMap<*, *>::close) + } + + open fun deleteStorageFiles() { + forEachMapSafe("deleteStorageFiles", BasicMap<*, *>::deleteStorageFiles) + } + + fun clean() { + forEachMapSafe("clean", BasicMap<*, *>::clean) + } + @Synchronized - private fun forEachMapSafe(actionName: String, action: (BasicMap<*, *, *>) -> Unit) { + private fun forEachMapSafe(actionName: String, action: (BasicMap<*, *>) -> Unit) { val actionExceptions = LinkedHashMap() maps.forEach { try { diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/CachingLazyStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/CachingLazyStorage.kt deleted file mode 100644 index 3b0864aa572..00000000000 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/CachingLazyStorage.kt +++ /dev/null @@ -1,158 +0,0 @@ -/* - * Copyright 2010-2015 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.jetbrains.kotlin.incremental.storage - -import com.intellij.util.CommonProcessors -import com.intellij.util.io.AppendablePersistentMap -import com.intellij.util.io.DataExternalizer -import com.intellij.util.io.IOUtil -import com.intellij.util.io.KeyDescriptor -import com.intellij.util.io.PersistentHashMap -import org.jetbrains.kotlin.incremental.IncrementalCompilationContext -import java.io.File -import java.io.IOException - - -/** - * It's lazy in a sense that PersistentHashMap is created only on write - */ -class CachingLazyStorage( - private val storageFile: File, - private val keyDescriptor: KeyDescriptor, - private val valueExternalizer: DataExternalizer -) : AppendableLazyStorage { - private var storage: PersistentHashMap? = null - private var isStorageFileExist = true - - private fun getStorageIfExists(): PersistentHashMap? { - if (storage != null) return storage - - if (!isStorageFileExist) return null - - if (storageFile.exists()) { - storage = createMap() - return storage - } - - isStorageFileExist = false - return null - } - - private fun getStorageOrCreateNew(): PersistentHashMap { - if (storage == null) { - storage = createMap() - } - return storage!! - } - - override val keys: Collection - @Synchronized - get() = buildList { - getStorageIfExists()?.processKeysWithExistingMapping(CommonProcessors.CollectProcessor(this)) - } - - @Synchronized - override operator fun contains(key: K): Boolean = - getStorageIfExists()?.containsMapping(key) ?: false - - @Synchronized - override operator fun get(key: K): V? = - getStorageIfExists()?.get(key) - - @Synchronized - override operator fun set(key: K, value: V) { - getStorageOrCreateNew().put(key, value) - } - - @Synchronized - override fun remove(key: K) { - getStorageIfExists()?.remove(key) - } - - @Synchronized - override fun append(key: K, value: V) { - check(valueExternalizer is AppendableDataExternalizer<*>) { - "`valueExternalizer` should implement the `AppendableDataExternalizer` interface to be able to call `append`" - } - getStorageOrCreateNew().appendData(key, AppendablePersistentMap.ValueDataAppender { valueExternalizer.save(it, value) }) - } - - @Synchronized - override fun clean() { - try { - storage?.close() - } finally { - storage = null - if (!IOUtil.deleteAllFilesStartingWith(storageFile)) { - throw IOException("Could not delete internal storage: ${storageFile.absolutePath}") - } - } - } - - @Synchronized - override fun flush(memoryCachesOnly: Boolean) { - val existingStorage = storage ?: return - - if (memoryCachesOnly) { - if (existingStorage.isDirty) { - existingStorage.dropMemoryCaches() - } - } else { - existingStorage.force() - } - } - - @Synchronized - override fun close() { - try { - storage?.close() - } finally { - storage = null - } - } - - private fun createMap(): PersistentHashMap = PersistentHashMap(storageFile, keyDescriptor, valueExternalizer) -} - -private fun createLazyStorageImpl( - storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: DataExternalizer, - icContext: IncrementalCompilationContext, -) = CachingLazyStorage(storageFile, keyDescriptor, valueExternalizer).let { - if (icContext.keepIncrementalCompilationCachesInMemory) { - DefaultInMemoryStorageWrapper(it, valueExternalizer).also { wrapper -> - icContext.transaction.registerInMemoryStorageWrapper(wrapper) - } - } else { - it - } -} - -fun createLazyStorage( - storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: DataExternalizer, - icContext: IncrementalCompilationContext, -): LazyStorage = createLazyStorageImpl(storageFile, keyDescriptor, valueExternalizer, icContext) - -fun createLazyStorage( - storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: AppendableDataExternalizer, - icContext: IncrementalCompilationContext, -): AppendableLazyStorage = createLazyStorageImpl(storageFile, keyDescriptor, valueExternalizer, icContext) \ No newline at end of file diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/ClassOneToManyMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/ClassOneToManyMap.kt index 597b982588f..adba75ae10b 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/ClassOneToManyMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/ClassOneToManyMap.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.incremental.storage +import com.intellij.util.io.EnumeratorStringDescriptor import org.jetbrains.kotlin.incremental.IncrementalCompilationContext import org.jetbrains.kotlin.incremental.dumpCollection import org.jetbrains.kotlin.name.FqName @@ -24,17 +25,17 @@ import java.io.File internal open class ClassOneToManyMap( storageFile: File, icContext: IncrementalCompilationContext, -) : AppendableBasicStringMap>(storageFile, StringCollectionExternalizer, icContext) { - override fun dumpValue(value: Collection): String = value.dumpCollection() +) : AppendableBasicStringMap>(storageFile, EnumeratorStringDescriptor.INSTANCE, icContext) { + override fun dumpValue(value: Collection): String = value.toSet().dumpCollection() @Synchronized fun add(key: FqName, value: FqName) { - storage.append(key.asString(), listOf(value.asString())) + storage.append(key.asString(), value.asString()) } @Synchronized operator fun get(key: FqName): Collection = - storage[key.asString()]?.map(::FqName) ?: setOf() + storage[key.asString()]?.mapTo(mutableSetOf(), ::FqName) ?: setOf() @Synchronized operator fun set(key: FqName, values: Collection) { diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/ComplementarySourceFilesMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/ComplementarySourceFilesMap.kt index fae06e49720..33057321049 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/ComplementarySourceFilesMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/ComplementarySourceFilesMap.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.incremental.storage +import com.intellij.util.io.EnumeratorStringDescriptor import org.jetbrains.kotlin.incremental.IncrementalCompilationContext import org.jetbrains.kotlin.incremental.dumpCollection import java.io.File @@ -12,7 +13,12 @@ import java.io.File class ComplementarySourceFilesMap( storageFile: File, icContext: IncrementalCompilationContext, -) : BasicStringMap>(storageFile, PathStringDescriptor, StringCollectionExternalizer, icContext) { +) : AppendableBasicStringMap>( + storageFile, + PathStringDescriptor, + EnumeratorStringDescriptor.INSTANCE, + icContext +) { operator fun set(sourceFile: File, complementaryFiles: Collection) { storage[pathConverter.toPath(sourceFile)] = pathConverter.toPaths(complementaryFiles) @@ -20,7 +26,7 @@ class ComplementarySourceFilesMap( operator fun get(sourceFile: File): Collection { val paths = storage[pathConverter.toPath(sourceFile)].orEmpty() - return pathConverter.toFiles(paths) + return pathConverter.toFiles(paths).toSet() } override fun dumpValue(value: Collection) = diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/IdToFileMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/IdToFileMap.kt index 613cf017634..eb50dbedb02 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/IdToFileMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/IdToFileMap.kt @@ -24,20 +24,15 @@ import java.io.File internal class IdToFileMap( file: File, icContext: IncrementalCompilationContext, -) : NonAppendableBasicMap(file, ExternalIntegerKeyDescriptor(), EnumeratorStringDescriptor.INSTANCE, icContext) { +) : AbstractBasicMap(file, ExternalIntegerKeyDescriptor(), EnumeratorStringDescriptor.INSTANCE, icContext) { override fun dumpKey(key: Int): String = key.toString() override fun dumpValue(value: String): String = value - operator fun get(id: Int): File? = storage[id]?.let { pathConverter.toFile(it) } - - operator fun contains(id: Int): Boolean = id in storage + fun getFile(id: Int): File? = storage[id]?.let { pathConverter.toFile(it) } operator fun set(id: Int, file: File) { storage[id] = pathConverter.toPath(file) } - fun remove(id: Int) { - storage.remove(id) - } } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt new file mode 100644 index 00000000000..4762d3f1d3b --- /dev/null +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorage.kt @@ -0,0 +1,162 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.incremental.storage + +import org.jetbrains.kotlin.utils.ThreadSafe + +/** + * [PersistentStorage] which reads from another underlying [PersistentStorage], keeps all changes to it in memory, and writes the changes + * back to the underlying storage on [applyChanges] or [close]. + */ +interface InMemoryStorageInterface : PersistentStorage { + + /** Applies in-memory changes to the underlying [PersistentStorage], then calls [clearChanges]. */ + fun applyChanges() + + /** Removes all in-memory changes. */ + fun clearChanges() +} + +/** See [InMemoryStorageInterface]. */ +@ThreadSafe +open class InMemoryStorage( + private val storage: PersistentStorage, +) : InMemoryStorageInterface { + + override val storageFile = storage.storageFile + + // The following properties store changes to `storage`. + // Note that: + // - The keys across these groups must be mutually exclusive. + // - `addedEntries.keys` must be outside `storage.keys`. + // - `modifiedEntries.keys`, `appendedEntries.keys`, and `removedKeys` must be subsets of `storage.keys`. + // - `appendedEntries` is meant to be used only by the subclass `AppendableInMemoryStorage`, but it is in this class because we want + // to share code as much as possible. + protected val addedEntries = LinkedHashMap() + protected val modifiedEntries = LinkedHashMap() + protected val appendedEntries = LinkedHashMap() + protected val removedKeys = LinkedHashSet() + + @get:Synchronized + override val keys: Set + get() = storage.keys + addedEntries.keys - removedKeys + + @Synchronized + override fun contains(key: KEY): Boolean = when (key) { + in addedEntries -> true + in modifiedEntries -> true + in appendedEntries -> true + in removedKeys -> false + else -> key in storage + } + + @Synchronized + override fun get(key: KEY): VALUE? = addedEntries[key] ?: modifiedEntries[key] ?: when (key) { + in appendedEntries -> { + @Suppress("UNCHECKED_CAST") + ((storage[key]!! as Collection<*>) + (appendedEntries[key]!! as Collection<*>)) as VALUE + } + in removedKeys -> null + else -> storage[key] + } + + @Synchronized + override fun set(key: KEY, value: VALUE) = when (key) { + in addedEntries -> addedEntries[key] = value + in modifiedEntries -> modifiedEntries[key] = value + in appendedEntries -> { + appendedEntries.remove(key) + modifiedEntries[key] = value + } + in removedKeys -> { + removedKeys.remove(key) + modifiedEntries[key] = value + } + in storage -> modifiedEntries[key] = value + else -> addedEntries[key] = value + } + + @Synchronized + override fun remove(key: KEY) { + when (key) { + in addedEntries -> addedEntries.remove(key) + in modifiedEntries -> { + modifiedEntries.remove(key) + removedKeys.add(key) + } + in appendedEntries -> { + appendedEntries.remove(key) + removedKeys.add(key) + } + in removedKeys -> Unit + in storage -> removedKeys.add(key) + else -> Unit + } + } + + @Synchronized + override fun applyChanges() { + addedEntries.forEach { + storage[it.key] = it.value + } + modifiedEntries.forEach { + storage[it.key] = it.value + } + check(appendedEntries.isEmpty()) { "appendedEntries is not empty" } + removedKeys.forEach { + storage.remove(it) + } + clearChanges() + } + + @Synchronized + override fun clearChanges() { + addedEntries.clear() + modifiedEntries.clear() + appendedEntries.clear() + removedKeys.clear() + } + + @Synchronized + override fun close() { + applyChanges() + storage.close() + } + +} + +/** [InMemoryStorage] where a map entry's value is a [Collection]. */ +@ThreadSafe +class AppendableInMemoryStorage>( + private val storage: AppendablePersistentStorage, +) : InMemoryStorage(storage), AppendablePersistentStorage { + + @Suppress("UNCHECKED_CAST") + @Synchronized + override fun append(key: KEY, elements: VALUE) = when (key) { + in addedEntries -> addedEntries[key] = (addedEntries[key]!! + elements) as VALUE + in modifiedEntries -> modifiedEntries[key] = (modifiedEntries[key]!! + elements) as VALUE + in appendedEntries -> appendedEntries[key] = (appendedEntries[key]!! + elements) as VALUE + in removedKeys -> { + removedKeys.remove(key) + // Note: We update `modifiedEntries` (not `appendedEntries`), because if the entry is removed and then appended, it is + // equivalent to being modified (not appended) + modifiedEntries[key] = elements + } + in storage -> appendedEntries[key] = elements + else -> addedEntries[key] = elements + } + + @Synchronized + override fun applyChanges() { + appendedEntries.forEach { + storage.append(it.key, it.value) + } + appendedEntries.clear() + super.applyChanges() + } + +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt deleted file mode 100644 index 461cbb53981..00000000000 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt +++ /dev/null @@ -1,171 +0,0 @@ -/* - * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. - * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. - */ - -package org.jetbrains.kotlin.incremental.storage - -import com.intellij.util.io.DataExternalizer -import java.util.* -import kotlin.collections.LinkedHashMap - -interface InMemoryStorageWrapper : AppendableLazyStorage { - fun resetInMemoryChanges() -} - -/** - * An in-memory wrapper for [origin] that keeps all the write operations in-memory. - * Flushes all the changes to the [origin] on [flush] invocation. - * [resetInMemoryChanges] should be called to reset in-memory changes of this wrapper. - */ -class DefaultInMemoryStorageWrapper( - private val origin: CachingLazyStorage, - private val valueExternalizer: DataExternalizer -) : - InMemoryStorageWrapper { - // These state properties keep the current diff that will be applied to the [origin] on flush if [resetInMemoryChanges] is not called - private val inMemoryStorage = LinkedHashMap() - private val removedKeys = hashSetOf() - private var isCleanRequested = false - - @get:Synchronized - override val keys: Collection - get() = if (isCleanRequested) inMemoryStorage.keys else (origin.keys - removedKeys) + inMemoryStorage.keys - - @Synchronized - override fun resetInMemoryChanges() { - isCleanRequested = false - inMemoryStorage.clear() - removedKeys.clear() - } - - @Synchronized - override fun clean() { - inMemoryStorage.clear() - removedKeys.clear() - isCleanRequested = true - } - - @Synchronized - override fun flush(memoryCachesOnly: Boolean) { - if (isCleanRequested) { - origin.clean() - } else { - for (key in removedKeys) { - origin.remove(key) - } - } - for ((key, valueWrapper) in inMemoryStorage) { - when (valueWrapper) { - is ValueWrapper.Value<*> -> origin[key] = valueWrapper.value.cast() - // if we were appending the value and didn't access it, - // then we have it as an append chain, so merge it and append to the origin as a single value - is ValueWrapper.AppendChain<*> -> origin.append(key, getMergedValue(key, valueWrapper, false)).also { - origin[key] // trigger chunks compaction - } - } - } - - resetInMemoryChanges() - - origin.flush(memoryCachesOnly) - } - - @Synchronized - override fun close() { - origin.close() - } - - @Synchronized - override fun append(key: K, value: V) { - /* - * Plain English explanation: - * 1. The key's value is present only in origin => appendToOrigin = true - * 2. The key's value was set in this wrapper => appendToOrigin = false - * 3. The key's value was appended but not set in this wrapper => appendToOrigin = true - */ - check(valueExternalizer is AppendableDataExternalizer) { - "`valueExternalizer` should implement the `AppendableDataExternalizer` interface to be able to call `append`" - } - val currentWrapper = inMemoryStorage[key] - if (currentWrapper is ValueWrapper.AppendChain<*>) { - (currentWrapper.parts.cast>()).add(value) - return - } - - val newWrapper = when (currentWrapper) { - is ValueWrapper.Value<*> -> ValueWrapper.AppendChain(mutableListOf(currentWrapper.value.cast(), value), false) - // if `append` is called for the first time, assume it will be called more, so don't store it as `ValueWrapper.Value` - else -> ValueWrapper.AppendChain(mutableListOf(value), true) - } - - inMemoryStorage[key] = newWrapper - } - - @Synchronized - override fun remove(key: K) { - removedKeys.add(key) - inMemoryStorage.remove(key) - } - - @Synchronized - override fun set(key: K, value: V) { - inMemoryStorage[key] = ValueWrapper.Value(value) - } - - @Synchronized - override fun get(key: K): V? { - val wrapper = inMemoryStorage[key] - return when { - wrapper is ValueWrapper.Value<*> -> wrapper.value.cast() - wrapper is ValueWrapper.AppendChain<*> -> getMergedValue(key, wrapper).also { mergedValue -> - inMemoryStorage[key] = ValueWrapper.Value(mergedValue) - } - key !in removedKeys -> origin[key] - else -> null - } - } - - @Synchronized - override fun contains(key: K) = key in inMemoryStorage || (key !in removedKeys && key in origin) - - /** - * Merges a value for a [key] from [origin] if it isn't in [removedKeys] and [useOriginValue] != false with [ValueWrapper.AppendChain] and returns the merged value - */ - private fun getMergedValue(key: K, wrapper: ValueWrapper, useOriginValue: Boolean = true): V { - check(wrapper !is ValueWrapper.Value<*>) { - "There's no need to merge values for $key" - } - check(valueExternalizer is AppendableDataExternalizer) { - "`valueExternalizer` should implement the `AppendableDataExternalizer` interface to be able to handle `append`" - } - return when (wrapper) { - is ValueWrapper.AppendChain<*> -> { - fun merge(acc: V, append: V) = valueExternalizer.append(acc, append) - - val initial = if (useOriginValue && wrapper.appendToOrigin) { - listOfNotNull(getOriginValue(key)).fold(valueExternalizer.createNil(), ::merge) - } else { - valueExternalizer.createNil() - } - (wrapper.parts.cast>()).fold(initial, ::merge) - } - else -> error("In-memory storage contains no value for $key") - } - } - - private fun getOriginValue(key: K): V? = if (key !in removedKeys) { - origin[key] - } else { - null - } - - @Suppress("UNCHECKED_CAST") - private fun Any?.cast() = this as T - - private sealed interface ValueWrapper { - class Value(val value: V) : ValueWrapper - - class AppendChain(val parts: MutableList, val appendToOrigin: Boolean) : ValueWrapper - } -} \ No newline at end of file diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt index 8452ee406f2..08f557cc6c9 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt @@ -16,19 +16,162 @@ package org.jetbrains.kotlin.incremental.storage -import java.io.Closeable +import com.intellij.util.CommonProcessors +import com.intellij.util.io.AppendablePersistentMap +import com.intellij.util.io.DataExternalizer +import com.intellij.util.io.KeyDescriptor +import com.intellij.util.io.PersistentHashMap +import org.jetbrains.kotlin.incremental.IncrementalCompilationContext +import org.jetbrains.kotlin.utils.ThreadSafe +import java.io.DataInput +import java.io.DataInputStream +import java.io.DataOutput +import java.io.File + +/** + * [PersistentStorage] which delegates operations to a [PersistentHashMap]. Note that the [PersistentHashMap] is created lazily (only when + * required). + */ +@ThreadSafe +open class LazyStorage( + override val storageFile: File, + private val keyDescriptor: KeyDescriptor, + private val valueExternalizer: DataExternalizer, +) : PersistentStorage { + + private var storage: PersistentHashMap? = null + + // Use this property to minimize I/O + private val isStorageFileExist: Boolean by lazy { + storageFile.exists() + } + + private fun getStorageIfExists(): PersistentHashMap? { + return storage ?: when { + isStorageFileExist -> createMap().also { storage = it } + else -> null + } + } + + protected fun getStorageOrCreateNew(): PersistentHashMap { + return storage ?: createMap().also { storage = it } + } + + private fun createMap() = PersistentHashMap(storageFile, keyDescriptor, valueExternalizer) + + @get:Synchronized + override val keys: Set + get() = buildSet { + getStorageIfExists()?.processKeysWithExistingMapping(CommonProcessors.CollectProcessor(this)) + } + + @Synchronized + override fun contains(key: KEY): Boolean = + getStorageIfExists()?.containsMapping(key) ?: false + + @Synchronized + override fun get(key: KEY): VALUE? = + getStorageIfExists()?.get(key) + + @Synchronized + override fun set(key: KEY, value: VALUE) { + getStorageOrCreateNew().put(key, value) + } + + @Synchronized + override fun remove(key: KEY) { + getStorageIfExists()?.remove(key) + } + + @Synchronized + override fun close() { + storage?.close() + } -interface LazyStorage : Closeable { - val keys: Collection - operator fun contains(key: K): Boolean - operator fun get(key: K): V? - operator fun set(key: K, value: V) - fun remove(key: K) - fun clean() - fun flush(memoryCachesOnly: Boolean) - override fun close() } -interface AppendableLazyStorage : LazyStorage { - fun append(key: K, value: V) -} \ No newline at end of file +/** [LazyStorage] where a map entry's value is a [Collection]. */ +@ThreadSafe +class AppendableLazyStorage>( + storageFile: File, + keyDescriptor: KeyDescriptor, + elementExternalizer: DataExternalizer, +) : LazyStorage(storageFile, keyDescriptor, AppendableCollectionExternalizer(elementExternalizer)), + AppendablePersistentStorage { + + private val appendableCollectionExternalizer = AppendableCollectionExternalizer(elementExternalizer) + + @Synchronized + override fun append(key: KEY, elements: VALUE) { + getStorageOrCreateNew().appendData( + key, + AppendablePersistentMap.ValueDataAppender { appendableCollectionExternalizer.append(it, elements) } + ) + } +} + +/** + * [DataExternalizer] for a [Collection] of type [E]. + * + * IMPORTANT: It is a *private* class because it is meant to be used only with a [PersistentHashMap] (e.g., the [read] method reads until + * the stream ends, [append] can be called multiple times and its implementation is identical to [save] -- these only work with a + * [PersistentHashMap]). + */ +private class AppendableCollectionExternalizer>( + private val elementExternalizer: DataExternalizer, +) : DataExternalizer { + + fun append(output: DataOutput, elements: VALUE) { + save(output, elements) + } + + override fun save(output: DataOutput, value: VALUE) { + value.forEach { elementExternalizer.save(output, it) } + } + + override fun read(input: DataInput): VALUE { + val result = ArrayList() + val stream = input as DataInputStream + + while (stream.available() > 0) { + result.add(elementExternalizer.read(stream)) + } + + @Suppress("UNCHECKED_CAST") + return result as VALUE + } +} + +fun createPersistentStorage( + storageFile: File, + keyDescriptor: KeyDescriptor, + valueExternalizer: DataExternalizer, + icContext: IncrementalCompilationContext, +): PersistentStorage { + return LazyStorage(storageFile, keyDescriptor, valueExternalizer).let { storage -> + if (icContext.keepIncrementalCompilationCachesInMemory) { + InMemoryStorage(storage).also { + icContext.transaction.registerInMemoryStorageWrapper(it) + } + } else { + storage + } + } +} + +fun > createAppendablePersistentStorage( + storageFile: File, + keyDescriptor: KeyDescriptor, + elementExternalizer: DataExternalizer, + icContext: IncrementalCompilationContext, +): AppendablePersistentStorage { + return AppendableLazyStorage(storageFile, keyDescriptor, elementExternalizer).let { storage -> + if (icContext.keepIncrementalCompilationCachesInMemory) { + AppendableInMemoryStorage(storage).also { + icContext.transaction.registerInMemoryStorageWrapper(it) + } + } else { + storage + } + } +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/LookupMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/LookupMap.kt index 1a5472844a2..0d11976e7fb 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/LookupMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/LookupMap.kt @@ -23,10 +23,10 @@ class LookupMap( storage: File, icContext: IncrementalCompilationContext, ) : - AppendableBasicMap>( + AppendableAbstractBasicMap>( storage, LookupSymbolKeyDescriptor(icContext.storeFullFqNamesInLookupCache), - IntCollectionExternalizer, + IntExternalizer, icContext, ) { @@ -35,23 +35,13 @@ class LookupMap( override fun dumpValue(value: Collection): String = value.toString() fun add(name: String, scope: String, fileId: Int) { - storage.append(LookupSymbolKey(name, scope), listOf(fileId)) + storage.append(LookupSymbolKey(name, scope), fileId) } - fun append(lookup: LookupSymbolKey, fileIds: Collection) { - storage.append(lookup, fileIds) - } - - operator fun get(key: LookupSymbolKey): Collection? = storage[key] + override fun get(key: LookupSymbolKey): Collection? = storage[key]?.toSet() operator fun set(key: LookupSymbolKey, fileIds: Set) { storage[key] = fileIds } - fun remove(key: LookupSymbolKey) { - storage.remove(key) - } - - val keys: Collection - get() = storage.keys } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt new file mode 100644 index 00000000000..67e1f0fd25f --- /dev/null +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorage.kt @@ -0,0 +1,116 @@ +/* + * Copyright 2010-2015 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.incremental.storage + +import org.jetbrains.kotlin.utils.ThreadSafe +import java.io.Closeable +import java.io.File + +/** + * Represents an in-memory map that is backed by a [storageFile]. + * + * Changes to this map may be written to [storageFile] at any time, and it is guaranteed to be written on [close]. + * + * This interface is similar to but simpler than [com.intellij.util.io.PersistentMapBase]. + */ +@Suppress("UnstableApiUsage") +interface PersistentStorage : Closeable { + + /** The storage file backing this map. */ + val storageFile: File + + val keys: Set + + operator fun contains(key: KEY): Boolean + + operator fun get(key: KEY): VALUE? + + operator fun set(key: KEY, value: VALUE) + + fun remove(key: KEY) + + /** Writes any remaining in-memory changes to [storageFile] and closes this map. */ + override fun close() +} + +/** [PersistentStorage] where a map entry's value is a [Collection]. */ +interface AppendablePersistentStorage> : PersistentStorage { + + /** Adds the given [elements] to the collection corresponding to the given [key]. */ + fun append(key: KEY, elements: VALUE) + + /** Adds the given [element] to the collection corresponding to the given [key]. */ + fun append(key: KEY, element: E) { + @Suppress("UNCHECKED_CAST") + append(key, listOf(element) as VALUE) + } +} + +/** + * [PersistentStorage] that delegates operations to another [storage]. + * + * THREAD SAFETY: All [PersistentStorage]s (both parent classes and their subclasses) need to be thread-safe. This requirement seems to come + * from JPS -- see commit 275a02c; Gradle builds don't have this requirement. To ensure thread safety, currently all non-private + * implementation methods of [PersistentStorage]s and their subclasses must be `@Synchronized`. A possibly better approach is to perform + * synchronization in JPS, so that we don't have to provide the thread safety guarantee for [PersistentStorage]s. + */ +@ThreadSafe +abstract class PersistentStorageWrapper( + private val storage: PersistentStorage, +) : PersistentStorage { // Can't use Kotlin delegation (`by storage`) here as we need to annotate the methods with @Synchronized + + override val storageFile: File = storage.storageFile + + @get:Synchronized + override val keys: Set + get() = storage.keys + + @Synchronized + override fun contains(key: KEY): Boolean = + storage.contains(key) + + @Synchronized + override fun get(key: KEY): VALUE? = + storage[key] + + @Synchronized + override fun set(key: KEY, value: VALUE) { + storage[key] = value + } + + @Synchronized + override fun remove(key: KEY) { + storage.remove(key) + } + + @Synchronized + override fun close() { + storage.close() + } +} + +/** [PersistentStorageWrapper] where a map entry's value is a [Collection]. */ +@ThreadSafe +abstract class AppendablePersistentStorageWrapper>( + private val appendableStorage: AppendablePersistentStorage, +) : PersistentStorageWrapper(appendableStorage), AppendablePersistentStorage { + + @Synchronized + override fun append(key: KEY, elements: VALUE) { + appendableStorage.append(key, elements) + } +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorageAdapter.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorageAdapter.kt new file mode 100644 index 00000000000..4bfddfe880b --- /dev/null +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/PersistentStorageAdapter.kt @@ -0,0 +1,76 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.incremental.storage + +import org.jetbrains.kotlin.utils.ThreadSafe +import java.io.File + +/** + * [PersistentStorage] which maps [KEY] to [VALUE] as viewed by the users of this class, but delegates operations to another [storage] which + * maps [INTERNAL_KEY] to [INTERNAL_VALUE]. + * + * The users of this class need to provide the transformations from [KEY] to [INTERNAL_KEY], [VALUE] to [INTERNAL_VALUE], and vice versa. + */ +@ThreadSafe +abstract class PersistentStorageAdapter( + private val storage: PersistentStorage, + private val publicToInternalKey: (KEY) -> INTERNAL_KEY, + private val internalToPublicKey: (INTERNAL_KEY) -> KEY, + private val publicToInternalValue: (VALUE) -> INTERNAL_VALUE, + private val internalToPublicValue: (INTERNAL_VALUE) -> VALUE, +) : PersistentStorage, BasicMap { + + override val storageFile: File = storage.storageFile + + @get:Synchronized + override val keys: Set + get() = storage.keys.mapTo(LinkedHashSet()) { internalToPublicKey(it) } + + @Synchronized + override fun contains(key: KEY): Boolean = + storage.contains(publicToInternalKey(key)) + + @Synchronized + override fun get(key: KEY): VALUE? = + storage[publicToInternalKey(key)]?.let { internalToPublicValue(it) } + + @Synchronized + override fun set(key: KEY, value: VALUE) { + storage[publicToInternalKey(key)] = publicToInternalValue(value) + } + + @Synchronized + override fun remove(key: KEY) { + storage.remove(publicToInternalKey(key)) + } + + @Synchronized + override fun close() { + storage.close() + } +} + +/** [PersistentStorageAdapter] where a map entry's value is a [Collection]. */ +@ThreadSafe +abstract class AppendablePersistentStorageAdapter, INTERNAL_KEY, INTERNAL_E, INTERNAL_VALUE : Collection>( + private val storage: AppendablePersistentStorage, + private val publicToInternalKey: (KEY) -> INTERNAL_KEY, + internalToPublicKey: (INTERNAL_KEY) -> KEY, + private val publicToInternalValue: (VALUE) -> INTERNAL_VALUE, + internalToPublicValue: (INTERNAL_VALUE) -> VALUE, +) : PersistentStorageAdapter( + storage, + publicToInternalKey, + internalToPublicKey, + publicToInternalValue, + internalToPublicValue +), AppendablePersistentStorage { + + @Synchronized + override fun append(key: KEY, elements: VALUE) { + storage.append(publicToInternalKey(key), publicToInternalValue(elements)) + } +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToJsOutputMap.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToJsOutputMap.kt index 4369541b4d7..167231a265a 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToJsOutputMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToJsOutputMap.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.incremental.storage +import com.intellij.util.io.EnumeratorStringDescriptor import org.jetbrains.kotlin.incremental.IncrementalCompilationContext import org.jetbrains.kotlin.incremental.dumpCollection import java.io.File @@ -12,16 +13,16 @@ import java.io.File class SourceToJsOutputMap( storageFile: File, icContext: IncrementalCompilationContext, -) : AppendableBasicStringMap>(storageFile, StringCollectionExternalizer, icContext) { +) : AppendableBasicStringMap>(storageFile, EnumeratorStringDescriptor.INSTANCE, icContext) { override fun dumpValue(value: Collection): String = value.dumpCollection() @Synchronized fun add(key: File, value: File) { - storage.append(pathConverter.toPath(key), listOf(pathConverter.toPath(value))) + storage.append(pathConverter.toPath(key), pathConverter.toPath(value)) } operator fun get(sourceFile: File): Collection = - storage[pathConverter.toPath(sourceFile)]?.map { pathConverter.toFile(it) } ?: setOf() + storage[pathConverter.toPath(sourceFile)]?.mapTo(mutableSetOf()) { pathConverter.toFile(it) } ?: setOf() @Synchronized @@ -31,7 +32,7 @@ class SourceToJsOutputMap( return } - storage[pathConverter.toPath(key)] = values.map { pathConverter.toPath(it) } + storage[pathConverter.toPath(key)] = values.toSet().map { pathConverter.toPath(it) } } @Synchronized diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToOutputMaps.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToOutputMaps.kt index 24e3cda6ef1..a460a1cbc8e 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToOutputMaps.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToOutputMaps.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.incremental.storage +import com.intellij.util.io.EnumeratorStringDescriptor import org.jetbrains.kotlin.incremental.IncrementalCompilationContext import org.jetbrains.kotlin.incremental.dumpCollection import org.jetbrains.kotlin.name.FqName @@ -36,28 +37,30 @@ internal abstract class AbstractSourceToOutputMap( private val nameTransformer: NameTransformer, storageFile: File, icContext: IncrementalCompilationContext, -) : AppendableBasicStringMap>(storageFile, PathStringDescriptor, StringCollectionExternalizer, icContext) { +) : AppendableBasicStringMap>( + storageFile, + PathStringDescriptor, + EnumeratorStringDescriptor.INSTANCE, + icContext +) { fun clearOutputsForSource(sourceFile: File) { remove(pathConverter.toPath(sourceFile)) } fun add(sourceFile: File, className: Name) { - storage.append(pathConverter.toPath(sourceFile), listOf(nameTransformer.asString(className))) + storage.append(pathConverter.toPath(sourceFile), nameTransformer.asString(className)) } fun contains(sourceFile: File): Boolean = pathConverter.toPath(sourceFile) in storage operator fun get(sourceFile: File): Collection = - storage[pathConverter.toPath(sourceFile)].orEmpty().map(nameTransformer::asName) + storage[pathConverter.toPath(sourceFile)].orEmpty().toSet().map(nameTransformer::asName) fun getFqNames(sourceFile: File): Collection = - storage[pathConverter.toPath(sourceFile)].orEmpty().map(nameTransformer::asFqName) + storage[pathConverter.toPath(sourceFile)].orEmpty().toSet().map(nameTransformer::asFqName) override fun dumpValue(value: Collection) = value.dumpCollection() - private fun remove(path: String) { - storage.remove(path) - } } \ No newline at end of file diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt index 330b4a400fd..5e8905a11f0 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt @@ -31,23 +31,6 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.jvm.JvmClassName import java.io.* -/** - * Externalizer that works correctly when [com.intellij.util.io.PersistentHashMap.appendData] is called - * - * Besides the [append] method, it should support incremental [save] and [read]. E.g. if [save] was called multiple times, [read] should be able to collect them together - */ -interface AppendableDataExternalizer : DataExternalizer { - /** - * Creates an empty appendable object - */ - fun createNil(): T - - /** - * Combines two non-serialized appendable objects - */ - fun append(currentValue: T, appendData: T): T -} - class LookupSymbolKeyDescriptor( /** If `true`, original values are saved; if `false`, only hashes are saved. */ private val storeFullFqNames: Boolean = false @@ -278,51 +261,6 @@ class DelegateDataExternalizer( } } -/** - * [DataExternalizer] for a [Collection]. - * - * If you need a [DataExternalizer] for a more specific instance of [Collection] (e.g., [List]), use [ListExternalizer] or create another - * instance of [GenericCollectionExternalizer]. - * - * Note: The implementations of this class and [GenericCollectionExternalizer] are similar but not exactly the same: the latter reads and - * writes the size of the collection to avoid resizing the collection when reading. Therefore, if we make this class extend - * [GenericCollectionExternalizer] to share code, we will need to update some expected files in tests as the serialized data will change - * slightly. - */ -open class CollectionExternalizer( - private val elementExternalizer: DataExternalizer, - private val newCollection: () -> MutableCollection -) : AppendableDataExternalizer> { - override fun read(input: DataInput): Collection { - val result = newCollection() - val stream = input as DataInputStream - - while (stream.available() > 0) { - result.add(elementExternalizer.read(stream)) - } - - return result - } - - override fun save(output: DataOutput, value: Collection) { - value.forEach { elementExternalizer.save(output, it) } - } - - override fun createNil() = newCollection() - - override fun append(currentValue: Collection, appendData: Collection) = when (currentValue) { - is MutableCollection<*> -> { - (currentValue as MutableCollection).addAll(appendData) - currentValue - } - else -> currentValue + appendData - } -} - -object StringCollectionExternalizer : CollectionExternalizer(EnumeratorStringDescriptor(), { HashSet() }) - -object IntCollectionExternalizer : CollectionExternalizer(IntExternalizer, { HashSet() }) - fun DataOutput.writeString(value: String) = StringExternalizer.save(this, value) fun DataInput.readString(): String = StringExternalizer.read(this) @@ -358,9 +296,56 @@ object ByteArrayExternalizer : DataExternalizer { } } -abstract class GenericCollectionExternalizer>( +/** + * DEPRECATED: [DataExternalizer] for a [Collection], whose implementation is tied to [com.intellij.util.io.PersistentHashMap] (e.g., the + * [read] method reads until the stream ends -- this can only work with a [com.intellij.util.io.PersistentHashMap]). + * + * Use [CollectionExternalizerV2] if possible. + */ +private class CollectionExternalizerForPersistentHashMap( private val elementExternalizer: DataExternalizer, - private val newCollection: (size: Int) -> MutableCollection + private val newCollection: () -> MutableCollection, +) : DataExternalizer> { + + override fun save(output: DataOutput, value: Collection) { + value.forEach { elementExternalizer.save(output, it) } + } + + override fun read(input: DataInput): Collection { + val result = newCollection() + val stream = input as DataInputStream + + while (stream.available() > 0) { + result.add(elementExternalizer.read(stream)) + } + + return result + } +} + +/** + * DEPRECATED: This class should not be used because its implementation is tied to [com.intellij.util.io.PersistentHashMap] + * (see [CollectionExternalizerForPersistentHashMap]). + * + * Currently, we can't change the name or implementation of this class because it is still used by the `compiler-reference-index` module in + * the Kotlin IDEA plugin and that code relies on this name and implementation being unchanged (see KT-62288). + * + * Once we remove that dependency, we can remove this class. + */ +class CollectionExternalizer( + private val elementExternalizer: DataExternalizer, + private val newCollection: () -> MutableCollection, +) : DataExternalizer> by CollectionExternalizerForPersistentHashMap(elementExternalizer, newCollection) + +/** DEPRECATED: See [CollectionExternalizer]. */ +@Suppress("unused") // See `CollectionExternalizer` +object IntCollectionExternalizer : + DataExternalizer> by CollectionExternalizerForPersistentHashMap(IntExternalizer, { ArrayList() }) + +/** [DataExternalizer] for a [Collection]. */ +open class CollectionExternalizerV2>( + private val elementExternalizer: DataExternalizer, + private val newCollection: (size: Int) -> MutableCollection, ) : DataExternalizer { override fun save(output: DataOutput, collection: C) { @@ -384,11 +369,13 @@ abstract class GenericCollectionExternalizer>( } } +object StringCollectionExternalizer : CollectionExternalizerV2>(StringExternalizer, { size -> ArrayList(size) }) + class ListExternalizer(elementExternalizer: DataExternalizer) : - GenericCollectionExternalizer>(elementExternalizer, { size -> ArrayList(size) }) + CollectionExternalizerV2>(elementExternalizer, { size -> ArrayList(size) }) class SetExternalizer(elementExternalizer: DataExternalizer) : - GenericCollectionExternalizer>(elementExternalizer, { size -> LinkedHashSet(size) }) + CollectionExternalizerV2>(elementExternalizer, { size -> LinkedHashSet(size) }) open class MapExternalizer>( private val keyExternalizer: DataExternalizer, diff --git a/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt b/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt index 8c47ad082e3..e530f81bb88 100644 --- a/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt +++ b/build-common/test/org/jetbrains/kotlin/incremental/CompilationTransactionTest.kt @@ -6,14 +6,13 @@ package org.jetbrains.kotlin.incremental import org.jetbrains.kotlin.build.report.DoNothingBuildReporter -import org.jetbrains.kotlin.incremental.storage.InMemoryStorageWrapper -import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertTrue +import org.jetbrains.kotlin.incremental.storage.InMemoryStorageInterface +import org.junit.jupiter.api.Assertions.* import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows import org.junit.jupiter.api.io.TempDir import java.io.Closeable +import java.io.File import java.nio.file.Files import java.nio.file.Path @@ -27,23 +26,21 @@ private class CacheMock(private val throwsException: Boolean = false) : Closeabl } } -private class InMemoryStorageWrapperMock : InMemoryStorageWrapper { +private class InMemoryStorageWrapperMock : InMemoryStorageInterface { var reset = false - override fun resetInMemoryChanges() { + override fun applyChanges() {} + + override fun clearChanges() { reset = true } - override val keys: Collection = emptyList() + override val storageFile = File("") - override fun clean() {} - - override fun flush(memoryCachesOnly: Boolean) {} + override val keys: Set = emptySet() override fun close() {} - override fun append(key: Any, value: Any) {} - override fun remove(key: Any) {} override fun set(key: Any, value: Any) {} diff --git a/build-common/test/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapperTest.kt b/build-common/test/org/jetbrains/kotlin/incremental/storage/InMemoryStorageTest.kt similarity index 97% rename from build-common/test/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapperTest.kt rename to build-common/test/org/jetbrains/kotlin/incremental/storage/InMemoryStorageTest.kt index a154ff83a2c..b87c577b90d 100644 --- a/build-common/test/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapperTest.kt +++ b/build-common/test/org/jetbrains/kotlin/incremental/storage/InMemoryStorageTest.kt @@ -15,7 +15,7 @@ import java.io.Closeable import java.nio.file.Files import java.nio.file.Path -class InMemoryStorageWrapperTest { +class InMemoryStorageTest { @TempDir private lateinit var workingDir: Path @@ -83,7 +83,7 @@ class InMemoryStorageWrapperTest { assertEquals(setOf(5), it[key5]) } withLookupMapInTransaction(storageRoot, useInMemoryWrapper = true, successful = true) { - it.clean() + it.clear() it.append(key1, setOf(4)) } withLookupMapInTransaction(storageRoot, useInMemoryWrapper = false, successful = true) { @@ -108,7 +108,7 @@ class InMemoryStorageWrapperTest { val savedState = workingDir.resolve("backup") storageRoot.toFile().copyRecursively(savedState.toFile()) withLookupMapInTransaction(storageRoot, useInMemoryWrapper = true, successful = false) { - it.clean() + it.clear() it.append(key1, setOf(3)) it.remove(key2) it[key3] = setOf(5) @@ -131,7 +131,6 @@ class InMemoryStorageWrapperTest { icContext.transaction.runWithin { transaction -> val lookupMap = LookupMap(storageFile, icContext) transaction.cachesManager = Closeable { - lookupMap.flush(false) lookupMap.close() } dataProvider(lookupMap) 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 0f2fb9f27d2..5d4de8ed58f 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 @@ -60,8 +60,6 @@ abstract class IncrementalCachesManager>(storageFile, PathStringDescriptor, StringCollectionExternalizer, icContext) { operator fun set(sourceFile: File, outputFiles: Collection) { - storage[icContext.pathConverterForSourceFiles.toPath(sourceFile)] = outputFiles.map(icContext.pathConverterForOutputFiles::toPath) + storage[icContext.pathConverterForSourceFiles.toPath(sourceFile)] = + outputFiles.toSet().map(icContext.pathConverterForOutputFiles::toPath) } operator fun get(sourceFile: File): Collection? = diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/snapshots/FileSnapshotMapTest.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/snapshots/FileSnapshotMapTest.kt index 2c2e0aabc74..36d17b89b4c 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/snapshots/FileSnapshotMapTest.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/snapshots/FileSnapshotMapTest.kt @@ -32,8 +32,7 @@ class FileSnapshotMapTest : TestWithWorkingDir() { @After override fun tearDown() { - snapshotMap.flush(false) - snapshotMap.closeForTest() + snapshotMap.close() super.tearDown() } diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/storage/SourceToOutputFilesMapTest.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/storage/SourceToOutputFilesMapTest.kt index dee416ad8b8..e9001c15d8d 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/storage/SourceToOutputFilesMapTest.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/storage/SourceToOutputFilesMapTest.kt @@ -50,8 +50,7 @@ class SourceToOutputFilesMapTest { @After fun tearDown() { - stofMap.flush(false) - stofMap.closeForTest() + stofMap.close() } @Test