From 4062328b50825779f6b38d4badeea0fa70e0bb77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Ekstr=C3=B6m?= Date: Tue, 14 Mar 2017 12:17:46 +0100 Subject: [PATCH 1/7] Expose `enqueueGarbageCollection` Scheduling garbage collection by time isn't appropriate for short-lived applications. This adds an alternative API which allows you to schedule the garbage collection instantly. --- Sources/SPTPersistentCache.m | 5 +++++ Sources/SPTPersistentCacheGarbageCollector.h | 5 +++++ Sources/SPTPersistentCacheGarbageCollector.m | 9 ++++++-- .../SPTPersistentCacheGarbageCollectorTests.m | 21 +++++++++++++++++-- Tests/SPTPersistentCacheTests.m | 12 +++++++++++ .../SPTPersistentCache/SPTPersistentCache.h | 5 +++++ 6 files changed, 53 insertions(+), 4 deletions(-) diff --git a/Sources/SPTPersistentCache.m b/Sources/SPTPersistentCache.m index dacfb62..9aa9956 100644 --- a/Sources/SPTPersistentCache.m +++ b/Sources/SPTPersistentCache.m @@ -391,6 +391,11 @@ - (void)unscheduleGarbageCollector [self.garbageCollector unschedule]; } +- (void)enqueueGarbageCollector +{ + [self.garbageCollector enqueueGarbageCollection]; +} + - (void)pruneWithCallback:(SPTPersistentCacheResponseCallback _Nullable)callback onQueue:(dispatch_queue_t _Nullable)queue { diff --git a/Sources/SPTPersistentCacheGarbageCollector.h b/Sources/SPTPersistentCacheGarbageCollector.h index cd08170..07638de 100644 --- a/Sources/SPTPersistentCacheGarbageCollector.h +++ b/Sources/SPTPersistentCacheGarbageCollector.h @@ -68,4 +68,9 @@ */ - (void)unschedule; +/** + * Enqueues the garbage collector. Called automatically if scheduled by the above functions. + */ +- (void)enqueueGarbageCollection; + @end diff --git a/Sources/SPTPersistentCacheGarbageCollector.m b/Sources/SPTPersistentCacheGarbageCollector.m index 06ffb09..6c6137e 100644 --- a/Sources/SPTPersistentCacheGarbageCollector.m +++ b/Sources/SPTPersistentCacheGarbageCollector.m @@ -61,7 +61,7 @@ - (void)dealloc #pragma mark - -- (void)enqueueGarbageCollection:(NSTimer *)timer +- (void)enqueueGarbageCollection { __weak __typeof(self) const weakSelf = self; NSBlockOperation *operation = [NSBlockOperation blockOperationWithBlock:^{ @@ -81,6 +81,11 @@ - (void)enqueueGarbageCollection:(NSTimer *)timer [self.queue addOperation:operation]; } +- (void)garbageCollectionTimerFired:(NSTimer *)timer +{ + [self enqueueGarbageCollection]; +} + - (void)schedule { if (!SPTPersistentCacheGarbageCollectorSchedulerIsInMainQueue()) { @@ -100,7 +105,7 @@ - (void)schedule self.timer = [NSTimer timerWithTimeInterval:self.options.garbageCollectionInterval target:self - selector:@selector(enqueueGarbageCollection:) + selector:@selector(garbageCollectionTimerFired:) userInfo:nil repeats:YES]; diff --git a/Tests/SPTPersistentCacheGarbageCollectorTests.m b/Tests/SPTPersistentCacheGarbageCollectorTests.m index 8c30ab8..473a864 100644 --- a/Tests/SPTPersistentCacheGarbageCollectorTests.m +++ b/Tests/SPTPersistentCacheGarbageCollectorTests.m @@ -25,7 +25,7 @@ @interface SPTPersistentCacheGarbageCollector () @property (nonatomic, strong) NSTimer *timer; -- (void)enqueueGarbageCollection:(NSTimer *)timer; +- (void)garbageCollectionTimerFired:(NSTimer *)timer; @end @@ -97,7 +97,7 @@ - (void)testGarbageCollectorEnqueue dataCacheForUnitTests.queue = self.garbageCollector.queue; dataCacheForUnitTests.testExpectation = expectation; - [self.garbageCollector enqueueGarbageCollection:nil]; + [self.garbageCollector enqueueGarbageCollection]; [self waitForExpectationsWithTimeout:1.0 handler:^(NSError * _Nullable error) { XCTAssertTrue(dataCacheForUnitTests.wasRunRegularGCCalled); @@ -106,6 +106,23 @@ - (void)testGarbageCollectorEnqueue }]; } +- (void)testGarbageCollectorTimerFired +{ + __weak XCTestExpectation *expectation = [self expectationWithDescription:@"testGarbageCollectorEnqueue"]; + + SPTPersistentCacheForTimerProxyUnitTests *dataCacheForUnitTests = (SPTPersistentCacheForTimerProxyUnitTests *)self.garbageCollector.cache; + dataCacheForUnitTests.queue = self.garbageCollector.queue; + + dataCacheForUnitTests.testExpectation = expectation; + [self.garbageCollector garbageCollectionTimerFired:nil]; + + [self waitForExpectationsWithTimeout:1.0 handler:^(NSError * _Nullable error) { + XCTAssertTrue(dataCacheForUnitTests.wasRunRegularGCCalled); + XCTAssertTrue(dataCacheForUnitTests.wasPruneBySizeCalled); + XCTAssertFalse(dataCacheForUnitTests.wasCalledFromIncorrectQueue); + }]; +} + - (void)testIsGarbageCollectionScheduled { XCTAssertFalse(self.garbageCollector.isGarbageCollectionScheduled); diff --git a/Tests/SPTPersistentCacheTests.m b/Tests/SPTPersistentCacheTests.m index 806a1bd..c451815 100644 --- a/Tests/SPTPersistentCacheTests.m +++ b/Tests/SPTPersistentCacheTests.m @@ -871,6 +871,18 @@ - (void)testUnscheduleGarbageCollection XCTAssertFalse(cache.garbageCollector.isGarbageCollectionScheduled); } +- (void)testEnqueueGargabeCollection +{ + SPTPersistentCache *cache = [self createCacheWithTimeCallback:^NSTimeInterval{ + // Exceed expiration interval by 1 sec + return kTestEpochTime + SPTPersistentCacheDefaultExpirationTimeSec + 1; + } + expirationTime:SPTPersistentCacheDefaultExpirationTimeSec]; + + [cache.garbageCollector enqueueGarbageCollection]; + XCTAssertFalse(cache.garbageCollector.isGarbageCollectionScheduled); +} + /** * This test also checks Req.#1.2 of cache API. */ diff --git a/include/SPTPersistentCache/SPTPersistentCache.h b/include/SPTPersistentCache/SPTPersistentCache.h index 8096a79..fe75a27 100644 --- a/include/SPTPersistentCache/SPTPersistentCache.h +++ b/include/SPTPersistentCache/SPTPersistentCache.h @@ -229,6 +229,11 @@ typedef NSString * _Nonnull(^SPTPersistentCacheChooseKeyCallback)(NSArray Date: Tue, 14 Mar 2017 15:21:56 +0100 Subject: [PATCH 2/7] Expose `minimumFreeDiskSpaceFraction` Defaulting to require 10% of free disk space does not make sense for some apps. For example, on a huge disk, say 64GB, there is no reason to require 6GB free space for a 30MB cache. --- Sources/SPTPersistentCacheFileManager.m | 4 +-- Sources/SPTPersistentCacheOptions.m | 3 ++ Tests/SPTPersistentCacheFileManagerTests.m | 28 +++++++++++++++++++ .../SPTPersistentCacheOptions.h | 7 +++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Sources/SPTPersistentCacheFileManager.m b/Sources/SPTPersistentCacheFileManager.m index 61e8075..2e30742 100644 --- a/Sources/SPTPersistentCacheFileManager.m +++ b/Sources/SPTPersistentCacheFileManager.m @@ -22,8 +22,6 @@ #import "SPTPersistentCacheDebugUtilities.h" #import "SPTPersistentCacheOptions.h" -static const double SPTPersistentCacheFileManagerMinFreeDiskSpace = 0.1; - const NSUInteger SPTPersistentCacheFileManagerSubDirNameLength = 2; @implementation SPTPersistentCacheFileManager @@ -179,7 +177,7 @@ - (SPTPersistentCacheDiskSize)optimizedDiskSizeForCacheSize:(SPTPersistentCacheD SPTPersistentCacheDiskSize totalSpace = fileSystemSize.longLongValue; SPTPersistentCacheDiskSize freeSpace = fileSystemFreeSpace.longLongValue + currentCacheSize; SPTPersistentCacheDiskSize proposedCacheSize = freeSpace - llrint(totalSpace * - SPTPersistentCacheFileManagerMinFreeDiskSpace); + self.options.minimumFreeDiskSpaceFraction); tempCacheSize = MAX(0, proposedCacheSize); diff --git a/Sources/SPTPersistentCacheOptions.m b/Sources/SPTPersistentCacheOptions.m index 3ad5297..ce3c249 100644 --- a/Sources/SPTPersistentCacheOptions.m +++ b/Sources/SPTPersistentCacheOptions.m @@ -27,6 +27,7 @@ const NSUInteger SPTPersistentCacheDefaultExpirationTimeSec = 10 * 60; const NSUInteger SPTPersistentCacheDefaultGCIntervalSec = 6 * 60 + 3; const NSUInteger SPTPersistentCacheDefaultCacheSizeInBytes = 0; // unbounded +const double SPTPersistentCacheDefaultMinFreeDiskSpaceFraction = 0.1; // 10% of total disk size const NSUInteger SPTPersistentCacheMinimumGCIntervalLimit = 60; const NSUInteger SPTPersistentCacheMinimumExpirationLimit = 60; @@ -55,6 +56,7 @@ - (instancetype)init _garbageCollectionInterval = SPTPersistentCacheDefaultGCIntervalSec; _defaultExpirationPeriod = SPTPersistentCacheDefaultExpirationTimeSec; _sizeConstraintBytes = SPTPersistentCacheDefaultCacheSizeInBytes; + _minimumFreeDiskSpaceFraction = SPTPersistentCacheDefaultMinFreeDiskSpaceFraction; _maxConcurrentOperations = NSOperationQueueDefaultMaxConcurrentOperationCount; _writePriority = NSOperationQueuePriorityNormal; _writeQualityOfService = NSQualityOfServiceDefault; @@ -117,6 +119,7 @@ - (id)copyWithZone:(NSZone *)zone copy.garbageCollectionInterval = self.garbageCollectionInterval; copy.defaultExpirationPeriod = self.defaultExpirationPeriod; copy.sizeConstraintBytes = self.sizeConstraintBytes; + copy.minimumFreeDiskSpaceFraction = self.minimumFreeDiskSpaceFraction; copy.debugOutput = self.debugOutput; copy.timingCallback = self.timingCallback; diff --git a/Tests/SPTPersistentCacheFileManagerTests.m b/Tests/SPTPersistentCacheFileManagerTests.m index bdea950..78ef5fe 100644 --- a/Tests/SPTPersistentCacheFileManagerTests.m +++ b/Tests/SPTPersistentCacheFileManagerTests.m @@ -69,6 +69,7 @@ - (void)setUp SPTPersistentCacheOptions *options = [SPTPersistentCacheOptions new]; options.cachePath = SPTPersistentCacheFileManagerTestsCachePath; options.cacheIdentifier = @"test"; + options.sizeConstraintBytes = (SPTPersistentCacheDiskSize)1024 * 1024 * 1024 * 3; // 3 GiB self.options = options; self.cacheFileManager = [[SPTPersistentCacheFileManagerForTests alloc] initWithOptions:self.options]; @@ -168,6 +169,33 @@ - (void)testOptimizedDiskSizeForCacheSizeSmall XCTAssertEqual(optimizedSize, (SPTPersistentCacheDiskSize)0); } +- (void)testMinimumFreeDiskSpaceFraction +{ + const SPTPersistentCacheDiskSize diskSize = (SPTPersistentCacheDiskSize)1024 * 1024 * 1024 * 16; // 16GiB + const SPTPersistentCacheDiskSize freeSpace = (SPTPersistentCacheDiskSize)1024 * 1024 * 1024 * 8; // 8GiB + NSFileManagerMock *fileManager = [NSFileManagerMock new]; + fileManager.mock_attributesOfFileSystemForPaths = @{SPTPersistentCacheFileManagerTestsCachePath: @{NSFileSystemSize: @(diskSize), + NSFileSystemFreeSize: @(freeSpace)}}; + self.cacheFileManager.test_fileManager = fileManager; + + self.cacheFileManager.options.minimumFreeDiskSpaceFraction = 1.0; + SPTPersistentCacheDiskSize optimizedSize = [self.cacheFileManager optimizedDiskSizeForCacheSize:0]; + XCTAssertEqual(optimizedSize, (SPTPersistentCacheDiskSize)0); + + self.cacheFileManager.options.minimumFreeDiskSpaceFraction = 0.0; + optimizedSize = [self.cacheFileManager optimizedDiskSizeForCacheSize:0]; + XCTAssertEqual(optimizedSize, (SPTPersistentCacheDiskSize)self.options.sizeConstraintBytes); + + self.cacheFileManager.options.minimumFreeDiskSpaceFraction = 0.5; + optimizedSize = [self.cacheFileManager optimizedDiskSizeForCacheSize:0]; + XCTAssertEqual(optimizedSize, (SPTPersistentCacheDiskSize)0); + + const SPTPersistentCacheDiskSize twoGiB = (SPTPersistentCacheDiskSize)1024 * 1024 * 1024 * 2; + self.cacheFileManager.options.minimumFreeDiskSpaceFraction = (freeSpace - twoGiB) / (double)diskSize; + optimizedSize = [self.cacheFileManager optimizedDiskSizeForCacheSize:0]; + XCTAssertEqual(optimizedSize, twoGiB); +} + - (void)testRemoveAllDataButKeysWithoutKeys { NSString *keyOne = @"AA"; diff --git a/include/SPTPersistentCache/SPTPersistentCacheOptions.h b/include/SPTPersistentCache/SPTPersistentCacheOptions.h index 243caa6..b87cee1 100644 --- a/include/SPTPersistentCache/SPTPersistentCacheOptions.h +++ b/include/SPTPersistentCache/SPTPersistentCacheOptions.h @@ -178,6 +178,13 @@ FOUNDATION_EXPORT const NSUInteger SPTPersistentCacheMinimumExpirationLimit; * @note Defaults to `0` (unbounded). */ @property (nonatomic, assign) NSUInteger sizeConstraintBytes; +/** + * The minimum fraction of free disk space required for caching. If there is less disk space + * available than this, the cache will be purged during garbage collection until the treshold + * is met. This could mean that the whole cache is evicted if the device is low on space. + * @note Defaults to `0.1`, 10% of the total disk size. + */ +@property (nonatomic, assign) double minimumFreeDiskSpaceFraction; /** * The queue priority for garbage collection. Defaults to NSOperationQueuePriorityLow. */ From 792a0db88330719858c395ff5ca892da9b863759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20Ekstr=C3=B6m?= Date: Tue, 9 May 2017 11:05:00 +0200 Subject: [PATCH 3/7] Update Xcode settings to silence warnings --- SPTPersistentCache.xcodeproj/project.pbxproj | 8 +++++++- .../xcshareddata/xcschemes/SPTPersistentCache.xcscheme | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/SPTPersistentCache.xcodeproj/project.pbxproj b/SPTPersistentCache.xcodeproj/project.pbxproj index a674451..22defa9 100644 --- a/SPTPersistentCache.xcodeproj/project.pbxproj +++ b/SPTPersistentCache.xcodeproj/project.pbxproj @@ -378,7 +378,7 @@ isa = PBXProject; attributes = { CLASSPREFIX = SPTPersistentCache; - LastUpgradeCheck = 0820; + LastUpgradeCheck = 0830; ORGANIZATIONNAME = Spotify; TargetAttributes = { 0510FF121BA2FF7A00ED0766 = { @@ -492,7 +492,10 @@ isa = XCBuildConfiguration; baseConfigurationReference = 9C882A801C65422700D463BB /* project.xcconfig */; buildSettings = { + CLANG_WARN_INFINITE_RECURSION = YES; + CLANG_WARN_SUSPICIOUS_MOVE = YES; ENABLE_STRICT_OBJC_MSGSEND = YES; + ENABLE_TESTABILITY = YES; GCC_DYNAMIC_NO_PIC = NO; GCC_SYMBOLS_PRIVATE_EXTERN = NO; HEADER_SEARCH_PATHS = ( @@ -500,6 +503,7 @@ "$(inherited)", ); MTL_ENABLE_DEBUG_INFO = YES; + ONLY_ACTIVE_ARCH = YES; }; name = Debug; }; @@ -507,6 +511,8 @@ isa = XCBuildConfiguration; baseConfigurationReference = 9C882A801C65422700D463BB /* project.xcconfig */; buildSettings = { + CLANG_WARN_INFINITE_RECURSION = YES; + CLANG_WARN_SUSPICIOUS_MOVE = YES; ENABLE_STRICT_OBJC_MSGSEND = YES; HEADER_SEARCH_PATHS = ( include, diff --git a/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme b/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme index 43b9818..b009920 100644 --- a/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme +++ b/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme @@ -1,6 +1,6 @@ Date: Mon, 19 Jun 2017 12:46:48 +0200 Subject: [PATCH 4/7] Remove inline string creation to fix nullability errors --- SPTPersistentCache.xcodeproj/project.pbxproj | 10 +++++++++- .../xcshareddata/xcschemes/SPTPersistentCache.xcscheme | 2 +- Sources/SPTPersistentCache.m | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/SPTPersistentCache.xcodeproj/project.pbxproj b/SPTPersistentCache.xcodeproj/project.pbxproj index 22defa9..5746155 100644 --- a/SPTPersistentCache.xcodeproj/project.pbxproj +++ b/SPTPersistentCache.xcodeproj/project.pbxproj @@ -378,7 +378,7 @@ isa = PBXProject; attributes = { CLASSPREFIX = SPTPersistentCache; - LastUpgradeCheck = 0830; + LastUpgradeCheck = 0900; ORGANIZATIONNAME = Spotify; TargetAttributes = { 0510FF121BA2FF7A00ED0766 = { @@ -492,7 +492,11 @@ isa = XCBuildConfiguration; baseConfigurationReference = 9C882A801C65422700D463BB /* project.xcconfig */; buildSettings = { + CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; + CLANG_WARN_COMMA = YES; CLANG_WARN_INFINITE_RECURSION = YES; + CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; + CLANG_WARN_STRICT_PROTOTYPES = YES; CLANG_WARN_SUSPICIOUS_MOVE = YES; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; @@ -511,7 +515,11 @@ isa = XCBuildConfiguration; baseConfigurationReference = 9C882A801C65422700D463BB /* project.xcconfig */; buildSettings = { + CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; + CLANG_WARN_COMMA = YES; CLANG_WARN_INFINITE_RECURSION = YES; + CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; + CLANG_WARN_STRICT_PROTOTYPES = YES; CLANG_WARN_SUSPICIOUS_MOVE = YES; ENABLE_STRICT_OBJC_MSGSEND = YES; HEADER_SEARCH_PATHS = ( diff --git a/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme b/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme index b009920..0587e29 100644 --- a/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme +++ b/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme @@ -1,6 +1,6 @@ Date: Thu, 10 Aug 2017 16:57:24 +0200 Subject: [PATCH 5/7] Set build active arch only to NO, needed for jenkins builds --- SPTPersistentCache.xcodeproj/project.pbxproj | 1 + 1 file changed, 1 insertion(+) diff --git a/SPTPersistentCache.xcodeproj/project.pbxproj b/SPTPersistentCache.xcodeproj/project.pbxproj index 5746155..1c8c8ed 100644 --- a/SPTPersistentCache.xcodeproj/project.pbxproj +++ b/SPTPersistentCache.xcodeproj/project.pbxproj @@ -535,6 +535,7 @@ isa = XCBuildConfiguration; buildSettings = { COMBINE_HIDPI_IMAGES = YES; + ONLY_ACTIVE_ARCH = NO; OTHER_LDFLAGS = "-ObjC"; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; From 94aa8aa622c3f25e8954e5da60e3dbb483655000 Mon Sep 17 00:00:00 2001 From: Kevin Elorza Date: Fri, 11 Aug 2017 12:01:56 +0200 Subject: [PATCH 6/7] Set valid architectures --- SPTPersistentCache.xcodeproj/project.pbxproj | 2 ++ 1 file changed, 2 insertions(+) diff --git a/SPTPersistentCache.xcodeproj/project.pbxproj b/SPTPersistentCache.xcodeproj/project.pbxproj index 1c8c8ed..9f0e471 100644 --- a/SPTPersistentCache.xcodeproj/project.pbxproj +++ b/SPTPersistentCache.xcodeproj/project.pbxproj @@ -508,6 +508,7 @@ ); MTL_ENABLE_DEBUG_INFO = YES; ONLY_ACTIVE_ARCH = YES; + VALID_ARCHS = "armv7 arm64 armv7k"; }; name = Debug; }; @@ -528,6 +529,7 @@ ); MTL_ENABLE_DEBUG_INFO = NO; VALIDATE_PRODUCT = YES; + VALID_ARCHS = "armv7 arm64 armv7k"; }; name = Release; }; From 3660d47a86c61c473b06ee2e323343e93ec3518a Mon Sep 17 00:00:00 2001 From: Erik Heinemark Date: Tue, 12 Sep 2017 22:19:55 +0200 Subject: [PATCH 7/7] Update settings for XCode 9 --- .../xcshareddata/xcschemes/SPTPersistentCache.xcscheme | 2 ++ 1 file changed, 2 insertions(+) diff --git a/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme b/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme index 0587e29..d1363c1 100644 --- a/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme +++ b/SPTPersistentCache.xcodeproj/xcshareddata/xcschemes/SPTPersistentCache.xcscheme @@ -40,6 +40,7 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" + language = "" shouldUseLaunchSchemeArgsEnv = "YES">