Bläddra i källkod

Remove cache delegate to prevent simultaneous op

onevcat 6 år sedan
förälder
incheckning
a95db41738
1 ändrade filer med 12 tillägg och 17 borttagningar
  1. 12 17
      Sources/Cache/MemoryStorage.swift

+ 12 - 17
Sources/Cache/MemoryStorage.swift

@@ -43,12 +43,17 @@ public enum MemoryStorage {
     /// items from memory.
     /// items from memory.
     public class Backend<T: CacheCostCalculable> {
     public class Backend<T: CacheCostCalculable> {
         let storage = NSCache<NSString, StorageObject<T>>()
         let storage = NSCache<NSString, StorageObject<T>>()
-        var keys = Set<String>()
 
 
-        var cleanTimer: Timer? = nil
-        let lock = NSLock()
+        // Keys trackes the objects once inside the storage. For object removing triggered by user, the corresponding
+        // key would be also removed. However, for the object removing triggered by cache rule/policy of system, the
+        // key will be remained there until next `removeExpired` happens.
+        //
+        // Breaking the strict tracking could save additional locking behaviors.
+        // See https://github.com/onevcat/Kingfisher/issues/1233
+        var keys = Set<String>()
 
 
-        let cacheDelegate = CacheDelegate<StorageObject<T>>()
+        private var cleanTimer: Timer? = nil
+        private let lock = NSLock()
 
 
         /// The config used in this storage. It is a value you can set and
         /// The config used in this storage. It is a value you can set and
         /// use to config the storage in air.
         /// use to config the storage in air.
@@ -67,10 +72,6 @@ public enum MemoryStorage {
             self.config = config
             self.config = config
             storage.totalCostLimit = config.totalCostLimit
             storage.totalCostLimit = config.totalCostLimit
             storage.countLimit = config.countLimit
             storage.countLimit = config.countLimit
-            storage.delegate = cacheDelegate
-            cacheDelegate.onObjectRemoved.delegate(on: self) { (self, obj) in
-                self.keys.remove(obj.key)
-            }
 
 
             cleanTimer = .scheduledTimer(withTimeInterval: config.cleanInterval, repeats: true) { [weak self] _ in
             cleanTimer = .scheduledTimer(withTimeInterval: config.cleanInterval, repeats: true) { [weak self] _ in
                 guard let self = self else { return }
                 guard let self = self else { return }
@@ -84,6 +85,9 @@ public enum MemoryStorage {
             for key in keys {
             for key in keys {
                 let nsKey = key as NSString
                 let nsKey = key as NSString
                 guard let object = storage.object(forKey: nsKey) else {
                 guard let object = storage.object(forKey: nsKey) else {
+                    // This could happen if the object is moved by cache `totalCostLimit` or `countLimit` rule.
+                    // We didn't remove the key yet until now, since we do not want to introduce additonal lock.
+                    // See https://github.com/onevcat/Kingfisher/issues/1233
                     keys.remove(key)
                     keys.remove(key)
                     continue
                     continue
                 }
                 }
@@ -163,15 +167,6 @@ public enum MemoryStorage {
             storage.removeAllObjects()
             storage.removeAllObjects()
             keys.removeAll()
             keys.removeAll()
         }
         }
-
-        class CacheDelegate<T>: NSObject, NSCacheDelegate {
-            let onObjectRemoved = Delegate<T, Void>()
-            func cache(_ cache: NSCache<AnyObject, AnyObject>, willEvictObject obj: Any) {
-                if let obj = obj as? T {
-                    onObjectRemoved.call(obj)
-                }
-            }
-        }
     }
     }
 }
 }