Open dmytrodanylyk opened 5 years ago
Currently, every operation lock memory and disk cache. You can boost performance a lot if you perform lock on particular key, not entire store.
This is already being addressed in #4 , expect an update in binaries soon!
@vladimir-ivanov-epam I was addressing a different issue. If you can, ping me in a slack channel of kotlinlang
.
There is one more thing I wanted to pointed out, I don't see any locking mechanism for entire operations. It seems your operations are not atomic which may result in nasty issues.
suspend fun save(key: String, data: Any?, lifeTimeMillis: Long = 0, isExpirable: Boolean = true) = scope.async {
val record = Record(data, isExpirable, lifeTimeMillis)
memory.saveRecord(key, record) // 1
if (diskCache.storedMB() >= maxMbCacheSize) {
System.out.println("Record can not be persisted because it would exceed the max limit megabytes settled down")
} else {
diskCache.saveRecord(key, record) // 2
}
deleteExpirableRecordsAction.deleteExpirableRecords().await()
}
E.g. Consider the following scenario:
async { save("key1", "data1") } // coroutine1
async { save("key2", "data2") } // coroutine2
Because save
operation doesn have a lock, following may happen:
coroutine1
executes memory.saveRecord(key, record)
coroutine2
executes memory.saveRecord(key, record)
coroutine2
executes diskCache.saveRecord(key, record)
coroutine1
executes diskCache.saveRecord(key, record)
As a result memory cache contains value data1
and disk cache contains value data2
.
Hi,
Great library, I had a brief look into the source code, here are some of my thoughts:
Dependencies
Not everyone is happy by bringing unnecessary dependencies, better to put them as separate libraries. E.g.
com.epam.coroutinecache:coroutines-cache-gson-mapper
API
CoroutinesCache
has aCoroutineScope
parameter. I think cache operations should be scoped, not enire cache. In most cases, cache is a singleton and we create it once per app lifetime since initialization of this object usually takes some time. Also, you default toGlobalScope
which is represented byDispatchers.Default
, in your case you should be usingDispatchers.IO
Bitmap
.Performance
LRU
implementation of memory and disk cache would be great.E.g.
@ProviderKey("TestKey", "Bitmap::class")
? With this approach you don't need reflection and can just immediately retrieve correct serializer/deserializer.Realiability For
DiskCache
it's better to use AtomicFile to have a backup.