[IC] Add back flush() API to PersistentStorage

In commit 4e89dcf, we removed `flush()` from `PersistentStorage`
(previously called `LazyStorage`) because it is not used in Gradle
builds.

However, `flush()` is still used in JPS builds, so we need to add that
API back in this commit.

#KT-62486 Fixed

Co-authored-by: Hung Nguyen <hungnv@google.com>


Merge-request: KOTLIN-MR-796
Merged-by: Aleksei Cherepanov <aleksei.cherepanov@jetbrains.com>
This commit is contained in:
Hung Nguyen
2023-10-16 18:30:47 +00:00
committed by Space Cloud
parent 498f2e534a
commit ecbf69504a
9 changed files with 68 additions and 33 deletions
@@ -197,12 +197,12 @@ open class LookupStorage(
@TestOnly
fun forceGC() {
removeGarbageForTests()
flush(false)
flush()
}
@TestOnly
fun dump(lookupSymbols: Set<LookupSymbol>): String {
flush(false)
flush()
val sb = StringBuilder()
val p = Printer(sb)
@@ -33,15 +33,6 @@ interface BasicMap<KEY, VALUE> : PersistentStorage<KEY, VALUE> {
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) {
check(!memoryCachesOnly) { "Expected memoryCachesOnly = false but it is `true`" }
}
/**
* 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.).
@@ -54,18 +45,6 @@ interface BasicMap<KEY, VALUE> : PersistentStorage<KEY, VALUE> {
}
}
/**
* 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 StringBuilder().apply {
@@ -17,10 +17,11 @@
package org.jetbrains.kotlin.incremental.storage
import org.jetbrains.annotations.TestOnly
import java.io.Closeable
import java.io.File
import java.io.IOException
open class BasicMapsOwner(val cachesDir: File) {
open class BasicMapsOwner(val cachesDir: File) : Closeable {
private val maps = arrayListOf<BasicMap<*, *>>()
companion object {
@@ -36,11 +37,11 @@ open class BasicMapsOwner(val cachesDir: File) {
return map
}
fun flush(memoryCachesOnly: Boolean) {
forEachMapSafe("flush") { it.flush(memoryCachesOnly) }
fun flush() {
forEachMapSafe("flush", BasicMap<*, *>::flush)
}
open fun close() {
override fun close() {
forEachMapSafe("close", BasicMap<*, *>::close)
}
@@ -48,8 +49,32 @@ open class BasicMapsOwner(val cachesDir: File) {
forEachMapSafe("deleteStorageFiles", BasicMap<*, *>::deleteStorageFiles)
}
/**
* DEPRECATED: This API should be removed because
* - It's not clear what [memoryCachesOnly] means.
* - In the past, when `memoryCachesOnly=true` we applied a small optimization: Checking
* [com.intellij.util.io.PersistentHashMap.isDirty] before calling [com.intellij.util.io.PersistentHashMap.force]. However, if that
* optimization is useful, it's better to always do it (perhaps inside the [com.intellij.util.io.PersistentHashMap.force] method
* itself) rather than doing it based on the value of this parameter.
*
* Instead, just call [flush] (without a parameter) directly.
*/
fun flush(@Suppress("UNUSED_PARAMETER") memoryCachesOnly: Boolean) {
flush()
}
/**
* DEPRECATED: This API should be removed because:
* - It's not obvious what "clean" means: It does not exactly describe the current implementation, and it also sounds similar to
* "clear" which means removing all the map entries, but this method does not do that.
* - This method currently calls [close] (and [deleteStorageFiles]). However, [close] is often already called separately and
* automatically, so this API makes it more likely for [close] to be accidentally called twice.
*
* Instead, just call [close] and/or [deleteStorageFiles] explicitly.
*/
fun clean() {
forEachMapSafe("clean", BasicMap<*, *>::clean)
close()
deleteStorageFiles()
}
@Synchronized
@@ -9,11 +9,16 @@ 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].
* back to the underlying storage on [applyChanges], [flush], or [close].
*/
interface InMemoryStorageInterface<KEY, VALUE> : PersistentStorage<KEY, VALUE> {
/** Applies in-memory changes to the underlying [PersistentStorage], then calls [clearChanges]. */
/**
* Applies in-memory changes to the underlying [PersistentStorage], then calls [clearChanges].
*
* Note that the changes are only propagated to the underlying [PersistentStorage], they may not be written to [storageFile] yet. If
* you want to propagate the changes to [storageFile], call [flush] instead.
*/
fun applyChanges()
/** Removes all in-memory changes. */
@@ -120,6 +125,12 @@ open class InMemoryStorage<KEY, VALUE>(
removedKeys.clear()
}
@Synchronized
override fun flush() {
applyChanges()
storage.flush()
}
@Synchronized
override fun close() {
applyChanges()
@@ -83,6 +83,11 @@ open class LazyStorage<KEY, VALUE>(
getStorageIfExists()?.remove(key)
}
@Synchronized
override fun flush() {
storage?.force()
}
@Synchronized
override fun close() {
storage?.close()
@@ -23,7 +23,7 @@ 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].
* Changes to this map may be written to [storageFile] at any time, and it is guaranteed to be written on [flush] or [close].
*
* This interface is similar to but simpler than [com.intellij.util.io.PersistentMapBase].
*/
@@ -43,7 +43,10 @@ interface PersistentStorage<KEY, VALUE> : Closeable {
fun remove(key: KEY)
/** Writes any remaining in-memory changes to [storageFile] and closes this map. */
/** Writes any remaining in-memory changes to [storageFile]. */
fun flush()
/** Writes any remaining in-memory changes to [storageFile] ([flush]) and closes this map. */
override fun close()
}
@@ -97,6 +100,11 @@ abstract class PersistentStorageWrapper<KEY, VALUE>(
storage.remove(key)
}
@Synchronized
override fun flush() {
storage.flush()
}
@Synchronized
override fun close() {
storage.close()
@@ -47,6 +47,11 @@ abstract class PersistentStorageAdapter<KEY, VALUE, INTERNAL_KEY, INTERNAL_VALUE
storage.remove(publicToInternalKey(key))
}
@Synchronized
override fun flush() {
storage.close()
}
@Synchronized
override fun close() {
storage.close()
@@ -39,6 +39,8 @@ private class InMemoryStorageWrapperMock : InMemoryStorageInterface<Any, Any> {
override val keys: Set<Any> = emptySet()
override fun flush() {}
override fun close() {}
override fun remove(key: Any) {}
@@ -73,7 +73,7 @@ class RelocatableCachesTest : TestWithWorkingDir() {
val filesToAdd = if (reverseFiles) files.reversedSet() else files
val lookupsToAdd = if (reverseLookups) lookups.reversedMultiMap() else lookups
lookupStorage.addAll(lookupsToAdd, filesToAdd)
lookupStorage.flush(memoryCachesOnly = false)
lookupStorage.flush()
}
private val File.storageRoot: File