diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index dea8df8..286d593 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,16 +12,14 @@ on: env: - # Use Xcode 15.2 or newer to support VisionOS - DEVELOPER_DIR: /Applications/Xcode_15.2.app + # Must exist on the current macos-latest runner image: + # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md + DEVELOPER_DIR: /Applications/Xcode_26.5.0.app/Contents/Developer jobs: test: name: Test runs-on: macos-latest - strategy: - matrix: - platform: ['iOS Simulator,name=iPhone 15'] steps: - uses: actions/checkout@v2 - name: Test diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index 339c8e1..1b6cb90 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -7,8 +7,9 @@ on: required: true env: - # Use Xcode 15.2 or newer to support VisionOS - DEVELOPER_DIR: /Applications/Xcode_15.2.app + # Must exist on the current macos-latest runner image: + # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md + DEVELOPER_DIR: /Applications/Xcode_26.5.0.app/Contents/Developer jobs: create_release: diff --git a/Makefile b/Makefile index 9caccff..6b1923e 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -PLATFORM="platform=iOS Simulator,name=iPhone 15" +PLATFORM="platform=iOS Simulator,name=iPhone 17" SDK="iphonesimulator" SHELL=/bin/bash -o pipefail XCODE_MAJOR_VERSION=$(shell xcodebuild -version | HEAD -n 1 | sed -E 's/Xcode ([0-9]+).*/\1/') @@ -16,14 +16,14 @@ analyze: ONLY_ACTIVE_ARCH=NO \ CODE_SIGNING_REQUIRED=NO \ CLANG_ANALYZER_OUTPUT=plist-html \ - CLANG_ANALYZER_OUTPUT_DIR="$(shell pwd)/clang" | xcpretty + CLANG_ANALYZER_OUTPUT_DIR="$(shell pwd)/clang" | xcbeautify if [[ -n `find $(shell pwd)/clang -name "*.html"` ]] ; then rm -rf `pwd`/clang; exit 1; fi rm -rf $(shell pwd)/clang test: xcodebuild clean test -destination ${PLATFORM} -sdk ${SDK} -workspace PINCache.xcworkspace -scheme PINCache \ ONLY_ACTIVE_ARCH=NO \ - CODE_SIGNING_REQUIRED=NO | xcpretty + CODE_SIGNING_REQUIRED=NO | xcbeautify carthage: carthage update --no-use-binaries --no-build @@ -41,6 +41,6 @@ example: fi xcodebuild clean build -project ${IOS_EXAMPLE_PROJECT} -scheme ${EXAMPLE_SCHEME} -destination ${PLATFORM} -sdk ${SDK} \ ONLY_ACTIVE_ARCH=NO \ - CODE_SIGNING_REQUIRED=NO | xcpretty + CODE_SIGNING_REQUIRED=NO | xcbeautify all: carthage cocoapods test analyze spm example diff --git a/Source/PINDiskCache.m b/Source/PINDiskCache.m index fc6b0c8..4eae46f 100644 --- a/Source/PINDiskCache.m +++ b/Source/PINDiskCache.m @@ -54,6 +54,45 @@ typedef NS_ENUM(NSUInteger, PINDiskCacheCondition) { return url.fileSystemRepresentation; } +// Metadata dates and size are nil when -resourceValuesForKeys: failed during the +// initial disk scan (e.g. the file disappeared mid-scan). Passing nil to -compare: +// is undefined: [NSDate compare:nil] returns NSOrderedSame in practice, making the +// date sorts weakly inconsistent (nil-dated entries compare "equal" to everything, +// so eviction order becomes arbitrary), while [NSNumber compare:nil] RAISES +// NSInvalidArgumentException — a real crash in trimDiskToSize: with a nil size. +// Define total orders instead, placing nil so that unknown-metadata entries are +// evicted FIRST on every trim path: nil dates sort ascending-first (oldest), and +// nil sizes sort descending-last (largest) because trimDiskToSize: walks its sort +// in reverse, largest first. +NS_INLINE NSComparisonResult PINDiskCacheCompareDates(NSDate *date1, NSDate *date2) +{ + if (date1 == nil && date2 == nil) { + return NSOrderedSame; + } + if (date1 == nil) { + return NSOrderedAscending; + } + if (date2 == nil) { + return NSOrderedDescending; + } + return [date1 compare:date2]; +} + +NS_INLINE NSComparisonResult PINDiskCacheCompareSizes(NSNumber *size1, NSNumber *size2) +{ + if (size1 == nil && size2 == nil) { + return NSOrderedSame; + } + if (size1 == nil) { + return NSOrderedDescending; + } + if (size2 == nil) { + return NSOrderedAscending; + } + return [size1 compare:size2]; +} + + @interface PINDiskCacheMetadata : NSObject // When the object was added to the disk cache @property (nonatomic, strong) NSDate *createdDate; @@ -66,6 +105,48 @@ @interface PINDiskCacheMetadata : NSObject @property (nonatomic) NSInteger accessCount; @end +// Comparator ordering metadata by eviction priority: entries that sort first are evicted first. +typedef NSComparisonResult (^PINDiskCacheEvictionPriorityComparator)(PINDiskCacheMetadata *metadata1, PINDiskCacheMetadata *metadata2); + +// Bounded selection of eviction candidates: one pass over the metadata keeping at most +// `limit` keys that sort first under `priorityComparator`, in O(limit) memory. +// +// This replaces -keysSortedByValueUsingComparator: in the trim paths. The full sort +// materializes O(N) contiguous keys+values storage inside CoreFoundation; on +// heavily-loaded caches under memory pressure that allocation can fail (trapping in +// __CFCreateArrayStorage) — which also meant a cache that grew too big to sort could +// never trim again. Must be called with the instance lock held. +static NSArray *PINDiskCacheSelectEvictionCandidates( + NSDictionary *metadata, + NSUInteger limit, + NSSet *excludedKeys, + PINDiskCacheEvictionPriorityComparator priorityComparator) +{ + NSMutableArray *candidates = [[NSMutableArray alloc] initWithCapacity:limit + 1]; + NSComparisonResult (^keyComparator)(NSString *, NSString *) = ^NSComparisonResult(NSString *key1, NSString *key2) { + return priorityComparator(metadata[key1], metadata[key2]); + }; + [metadata enumerateKeysAndObjectsUsingBlock:^(NSString *key, PINDiskCacheMetadata *entryMetadata, BOOL *stop) { + if ([excludedKeys containsObject:key]) { + return; + } + // Cheap rejection: not higher priority than the lowest-priority retained candidate. + if (candidates.count == limit && + priorityComparator(entryMetadata, metadata[candidates.lastObject]) != NSOrderedAscending) { + return; + } + NSUInteger insertionIndex = [candidates indexOfObject:key + inSortedRange:NSMakeRange(0, candidates.count) + options:NSBinarySearchingInsertionIndex + usingComparator:keyComparator]; + [candidates insertObject:key atIndex:insertionIndex]; + if (candidates.count > limit) { + [candidates removeLastObject]; + } + }]; + return candidates; +} + @interface PINDiskCache () { PINDiskCacheSerializerBlock _serializer; PINDiskCacheDeserializerBlock _deserializer; @@ -797,34 +878,58 @@ - (BOOL)removeFileAndExecuteBlocksForKey:(NSString *)key return YES; } +// Upper bound on eviction candidates selected per pass. Bounds the trim's memory to +// O(batch) regardless of cache entry count; the outer loop repeats until under target. +static const NSUInteger PINDiskCacheEvictionBatchLimit = 256; + - (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 - [keysToRemove addObject:key]; - NSNumber *byteSize = _metadata[key].size; - if (byteSize) { - bytesSaved += [byteSize unsignedIntegerValue]; - } - if (_byteCount - bytesSaved <= trimByteCount) { - break; + // Largest entries evicted first; nil sizes treated as largest (their disk usage is + // unaccounted, so they are the least trustworthy entries to keep). + PINDiskCacheEvictionPriorityComparator largestFirst = ^NSComparisonResult(PINDiskCacheMetadata *metadata1, PINDiskCacheMetadata *metadata2) { + return PINDiskCacheCompareSizes(metadata2.size, metadata1.size); + }; + + // Keys already attempted this trim: removeFileAndExecuteBlocksForKey: can decline a + // key without touching its metadata (file already gone), so without exclusion the + // next batch would select the same key forever. + NSMutableSet *attemptedKeys = [[NSMutableSet alloc] init]; + BOOL trimming = YES; + while (trimming) { + NSMutableArray *keysToRemove = nil; + + [self lockForWriting]; + if (_byteCount > trimByteCount) { + NSArray *candidates = PINDiskCacheSelectEvictionCandidates(_metadata, PINDiskCacheEvictionBatchLimit, attemptedKeys, largestFirst); + keysToRemove = [[NSMutableArray alloc] init]; + NSUInteger bytesSaved = 0; + for (NSString *key in candidates) { + [keysToRemove addObject:key]; + NSNumber *byteSize = _metadata[key].size; + if (byteSize) { + bytesSaved += [byteSize unsignedIntegerValue]; + } + // Avoid NSUInteger underflow when metadata sizes overshoot _byteCount + // (stale bookkeeping): a wrapped subtraction never satisfies the break + // and the loop evicts the entire cache. + if (bytesSaved >= _byteCount || _byteCount - bytesSaved <= trimByteCount) { + trimming = NO; + break; + } } + } else { + trimming = NO; } + [self unlock]; + + if (keysToRemove.count == 0) { + // Nothing evictable this pass (metadata empty or fully attempted); stop rather than spin. + break; + } + for (NSString *key in keysToRemove) { + [attemptedKeys addObject:key]; + [self removeFileAndExecuteBlocksForKey:key]; } - [self unlock]; - - for (NSString *key in keysToRemove) { - [self removeFileAndExecuteBlocksForKey:key]; } } @@ -835,77 +940,84 @@ - (void)trimDiskToSizeByEvictionStrategy:(NSUInteger)trimByteCount [self removeExpiredObjects]; } - NSMutableArray *keysToRemove = nil; - - [self lockForWriting]; - if (_byteCount > trimByteCount) { - PINCacheEvictionStrategy strategy = self->_evictionStrategy; - keysToRemove = [[NSMutableArray alloc] init]; - - // last modified represents last access. - NSArray *keysSortedByEvictionStrategy = nil; - switch (strategy) { - case PINCacheEvictionStrategyLeastRecentlyUsed: - keysSortedByEvictionStrategy = [_metadata keysSortedByValueUsingComparator:^NSComparisonResult(PINDiskCacheMetadata * _Nonnull obj1, PINDiskCacheMetadata * _Nonnull obj2) { - return [obj1.lastModifiedDate compare:obj2.lastModifiedDate]; - }]; - break; - - case PINCacheEvictionStrategyLeastFrequentlyUsed: - keysSortedByEvictionStrategy = [_metadata keysSortedByValueUsingComparator:^NSComparisonResult(PINDiskCacheMetadata * _Nonnull obj1, PINDiskCacheMetadata * _Nonnull obj2) { - if (obj1.accessCount < obj2.accessCount) { - return NSOrderedAscending; - } else if (obj1.accessCount > obj2.accessCount) { - return NSOrderedDescending; - } else { - return [obj1.lastModifiedDate compare:obj2.lastModifiedDate]; - } - }]; - break; - } - - NSUInteger bytesSaved = 0; - // objects accessed last first. - for (NSString *key in keysSortedByEvictionStrategy) { - [keysToRemove addObject:key]; - NSNumber *byteSize = _metadata[key].size; - if (byteSize) { - bytesSaved += [byteSize unsignedIntegerValue]; - } - if (_byteCount - bytesSaved <= trimByteCount) { - break; + // last modified represents last access; least-recently/frequently used evicted first. + PINDiskCacheEvictionPriorityComparator lruFirst = ^NSComparisonResult(PINDiskCacheMetadata *metadata1, PINDiskCacheMetadata *metadata2) { + return PINDiskCacheCompareDates(metadata1.lastModifiedDate, metadata2.lastModifiedDate); + }; + PINDiskCacheEvictionPriorityComparator lfuFirst = ^NSComparisonResult(PINDiskCacheMetadata *metadata1, PINDiskCacheMetadata *metadata2) { + if (metadata1.accessCount < metadata2.accessCount) { + return NSOrderedAscending; + } else if (metadata1.accessCount > metadata2.accessCount) { + return NSOrderedDescending; + } else { + return PINDiskCacheCompareDates(metadata1.lastModifiedDate, metadata2.lastModifiedDate); + } + }; + + // Keys already attempted this trim: removeFileAndExecuteBlocksForKey: can decline a + // key without touching its metadata (file already gone), so without exclusion the + // next batch would select the same key forever. + NSMutableSet *attemptedKeys = [[NSMutableSet alloc] init]; + BOOL trimming = YES; + while (trimming) { + NSMutableArray *keysToRemove = nil; + + [self lockForWriting]; + if (_byteCount > trimByteCount) { + PINDiskCacheEvictionPriorityComparator priorityComparator = + (self->_evictionStrategy == PINCacheEvictionStrategyLeastFrequentlyUsed) ? lfuFirst : lruFirst; + NSArray *candidates = PINDiskCacheSelectEvictionCandidates(_metadata, PINDiskCacheEvictionBatchLimit, attemptedKeys, priorityComparator); + keysToRemove = [[NSMutableArray alloc] init]; + NSUInteger bytesSaved = 0; + for (NSString *key in candidates) { + [keysToRemove addObject:key]; + NSNumber *byteSize = _metadata[key].size; + if (byteSize) { + bytesSaved += [byteSize unsignedIntegerValue]; + } + // Avoid NSUInteger underflow when metadata sizes overshoot _byteCount + // (stale bookkeeping): a wrapped subtraction never satisfies the break + // and the loop evicts the entire cache. + if (bytesSaved >= _byteCount || _byteCount - bytesSaved <= trimByteCount) { + trimming = NO; + break; + } } + } else { + trimming = NO; } + [self unlock]; + + if (keysToRemove.count == 0) { + // Nothing evictable this pass (metadata empty or fully attempted); stop rather than spin. + break; + } + for (NSString *key in keysToRemove) { + [attemptedKeys addObject:key]; + [self removeFileAndExecuteBlocksForKey:key]; } - [self unlock]; - - for (NSString *key in keysToRemove) { - [self removeFileAndExecuteBlocksForKey:key]; } } - (void)trimDiskToDate:(NSDate *)trimDate { [self lockForWriting]; - NSArray *keysSortedByCreatedDate = [_metadata keysSortedByValueUsingComparator:^NSComparisonResult(PINDiskCacheMetadata * _Nonnull obj1, PINDiskCacheMetadata * _Nonnull obj2) { - return [obj1.createdDate compare:obj2.createdDate]; - }]; - + // Date trimming is a pure threshold filter, so no sort is needed — the previous + // full sort existed only to enable an early break, and its O(N) temporary + // storage is the same allocation-failure hazard as the sorts replaced above. NSMutableArray *keysToRemove = [[NSMutableArray alloc] init]; - - for (NSString *key in keysSortedByCreatedDate) { // oldest files first - NSDate *createdDate = _metadata[key].createdDate; - if (!createdDate || _metadata[key].ageLimit > 0.0) - continue; - + + [_metadata enumerateKeysAndObjectsUsingBlock:^(NSString *key, PINDiskCacheMetadata *metadata, BOOL *stop) { + NSDate *createdDate = metadata.createdDate; + if (!createdDate || metadata.ageLimit > 0.0) { + return; + } if ([createdDate compare:trimDate] == NSOrderedAscending) { // older than trim date [keysToRemove addObject:key]; - } else { - break; } - } + }]; [self unlock]; - + for (NSString *key in keysToRemove) { [self removeFileAndExecuteBlocksForKey:key]; } diff --git a/Tests/PINCacheTests.m b/Tests/PINCacheTests.m index c154a12..47e850f 100644 --- a/Tests/PINCacheTests.m +++ b/Tests/PINCacheTests.m @@ -535,6 +535,31 @@ - (void)testDiskSizeByDateWithObjectExpiration XCTAssertTrue([keys.lastObject isEqualToString:key1] || [keys.lastObject isEqualToString:key4]); } +// The bounded-batch trim (256 candidates per pass) must evict across multiple batches +// and still reach its byte target. This guards the replacement of the full +// keysSortedByValueUsingComparator: sort, whose O(N) temporary storage could fail to +// allocate on heavily-loaded caches under memory pressure (trapping in +// __CFCreateArrayStorage) — after which the cache could never trim again. +- (void)testTrimToSizeByEvictionStrategyWithMoreEntriesThanOneBatch +{ + [self.cache.diskCache removeAllObjects]; + + const NSUInteger entryCount = 300; // > one 256-entry selection batch + for (NSUInteger i = 0; i < entryCount; i++) { + [self.cache.diskCache setObject:@"value" forKey:[NSString stringWithFormat:@"batchKey%03lu", (unsigned long)i]]; + } + XCTAssertTrue(self.cache.diskCache.byteCount > 0, @"setup should have written bytes"); + + [self.cache.diskCache trimToSizeByEvictionStrategy:1]; + + __block NSUInteger remainingKeys = 0; + [self.cache.diskCache enumerateObjectsWithBlock:^(NSString * _Nonnull key, NSURL * _Nullable fileURL, BOOL * _Nonnull stop) { + remainingKeys++; + }]; + XCTAssertTrue(remainingKeys <= 1, @"trim must evict past the first selection batch (had %lu keys left)", (unsigned long)remainingKeys); + XCTAssertTrue(self.cache.diskCache.byteCount <= 1, @"trim must reach its byte target across batches"); +} + - (void)testOneThousandAndOneWrites { NSUInteger max = 1001;