Refactoring to reduce a bunch of lock churn on cleanup. (#212)

* Refactoring to reduce a bunch of lock churn on cleanup. * CHANGELOG entry
pinterest · Feb 23, 2018 · d3a24a1 · d3a24a1
1 parent 6ee9edc
commit d3a24a1
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 * Add your own contributions to the next release on the line below this with your name.
 - [fix] Fix up warnings and upgrade to PINOperation 1.1.1: [#213](https://github.com/pinterest/PINCache/pull/213)
+- [performance] Reduce locking churn in cleanup methods. [#212](https://github.com/pinterest/PINCache/pull/212)
 
 ## 3.0.1 -- Beta 6
 - [fix] Add some sane limits to the disk cache: [#201]https://github.com/pinterest/PINCache/pull/201

diff --git a/Source/PINDiskCache.m b/Source/PINDiskCache.m
@@ -577,48 +577,64 @@ - (BOOL)removeFileAndExecuteBlocksForKey:(NSString *)key
 
 - (void)trimDiskToSize:(NSUInteger)trimByteCount
 {
+    NSMutableArray *keysToRemove = nil;
+
     [self lockForWriting];
         if (_byteCount > trimByteCount) {
+            keysToRemove = [[NSMutableArray alloc] init];
+
             NSArray *keysSortedBySize = [_metadata keysSortedByValueUsingComparator:^NSComparisonResult(PINDiskCacheMetadata * _Nonnull obj1, PINDiskCacheMetadata * _Nonnull obj2) {
                 return [obj1.size compare:obj2.size];
             }];
 
+            NSUInteger bytesSaved = 0;
             for (NSString *key in [keysSortedBySize reverseObjectEnumerator]) { // largest objects first
-                [self unlock];
-
-                //unlock, removeFileAndExecuteBlocksForKey handles locking itself
-                [self removeFileAndExecuteBlocksForKey:key];
-
-                [self lock];
-
-                if (_byteCount <= trimByteCount)
+                [keysToRemove addObject:key];
+                NSNumber *byteSize = _metadata[key].size;
+                if (byteSize) {
+                    bytesSaved += [byteSize unsignedIntegerValue];
+                }
+                if (_byteCount - bytesSaved <= trimByteCount) {
                     break;
+                }
             }
         }
     [self unlock];
+
+    for (NSString *key in keysToRemove) {
+        [self removeFileAndExecuteBlocksForKey:key];
+    }
 }
 
 - (void)trimDiskToSizeByDate:(NSUInteger)trimByteCount
 {
+    NSMutableArray *keysToRemove = nil;
+
     [self lockForWriting];
         if (_byteCount > trimByteCount) {
+            keysToRemove = [[NSMutableArray alloc] init];
+
             NSArray *keysSortedByDate = [_metadata keysSortedByValueUsingComparator:^NSComparisonResult(PINDiskCacheMetadata * _Nonnull obj1, PINDiskCacheMetadata * _Nonnull obj2) {
                 return [obj1.date compare:obj2.date];
             }];
 
+            NSUInteger bytesSaved = 0;
             for (NSString *key in keysSortedByDate) { // oldest objects first
-                [self unlock];
-
-                //unlock, removeFileAndExecuteBlocksForKey handles locking itself
-                [self removeFileAndExecuteBlocksForKey:key];
-
-                [self lock];
-
-                if (_byteCount <= trimByteCount)
+                [keysToRemove addObject:key];
+                NSNumber *byteSize = _metadata[key].size;
+                if (byteSize) {
+                    bytesSaved += [byteSize unsignedIntegerValue];
+                }
+                if (_byteCount - bytesSaved <= trimByteCount) {
                     break;
+                }
             }
         }
     [self unlock];
+
+    for (NSString *key in keysToRemove) {
+        [self removeFileAndExecuteBlocksForKey:key];
+    }
 }
 
 - (void)trimDiskToDate:(NSDate *)trimDate
@@ -627,24 +643,25 @@ - (void)trimDiskToDate:(NSDate *)trimDate
         NSArray *keysSortedByDate = [_metadata keysSortedByValueUsingComparator:^NSComparisonResult(PINDiskCacheMetadata * _Nonnull obj1, PINDiskCacheMetadata * _Nonnull obj2) {
             return [obj1.date compare:obj2.date];
         }];
+
+        NSMutableArray *keysToRemove = [[NSMutableArray alloc] init];
 
         for (NSString *key in keysSortedByDate) { // oldest files first
             NSDate *accessDate = _metadata[key].date;
             if (!accessDate)
                 continue;
 
             if ([accessDate compare:trimDate] == NSOrderedAscending) { // older than trim date
-                [self unlock];
-
-                //unlock, removeFileAndExecuteBlocksForKey handles locking itself
-                [self removeFileAndExecuteBlocksForKey:key];
-
-                [self lock];
+                [keysToRemove addObject:key];
             } else {
                 break;
             }
         }
     [self unlock];
+
+    for (NSString *key in keysToRemove) {
+        [self removeFileAndExecuteBlocksForKey:key];
+    }
 }
 
 - (void)trimToAgeLimitRecursively