From 99830d7f286ea7e202ce7a7ec54a928209b491dd Mon Sep 17 00:00:00 2001 From: "Alexander.Likhachev" Date: Tue, 21 Feb 2023 17:13:11 +0100 Subject: [PATCH] [IC] Segregate `LazyStorage` into `LazyStorage` and `AppendableLazyStorage` #KT-56052 In Progress --- .../kotlin/incremental/storage/BasicMap.kt | 54 ++++++++++---- .../incremental/storage/BasicMapsOwner.kt | 10 +-- .../incremental/storage/CachingLazyStorage.kt | 35 ++++++++- .../incremental/storage/ClassOneToManyMap.kt | 2 +- .../kotlin/incremental/storage/IdToFileMap.kt | 2 +- .../storage/InMemoryStorageWrapper.kt | 73 ++++++++++--------- .../kotlin/incremental/storage/LazyStorage.kt | 5 +- .../kotlin/incremental/storage/LookupMap.kt | 2 +- .../storage/SourceToJsOutputMap.kt | 2 +- .../incremental/storage/SourceToOutputMaps.kt | 2 +- .../incremental/storage/externalizers.kt | 16 +++- 11 files changed, 142 insertions(+), 61 deletions(-) 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 3c6ff37abcc..8a951f108eb 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/BasicMap.kt @@ -24,22 +24,11 @@ import org.jetbrains.kotlin.incremental.IncrementalCompilationContext import org.jetbrains.kotlin.utils.Printer import java.io.File -abstract class BasicMap, V>( +abstract class BasicMap, V, StorageType : LazyStorage>( internal val storageFile: File, - keyDescriptor: KeyDescriptor, - valueExternalizer: DataExternalizer, + protected val storage: StorageType, protected val icContext: IncrementalCompilationContext, ) { - protected val storage: LazyStorage = CachingLazyStorage(storageFile, keyDescriptor, valueExternalizer).let { - if (icContext.keepIncrementalCompilationCachesInMemory) { - DefaultInMemoryStorageWrapper(it).also { wrapper -> - icContext.transaction.registerInMemoryStorageWrapper(wrapper) - } - } else { - it - } - } - protected val pathConverter get() = icContext.pathConverter @@ -86,12 +75,34 @@ abstract class BasicMap, V>( protected abstract fun dumpValue(value: V): String } +abstract class NonAppendableBasicMap, V>( + storageFile: File, + keyDescriptor: KeyDescriptor, + valueExternalizer: DataExternalizer, + icContext: IncrementalCompilationContext, +) : BasicMap>( + storageFile, + createLazyStorage(storageFile, keyDescriptor, valueExternalizer, icContext), + icContext +) + +abstract class AppendableBasicMap, V>( + storageFile: File, + keyDescriptor: KeyDescriptor, + valueExternalizer: AppendableDataExternalizer, + icContext: IncrementalCompilationContext, +) : BasicMap>( + storageFile, + createLazyStorage(storageFile, keyDescriptor, valueExternalizer, icContext), + icContext +) + abstract class BasicStringMap( storageFile: File, keyDescriptor: KeyDescriptor, valueExternalizer: DataExternalizer, icContext: IncrementalCompilationContext, -) : BasicMap(storageFile, keyDescriptor, valueExternalizer, icContext) { +) : NonAppendableBasicMap(storageFile, keyDescriptor, valueExternalizer, icContext) { constructor( storageFile: File, valueExternalizer: DataExternalizer, @@ -100,3 +111,18 @@ abstract class BasicStringMap( override fun dumpKey(key: String): String = key } + +abstract class AppendableBasicStringMap( + storageFile: File, + keyDescriptor: KeyDescriptor, + valueExternalizer: AppendableDataExternalizer, + icContext: IncrementalCompilationContext, +) : AppendableBasicMap(storageFile, keyDescriptor, valueExternalizer, icContext) { + constructor( + storageFile: File, + valueExternalizer: AppendableDataExternalizer, + icContext: IncrementalCompilationContext, + ) : this(storageFile, EnumeratorStringDescriptor.INSTANCE, valueExternalizer, icContext) + + override fun dumpKey(key: String): String = key +} \ No newline at end of file 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 57d90607c4f..25ec537fd02 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,17 +31,17 @@ 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) + forEachMapSafe("clean", BasicMap<*, *, *>::clean) } open fun close() { - forEachMapSafe("close", BasicMap<*, *>::close) + forEachMapSafe("close", BasicMap<*, *, *>::close) } open fun flush(memoryCachesOnly: Boolean) { @@ -49,7 +49,7 @@ open class BasicMapsOwner(val cachesDir: File) { } @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 index 1f11945a828..93188306eb6 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/CachingLazyStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/CachingLazyStorage.kt @@ -20,6 +20,7 @@ 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 @@ -31,7 +32,7 @@ class CachingLazyStorage( private val storageFile: File, private val keyDescriptor: KeyDescriptor, private val valueExternalizer: DataExternalizer -) : LazyStorage { +) : AppendableLazyStorage { private var storage: PersistentHashMap? = null private var isStorageFileExist = true @@ -80,6 +81,9 @@ class CachingLazyStorage( @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, { valueExternalizer.save(it, value) }) } @@ -119,3 +123,32 @@ class CachingLazyStorage( 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 5f6789276a3..597b982588f 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/ClassOneToManyMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/ClassOneToManyMap.kt @@ -24,7 +24,7 @@ import java.io.File internal open class ClassOneToManyMap( storageFile: File, icContext: IncrementalCompilationContext, -) : BasicStringMap>(storageFile, StringCollectionExternalizer, icContext) { +) : AppendableBasicStringMap>(storageFile, StringCollectionExternalizer, icContext) { override fun dumpValue(value: Collection): String = value.dumpCollection() @Synchronized 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 4e7a8799a66..613cf017634 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/IdToFileMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/IdToFileMap.kt @@ -24,7 +24,7 @@ import java.io.File internal class IdToFileMap( file: File, icContext: IncrementalCompilationContext, -) : BasicMap(file, ExternalIntegerKeyDescriptor(), EnumeratorStringDescriptor.INSTANCE, icContext) { +) : NonAppendableBasicMap(file, ExternalIntegerKeyDescriptor(), EnumeratorStringDescriptor.INSTANCE, icContext) { override fun dumpKey(key: Int): String = key.toString() override fun dumpValue(value: String): String = value diff --git a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt index 8d1b6931320..968409b4112 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/InMemoryStorageWrapper.kt @@ -5,10 +5,11 @@ package org.jetbrains.kotlin.incremental.storage +import com.intellij.util.io.DataExternalizer import java.util.* import kotlin.collections.LinkedHashMap -interface InMemoryStorageWrapper : LazyStorage { +interface InMemoryStorageWrapper : AppendableLazyStorage { fun resetInMemoryChanges() } @@ -17,7 +18,12 @@ interface InMemoryStorageWrapper : LazyStorage { * 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: LazyStorage) : InMemoryStorageWrapper { +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 @@ -65,25 +71,21 @@ class DefaultInMemoryStorageWrapper(private val origin: LazyStorage) } override fun append(key: K, value: V) { - @Suppress("UNCHECKED_CAST") - when (value) { - is Collection<*> -> { - when { - key in inMemoryStorage -> { - // the key's value was set in-memory, so we keep changing it without additional marking as append - (inMemoryStorage.getValue(key).value as MutableCollection).addAll(value) - } - key !in removedKeys && key in origin -> { - val collection = copyCollection(origin[key] as Collection<*>) - collection.addAll(value) - inMemoryStorage[key] = ValueWrapper(collection as V, isAppend = true) - } - else -> { - set(key, value) - } - } + check(valueExternalizer is AppendableDataExternalizer) { + "`valueExternalizer` should implement the `AppendableDataExternalizer` interface to be able to call `append`" + } + when { + key in inMemoryStorage -> { + val currentEntry = inMemoryStorage.getValue(key) + // should we avoid new allocations by performing appends in-place? + inMemoryStorage[key] = ValueWrapper(valueExternalizer.append(currentEntry.value, value), isAppend = currentEntry.isAppend) + } + key !in removedKeys && key in origin -> { + inMemoryStorage[key] = ValueWrapper(value, isAppend = true) + } + else -> { + set(key, value) } - else -> error("The value is expected to implement the Collection interface") } } @@ -93,23 +95,26 @@ class DefaultInMemoryStorageWrapper(private val origin: LazyStorage) } override fun set(key: K, value: V) { - @Suppress("UNCHECKED_CAST") - inMemoryStorage[key] = ValueWrapper(if (value is Collection<*>) copyCollection(value) as V else value) + inMemoryStorage[key] = ValueWrapper(value) } - private fun copyCollection(collection: Collection<*>): MutableCollection { - val newCollection = when (collection) { - is Set<*> -> TreeSet() - else -> ArrayList(collection.size) + override fun get(key: K): V? { + return when (key) { + in inMemoryStorage -> { + val entry = inMemoryStorage.getValue(key) + val originValue = origin[key] + if (entry.isAppend && originValue != null) { + check(valueExternalizer is AppendableDataExternalizer) { + "`valueExternalizer` should implement the `AppendableDataExternalizer` interface to be able to handle `append`" + } + valueExternalizer.append(originValue, entry.value) + } else { + entry.value + } + } + !in removedKeys -> origin[key] + else -> null } - newCollection.addAll(collection) - return newCollection - } - - override fun get(key: K): V? = when (key) { - in inMemoryStorage -> inMemoryStorage.getValue(key).value - !in removedKeys -> origin[key] - else -> null } override fun contains(key: K) = key in inMemoryStorage || (key !in removedKeys && key in origin) 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 831ed500882..32dc81fa1c7 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/LazyStorage.kt @@ -22,8 +22,11 @@ interface LazyStorage { operator fun get(key: K): V? operator fun set(key: K, value: V) fun remove(key: K) - fun append(key: K, value: V) fun clean() fun flush(memoryCachesOnly: Boolean) fun close() +} + +interface AppendableLazyStorage : LazyStorage { + fun append(key: K, value: V) } \ No newline at end of file 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 8abf9069666..1a5472844a2 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/LookupMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/LookupMap.kt @@ -23,7 +23,7 @@ class LookupMap( storage: File, icContext: IncrementalCompilationContext, ) : - BasicMap>( + AppendableBasicMap>( storage, LookupSymbolKeyDescriptor(icContext.storeFullFqNamesInLookupCache), IntCollectionExternalizer, 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 31206097a21..4369541b4d7 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToJsOutputMap.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToJsOutputMap.kt @@ -12,7 +12,7 @@ import java.io.File class SourceToJsOutputMap( storageFile: File, icContext: IncrementalCompilationContext, -) : BasicStringMap>(storageFile, StringCollectionExternalizer, icContext) { +) : AppendableBasicStringMap>(storageFile, StringCollectionExternalizer, icContext) { override fun dumpValue(value: Collection): String = value.dumpCollection() @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 90a9d20a833..24e3cda6ef1 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToOutputMaps.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/SourceToOutputMaps.kt @@ -36,7 +36,7 @@ internal abstract class AbstractSourceToOutputMap( private val nameTransformer: NameTransformer, storageFile: File, icContext: IncrementalCompilationContext, -) : BasicStringMap>(storageFile, PathStringDescriptor, StringCollectionExternalizer, icContext) { +) : AppendableBasicStringMap>(storageFile, PathStringDescriptor, StringCollectionExternalizer, icContext) { fun clearOutputsForSource(sourceFile: File) { remove(pathConverter.toPath(sourceFile)) } 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 a5d46ff8e87..3f3fbd681cd 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/storage/externalizers.kt @@ -31,6 +31,18 @@ import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.jvm.JvmClassName import java.io.* +/** + * Externalizer that works correctly when [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 { + /** + * Combines two non-serialized values + */ + 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 @@ -276,7 +288,7 @@ class DelegateDataExternalizer( open class CollectionExternalizer( private val elementExternalizer: DataExternalizer, private val newCollection: () -> MutableCollection -) : DataExternalizer> { +) : AppendableDataExternalizer> { override fun read(input: DataInput): Collection { val result = newCollection() val stream = input as DataInputStream @@ -291,6 +303,8 @@ open class CollectionExternalizer( override fun save(output: DataOutput, value: Collection) { value.forEach { elementExternalizer.save(output, it) } } + + override fun append(currentValue: Collection, appendData: Collection) = currentValue + appendData } object StringCollectionExternalizer : CollectionExternalizer(EnumeratorStringDescriptor(), { HashSet() })