[LL] CleanableSoftValueCache: Don't perform cleanup when putting the same value

- If we have some value `X` in the cache, and we put `X` a second time
  into the same location, `X` shouldn't be cleaned up because it wasn't
  actually removed from the cache.
- With this feature, it's necessary to implement `put` in terms of
  `backingMap.compute`, because we need to compare the old and the new
  value atomically, and based on that, decide whether to create a new
  soft reference.

^KT-61222
This commit is contained in:
Marco Pennekamp
2024-01-17 17:14:16 +01:00
committed by Space Team
parent 80a2382cf6
commit 53e545aa3f
@@ -147,15 +147,37 @@ class CleanableSoftValueCache<K : Any, V : Any>(
/**
* Adds or replaces [value] to/in the cache at the given [key]. Must be called in a read action.
*
* @return The old value that has been replaced, if any. As replacement constitutes removal, the cleaner associated with the value will
* be invoked by [put].
* As replacement constitutes removal, cleanup will be performed on the replaced value. When the existing value and the new value are
* the same (referentially equal), cleanup will not be performed, because the existing value effectively wasn't removed from the cache.
*
* @return The old value that has been replaced, if any.
*/
fun put(key: K, value: V): V? {
val oldRef = backingMap.put(key, createSoftReference(key, value))
oldRef?.performCleanup()
// We implement `put` in terms of `backingMap.compute` to avoid creation of a new soft reference when the old and the new value are
// referentially equal. A combination of `backingMap.get` and `backingMap.put` would not be atomic, because the existing value
// fetched with `backingMap.get` might be outdated by the time we invoke `backingMap.put` based on the `old === new` comparison.
// This function's implementation is different from `CleanableSoftValueCache.compute` because `put` needs to return the old value,
// not the new value.
var oldValue: V? = null
var removedRef: SoftReferenceWithCleanup<K, V>? = null
// See `compute` for additional comments on the implementation, as it is similar to this implementation.
backingMap.compute(key) { _, currentRef ->
val currentValue = currentRef?.get()
oldValue = currentValue
if (value === currentValue) {
return@compute currentRef
}
removedRef = currentRef
createSoftReference(key, value)
}
removedRef?.performCleanup()
processQueue()
return oldRef?.get()
return oldValue
}
/**