From 0d5774e076e0b9f7660479738fbef73a7ee50769 Mon Sep 17 00:00:00 2001 From: Alexander Cohen Date: Sun, 30 Nov 2025 11:47:00 -0500 Subject: [PATCH 1/2] Add swiftformat and apply code formatting - Add .swiftformat configuration file - Remove .swift-version file - Apply consistent code formatting across codebase - Optimize MemoryArsenal set operation to reduce lookups - Update tests and benchmarks --- .swift-version | 1 - .swiftformat | 23 ++++ Sources/Arsenal/Arsenal.swift | 8 +- .../Arsenal/Implementations/DiskArsenal.swift | 45 ++++++-- .../Implementations/MemoryArsenal.swift | 41 ++++--- .../Implementations/SwiftDataArsenal.swift | 16 +-- Tests/ArsenalTests/ArsenalBenchmarks.swift | 101 ++++++++++++------ Tests/ArsenalTests/ArsenalTests.swift | 46 +++++--- 8 files changed, 194 insertions(+), 87 deletions(-) delete mode 100644 .swift-version create mode 100644 .swiftformat diff --git a/.swift-version b/.swift-version deleted file mode 100644 index 0cda48a..0000000 --- a/.swift-version +++ /dev/null @@ -1 +0,0 @@ -6.2 diff --git a/.swiftformat b/.swiftformat new file mode 100644 index 0000000..971c9aa --- /dev/null +++ b/.swiftformat @@ -0,0 +1,23 @@ +# SwiftFormat configuration + +# Format options +--indent 4 +--indentcase false +--trimwhitespace always +--wraparguments before-first +--wrapparameters before-first +--wrapcollections before-first +--maxwidth 120 +--linebreaks lf + +# Enabled rules +--enable isEmpty +--enable sortImports + +# Disabled rules +--disable andOperator +--disable blankLinesBetweenScopes +--disable redundantSelf + +# Swift version +--swiftversion 6.2 diff --git a/Sources/Arsenal/Arsenal.swift b/Sources/Arsenal/Arsenal.swift index b755ec6..54dcf56 100644 --- a/Sources/Arsenal/Arsenal.swift +++ b/Sources/Arsenal/Arsenal.swift @@ -192,7 +192,9 @@ public protocol ArsenalItem: AnyObject, Sendable { /// - identifier: A unique identifier for this cache instance. /// - costLimit: The maximum cost for the memory cache. Defaults to 500 MB. /// - maxStaleness: The maximum age for disk-cached items in seconds. Defaults to 1 day. - public convenience init(_ identifier: String, costLimit: UInt64 = UInt64(5e+8), maxStaleness: TimeInterval = 86400) { + public convenience init( + _ identifier: String, costLimit: UInt64 = UInt64(5e+8), maxStaleness: TimeInterval = 86400 + ) { self.init( identifier, resources: [ @@ -335,7 +337,9 @@ public protocol ArsenalItem: AnyObject, Sendable { } } - private func forEachResource(of types: Set, action: @Sendable @escaping (any ArsenalImp) async -> Void) async { + private func forEachResource( + of types: Set, action: @Sendable @escaping (any ArsenalImp) async -> Void + ) async { for type in types { if let res = resources[type] { await action(res) diff --git a/Sources/Arsenal/Implementations/DiskArsenal.swift b/Sources/Arsenal/Implementations/DiskArsenal.swift index ba193a8..10c7655 100644 --- a/Sources/Arsenal/Implementations/DiskArsenal.swift +++ b/Sources/Arsenal/Implementations/DiskArsenal.swift @@ -147,7 +147,9 @@ import os public func value(for key: String) -> T? { ArsenalActor.assertIsolated() - guard let url = urlProvider.url(for: key), let data = try? Data(contentsOf: url, options: .mappedIfSafe) else { + guard let url = urlProvider.url(for: key), + let data = try? Data(contentsOf: url, options: .mappedIfSafe) + else { return nil } return T.from(data: data) as? T @@ -189,14 +191,22 @@ import os return } - guard let urls = try? urlProvider.fileManager.contentsOfDirectory(at: baseURL, includingPropertiesForKeys: [.contentModificationDateKey, .fileSizeKey], options: .skipsHiddenFiles) else { + guard + let urls = try? urlProvider.fileManager.contentsOfDirectory( + at: baseURL, includingPropertiesForKeys: [.contentModificationDateKey, .fileSizeKey], + options: .skipsHiddenFiles + ) + else { return } // sort URLs newest to oldest var sortedUrls = urls.sorted { url1, url2 in - guard let date1 = try? url1.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate, - let date2 = try? url2.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate + guard + let date1 = try? url1.resourceValues(forKeys: [.contentModificationDateKey]) + .contentModificationDate, + let date2 = try? url2.resourceValues(forKeys: [.contentModificationDateKey]) + .contentModificationDate else { return false } @@ -208,7 +218,10 @@ import os let now = Date() var itemsWithoutDates: [URL] = [] while let url = sortedUrls.popLast() { - guard let date = try? url.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate else { + guard + let date = try? url.resourceValues(forKeys: [.contentModificationDateKey]) + .contentModificationDate + else { // Keep items without valid dates for cost-based purge itemsWithoutDates.append(url) continue @@ -249,7 +262,9 @@ import os guard let baseURL = urlProvider.cacheURL else { return } - if let urls = try? urlProvider.fileManager.contentsOfDirectory(at: baseURL, includingPropertiesForKeys: nil) { + if let urls = try? urlProvider.fileManager.contentsOfDirectory( + at: baseURL, includingPropertiesForKeys: nil + ) { for url in urls { deleteItem(at: url) } @@ -280,7 +295,11 @@ import os return 0 } - guard let urls = try? urlProvider.fileManager.contentsOfDirectory(at: baseURL, includingPropertiesForKeys: [.fileSizeKey], options: .skipsHiddenFiles) else { + guard + let urls = try? urlProvider.fileManager.contentsOfDirectory( + at: baseURL, includingPropertiesForKeys: [.fileSizeKey], options: .skipsHiddenFiles + ) + else { return 0 } @@ -329,15 +348,21 @@ class ArsenalURLProvider { /// Creates the directory if it doesn't exist. Returns `nil` if the /// Caches directory cannot be accessed. var cacheURL: URL? { - if let baseURL = fileManager.urls(for: .cachesDirectory, in: .userDomainMask).first?.appending(component: sanitizedIdentifier) { + if let baseURL = fileManager.urls(for: .cachesDirectory, in: .userDomainMask).first?.appending( + component: sanitizedIdentifier + ) { try? fileManager.createDirectory(at: baseURL, withIntermediateDirectories: true) return baseURL } return nil } - private lazy var sanitizedIdentifier: String = sanitize(prefix.isEmpty ? identifier : prefix + "." + identifier) - private lazy var allowedCharacterSet: CharacterSet = .alphanumerics.union(CharacterSet(charactersIn: "._-")) + private lazy var sanitizedIdentifier: String = sanitize( + prefix.isEmpty ? identifier : prefix + "." + identifier + ) + private lazy var allowedCharacterSet: CharacterSet = .alphanumerics.union( + CharacterSet(charactersIn: "._-") + ) /// Sanitizes a string to be a valid file name. /// diff --git a/Sources/Arsenal/Implementations/MemoryArsenal.swift b/Sources/Arsenal/Implementations/MemoryArsenal.swift index bfb6be5..7f45776 100644 --- a/Sources/Arsenal/Implementations/MemoryArsenal.swift +++ b/Sources/Arsenal/Implementations/MemoryArsenal.swift @@ -4,6 +4,7 @@ // Copyright (c) 2024 Alexander Cohen. All rights reserved. // +import Darwin import Foundation import os @@ -69,15 +70,16 @@ import os public func set(_ value: T?, key: String) { ArsenalActor.assertIsolated() - if let img = cache[key] { - cost -= img.cost - cache[key] = nil - } + // Get old cost if exists (single lookup) + let oldCost = cache[key]?.cost ?? 0 if let img = value { let item = MemoryItem(key: key, value: img) cache[key] = item - cost += item.cost + cost = cost - oldCost + item.cost + } else { + cache[key] = nil + cost -= oldCost } purge() } @@ -94,7 +96,6 @@ import os guard var item = cache[key] else { return nil } - // update the last accessed date to ensure purges are correctly ordered item.updateTimestamp() cache[key] = item return item.value @@ -127,7 +128,9 @@ import os // then we recreate the cache with what // is really owned. - logger.debug("Purge unowned: trying to purge \(self.cache.count) items using \(self.cost) in cost") + logger.debug( + "Purge unowned: trying to purge \(self.cache.count) items using \(self.cost) in cost" + ) let weakItems = cache.values.map { $0.weakify() } cache.removeAll() @@ -152,17 +155,10 @@ import os return } - // least recently accessed first (LRU) - var sorted = cache.values.sorted { item1, item2 in - item1.timestamp.compare(item2.timestamp) == .orderedAscending - } - - while !sorted.isEmpty, cost > costLimit { - guard let item = sorted.first else { - break - } + // Sort by timestamp descending (most recent first), so popLast() gives oldest + var sorted = cache.values.sorted { $0.timestamp > $1.timestamp } - sorted.remove(at: 0) + while cost > costLimit, let item = sorted.popLast() { cache[item.key] = nil cost -= item.cost } @@ -182,15 +178,16 @@ import os let key: String let value: T let cost: UInt64 - var timestamp: Date = .init() + var timestamp: UInt64 init(key: String, value: T) { self.key = key self.value = value cost = value.cost + timestamp = clock_gettime_nsec_np(CLOCK_UPTIME_RAW) } - fileprivate init(key: String, value: T, cost: UInt64, timestamp: Date) { + init(key: String, value: T, cost: UInt64, timestamp: UInt64) { self.key = key self.value = value self.cost = cost @@ -198,7 +195,7 @@ import os } mutating func updateTimestamp() { - timestamp = Date() + timestamp = clock_gettime_nsec_np(CLOCK_UPTIME_RAW) } func weakify() -> MemoryWeakItem { @@ -210,9 +207,9 @@ import os let key: String weak var value: T? let cost: UInt64 - var timestamp: Date = .init() + let timestamp: UInt64 - init(key: String, value: T, cost: UInt64, timestamp: Date) { + init(key: String, value: T, cost: UInt64, timestamp: UInt64) { self.key = key self.value = value self.cost = cost diff --git a/Sources/Arsenal/Implementations/SwiftDataArsenal.swift b/Sources/Arsenal/Implementations/SwiftDataArsenal.swift index 62be3fc..f56bd4c 100644 --- a/Sources/Arsenal/Implementations/SwiftDataArsenal.swift +++ b/Sources/Arsenal/Implementations/SwiftDataArsenal.swift @@ -135,9 +135,11 @@ import Foundation let item = ArsenalItemModel(key: key, value: val, cost: val.cost) modelContext?.insert(item) } else { - let fetchDescriptor = FetchDescriptor(predicate: #Predicate { item in - item.key == key - }) + let fetchDescriptor = FetchDescriptor( + predicate: #Predicate { item in + item.key == key + } + ) do { if let modelContext, let item = try modelContext.fetch(fetchDescriptor).first { modelContext.delete(item) @@ -155,9 +157,11 @@ import Foundation public func value(for key: String) -> T? { ArsenalActor.assertIsolated() - let fetchDescriptor = FetchDescriptor(predicate: #Predicate { item in - item.key == key - }) + let fetchDescriptor = FetchDescriptor( + predicate: #Predicate { item in + item.key == key + } + ) return try? modelContext?.fetch(fetchDescriptor).first?.value } diff --git a/Tests/ArsenalTests/ArsenalBenchmarks.swift b/Tests/ArsenalTests/ArsenalBenchmarks.swift index 102edae..c083129 100644 --- a/Tests/ArsenalTests/ArsenalBenchmarks.swift +++ b/Tests/ArsenalTests/ArsenalBenchmarks.swift @@ -1,6 +1,7 @@ -@testable import Arsenal import XCTest +@testable import Arsenal + /// Performance benchmarks for Arsenal caching operations. /// /// Run with: `swift test --filter Benchmark` @@ -9,9 +10,12 @@ class ArsenalBenchmarks: XCTestCase { // MARK: - Memory Cache Benchmarks func testBenchmarkMemorySet() async throws { - let cache = await Arsenal("benchMemorySet", resources: [ - .memory: MemoryArsenal(costLimit: 0), // No limit for benchmark - ]) + let cache = await Arsenal( + "benchMemorySet", + resources: [ + .memory: MemoryArsenal(costLimit: 0), // No limit for benchmark + ] + ) let items = (0 ..< 1000).map { i in BenchmarkItem(data: Data(repeating: UInt8(i % 256), count: 1024), cost: 1024) @@ -32,9 +36,12 @@ class ArsenalBenchmarks: XCTestCase { } func testBenchmarkMemoryGet() async throws { - let cache = await Arsenal("benchMemoryGet", resources: [ - .memory: MemoryArsenal(costLimit: 0), - ]) + let cache = await Arsenal( + "benchMemoryGet", + resources: [ + .memory: MemoryArsenal(costLimit: 0), + ] + ) // Pre-populate cache for i in 0 ..< 1000 { @@ -58,9 +65,12 @@ class ArsenalBenchmarks: XCTestCase { func testBenchmarkMemorySetWithPurge() async throws { // Cache with limit that will trigger purges - let cache = await Arsenal("benchMemoryPurge", resources: [ - .memory: MemoryArsenal(costLimit: 100 * 1024), // 100 KB limit - ]) + let cache = await Arsenal( + "benchMemoryPurge", + resources: [ + .memory: MemoryArsenal(costLimit: 100 * 1024), // 100 KB limit + ] + ) let items = (0 ..< 500).map { i in BenchmarkItem(data: Data(repeating: UInt8(i % 256), count: 1024), cost: 1024) @@ -83,9 +93,12 @@ class ArsenalBenchmarks: XCTestCase { // MARK: - Disk Cache Benchmarks func testBenchmarkDiskSet() async throws { - let cache = await Arsenal("benchDiskSet", resources: [ - .disk: DiskArsenal("benchDiskSet", maxStaleness: 0, costLimit: 0), - ]) + let cache = await Arsenal( + "benchDiskSet", + resources: [ + .disk: DiskArsenal("benchDiskSet", maxStaleness: 0, costLimit: 0), + ] + ) // Clear any existing data await cache.clear([.disk]) @@ -109,9 +122,12 @@ class ArsenalBenchmarks: XCTestCase { } func testBenchmarkDiskGet() async throws { - let cache = await Arsenal("benchDiskGet", resources: [ - .disk: DiskArsenal("benchDiskGet", maxStaleness: 0, costLimit: 0), - ]) + let cache = await Arsenal( + "benchDiskGet", + resources: [ + .disk: DiskArsenal("benchDiskGet", maxStaleness: 0, costLimit: 0), + ] + ) // Pre-populate cache for i in 0 ..< 100 { @@ -193,13 +209,18 @@ class ArsenalBenchmarks: XCTestCase { // MARK: - Large Item Benchmarks func testBenchmarkLargeItemMemory() async throws { - let cache = await Arsenal("benchLargeMemory", resources: [ - .memory: MemoryArsenal(costLimit: 0), - ]) + let cache = await Arsenal( + "benchLargeMemory", + resources: [ + .memory: MemoryArsenal(costLimit: 0), + ] + ) // 1 MB items let items = (0 ..< 50).map { i in - BenchmarkItem(data: Data(repeating: UInt8(i % 256), count: 1024 * 1024), cost: UInt64(1024 * 1024)) + BenchmarkItem( + data: Data(repeating: UInt8(i % 256), count: 1024 * 1024), cost: UInt64(1024 * 1024) + ) } measure { @@ -217,15 +238,20 @@ class ArsenalBenchmarks: XCTestCase { } func testBenchmarkLargeItemDisk() async throws { - let cache = await Arsenal("benchLargeDisk", resources: [ - .disk: DiskArsenal("benchLargeDisk", maxStaleness: 0, costLimit: 0), - ]) + let cache = await Arsenal( + "benchLargeDisk", + resources: [ + .disk: DiskArsenal("benchLargeDisk", maxStaleness: 0, costLimit: 0), + ] + ) await cache.clear([.disk]) // 1 MB items let items = (0 ..< 20).map { i in - BenchmarkItem(data: Data(repeating: UInt8(i % 256), count: 1024 * 1024), cost: UInt64(1024 * 1024)) + BenchmarkItem( + data: Data(repeating: UInt8(i % 256), count: 1024 * 1024), cost: UInt64(1024 * 1024) + ) } measure { @@ -245,9 +271,12 @@ class ArsenalBenchmarks: XCTestCase { // MARK: - Throughput Benchmarks func testBenchmarkMemoryThroughput() async throws { - let cache = await Arsenal("benchThroughput", resources: [ - .memory: MemoryArsenal(costLimit: 0), - ]) + let cache = await Arsenal( + "benchThroughput", + resources: [ + .memory: MemoryArsenal(costLimit: 0), + ] + ) let item = BenchmarkItem(data: Data(repeating: 0, count: 512), cost: 512) @@ -273,9 +302,12 @@ class ArsenalBenchmarks: XCTestCase { // MARK: - Clear Benchmarks func testBenchmarkClearMemory() async throws { - let cache = await Arsenal("benchClearMemory", resources: [ - .memory: MemoryArsenal(costLimit: 0), - ]) + let cache = await Arsenal( + "benchClearMemory", + resources: [ + .memory: MemoryArsenal(costLimit: 0), + ] + ) measure { let expectation = self.expectation(description: "Clear memory") @@ -294,9 +326,12 @@ class ArsenalBenchmarks: XCTestCase { } func testBenchmarkClearDisk() async throws { - let cache = await Arsenal("benchClearDisk", resources: [ - .disk: DiskArsenal("benchClearDisk", maxStaleness: 0, costLimit: 0), - ]) + let cache = await Arsenal( + "benchClearDisk", + resources: [ + .disk: DiskArsenal("benchClearDisk", maxStaleness: 0, costLimit: 0), + ] + ) measure { let expectation = self.expectation(description: "Clear disk") diff --git a/Tests/ArsenalTests/ArsenalTests.swift b/Tests/ArsenalTests/ArsenalTests.swift index abe80de..dd19cad 100644 --- a/Tests/ArsenalTests/ArsenalTests.swift +++ b/Tests/ArsenalTests/ArsenalTests.swift @@ -1,6 +1,7 @@ -@testable import Arsenal import XCTest +@testable import Arsenal + @available(iOS 17.0, macOS 14.0, macCatalyst 17.0, watchOS 10.0, visionOS 1.0, *) class ArsenalTests: XCTestCase { var memoryCache: Arsenal! @@ -9,8 +10,12 @@ class ArsenalTests: XCTestCase { override func setUp() async throws { try await super.setUp() - await memoryCache = Arsenal("testMemory", resources: [.memory: MemoryArsenal(costLimit: 1024 * 500)]) - await diskCache = Arsenal("testDisk", resources: [.disk: DiskArsenal("testDisk", maxStaleness: 2)]) + await memoryCache = Arsenal( + "testMemory", resources: [.memory: MemoryArsenal(costLimit: 1024 * 500)] + ) + await diskCache = Arsenal( + "testDisk", resources: [.disk: DiskArsenal("testDisk", maxStaleness: 2)] + ) await combinedCache = Arsenal("testCombined", costLimit: 1024 * 100, maxStaleness: 86400) } @@ -31,13 +36,17 @@ class ArsenalTests: XCTestCase { let retrievedItem = await memoryCache.value(for: key) XCTAssertNotNil(retrievedItem, "Item should be retrievable from memory cache.") - XCTAssertEqual(retrievedItem?.toData(), item.toData(), "Retrieved item data should match the original.") + XCTAssertEqual( + retrievedItem?.toData(), item.toData(), "Retrieved item data should match the original." + ) await diskCache.set(item, key: key) let diskItem = await diskCache.value(for: key) XCTAssertNotNil(diskItem, "Item should be retrievable from disk cache.") - XCTAssertEqual(diskItem?.toData(), item.toData(), "Retrieved item data from disk should match the original.") + XCTAssertEqual( + diskItem?.toData(), item.toData(), "Retrieved item data from disk should match the original." + ) } func testRemoveItem() async { @@ -100,9 +109,12 @@ class ArsenalTests: XCTestCase { func testLRUOrdering() async { // Create a small cache that can hold 2 items (costLimit 2500, each item 1000) - let smallCache = await Arsenal("testLRU", resources: [ - .memory: MemoryArsenal(costLimit: 2500), - ]) + let smallCache = await Arsenal( + "testLRU", + resources: [ + .memory: MemoryArsenal(costLimit: 2500), + ] + ) // Add 2 items (each 1000 cost, total 2000 = under limit) let item0 = TestItem(data: Data(repeating: 0, count: 100), cost: 1000) @@ -162,9 +174,12 @@ class ArsenalTests: XCTestCase { func testDiskCostBasedPurge() async { // Create disk cache with cost limit - let costLimitedDisk = await Arsenal("testDiskCost", resources: [ - .disk: DiskArsenal("testDiskCost", maxStaleness: 0, costLimit: 2000), - ]) + let costLimitedDisk = await Arsenal( + "testDiskCost", + resources: [ + .disk: DiskArsenal("testDiskCost", maxStaleness: 0, costLimit: 2000), + ] + ) // Add items exceeding cost limit for i in 0 ..< 5 { @@ -347,10 +362,15 @@ class ArsenalTests: XCTestCase { } let finalCost = await diskCache.diskResourceCost - XCTAssertEqual(finalCost, 100, "Cost should only reflect the single item, not accumulated from overwrites.") + XCTAssertEqual( + finalCost, 100, "Cost should only reflect the single item, not accumulated from overwrites." + ) let retrieved = await diskCache.value(for: key) - XCTAssertEqual(retrieved?.toData(), Data(repeating: 5, count: 100), "Retrieved item should be the last written value.") + XCTAssertEqual( + retrieved?.toData(), Data(repeating: 5, count: 100), + "Retrieved item should be the last written value." + ) } func testEmptyCachePurge() async { From 70c2551ab51479da067cc0bc2eb3121ecd6cc73a Mon Sep 17 00:00:00 2001 From: Alexander Cohen Date: Sun, 30 Nov 2025 12:12:57 -0500 Subject: [PATCH 2/2] Add performance optimizations and purgeUnowned test - Run memory and disk operations concurrently in forEachResource using async let - Remove unnecessary async from cost() and costLimit() methods - Optimize purgeUnowned() to use single-pass instead of multiple array iterations - Add test for purgeUnowned behavior --- Sources/Arsenal/Arsenal.swift | 21 +++++++++---- .../Implementations/MemoryArsenal.swift | 22 ++++++++------ Tests/ArsenalTests/ArsenalTests.swift | 30 +++++++++++++++++++ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/Sources/Arsenal/Arsenal.swift b/Sources/Arsenal/Arsenal.swift index 54dcf56..e0c1c0a 100644 --- a/Sources/Arsenal/Arsenal.swift +++ b/Sources/Arsenal/Arsenal.swift @@ -312,7 +312,7 @@ public protocol ArsenalItem: AnyObject, Sendable { /// /// - Parameter types: The resource types to include. Defaults to both memory and disk. /// - Returns: The sum of costs across all specified resources. - public func cost(_ types: Set = [.memory, .disk]) async -> UInt64 { + public func cost(_ types: Set = [.memory, .disk]) -> UInt64 { types.reduce(into: 0) { $0 += resources[$1]?.cost ?? 0 } @@ -322,7 +322,7 @@ public protocol ArsenalItem: AnyObject, Sendable { /// /// - Parameter types: The resource types to include. Defaults to both memory and disk. /// - Returns: The sum of cost limits across all specified resources. - public func costLimit(_ types: Set = [.memory, .disk]) async -> UInt64 { + public func costLimit(_ types: Set = [.memory, .disk]) -> UInt64 { types.reduce(into: 0) { $0 += resources[$1]?.costLimit ?? 0 } @@ -337,13 +337,22 @@ public protocol ArsenalItem: AnyObject, Sendable { } } + // Note: Using explicit async let instead of TaskGroup because Swift 6's + // region-based isolation checker doesn't support TaskGroup in this actor context. private func forEachResource( of types: Set, action: @Sendable @escaping (any ArsenalImp) async -> Void ) async { - for type in types { - if let res = resources[type] { - await action(res) - } + let memRes = types.contains(.memory) ? memoryResource : nil + let diskRes = types.contains(.disk) ? diskResource : nil + + if let memRes, let diskRes { + async let memTask: Void = action(memRes) + async let diskTask: Void = action(diskRes) + _ = await (memTask, diskTask) + } else if let memRes { + await action(memRes) + } else if let diskRes { + await action(diskRes) } } diff --git a/Sources/Arsenal/Implementations/MemoryArsenal.swift b/Sources/Arsenal/Implementations/MemoryArsenal.swift index 7f45776..f941c96 100644 --- a/Sources/Arsenal/Implementations/MemoryArsenal.swift +++ b/Sources/Arsenal/Implementations/MemoryArsenal.swift @@ -122,11 +122,9 @@ import os public func purgeUnowned() { ArsenalActor.assertIsolated() - // this make all items weak - // by doing so, all items that aren't - // referenced anywhere will be nilled out - // then we recreate the cache with what - // is really owned. + // Convert items to weak references - items not referenced + // elsewhere will be deallocated when we clear the cache. + // Then rebuild with only the surviving items. logger.debug( "Purge unowned: trying to purge \(self.cache.count) items using \(self.cost) in cost" @@ -134,10 +132,16 @@ import os let weakItems = cache.values.map { $0.weakify() } cache.removeAll() - cost = 0 - let strongItems = weakItems.compactMap { $0.strongify() } - cost = strongItems.reduce(0) { $0 + $1.cost } - strongItems.forEach { cache[$0.key] = $0 } + + // Single pass: filter valid items, rebuild cache, and sum cost + var newCost: UInt64 = 0 + for weakItem in weakItems { + if let strongItem = weakItem.strongify() { + cache[strongItem.key] = strongItem + newCost += strongItem.cost + } + } + cost = newCost logger.debug("After purge we have \(self.cache.count) items using \(self.cost) in cost") } diff --git a/Tests/ArsenalTests/ArsenalTests.swift b/Tests/ArsenalTests/ArsenalTests.swift index dd19cad..2086a03 100644 --- a/Tests/ArsenalTests/ArsenalTests.swift +++ b/Tests/ArsenalTests/ArsenalTests.swift @@ -383,6 +383,36 @@ class ArsenalTests: XCTestCase { XCTAssertEqual(memoryCost, 0, "Empty cache should have zero cost.") XCTAssertEqual(diskCost, 0, "Empty disk cache should have zero cost.") } + + func testPurgeUnownedReleasesUnreferencedItems() async { + // Keep a strong reference to this item + let retainedItem = TestItem(data: Data(repeating: 1, count: 100), cost: 1000) + await memoryCache.set(retainedItem, key: "retained") + + // Add item without keeping a reference (only cache holds it) + await memoryCache.set( + TestItem(data: Data(repeating: 2, count: 100), cost: 1000), + key: "unretained" + ) + + let costBefore = await memoryCache.memoryResourceCost + XCTAssertEqual(costBefore, 2000, "Should have 2 items before purge.") + + // Purge items not referenced outside the cache + await memoryCache.purgeUnowned([.memory]) + + let costAfter = await memoryCache.memoryResourceCost + XCTAssertEqual(costAfter, 1000, "Should have 1 item after purging unowned.") + + // Retained item should still exist + let retrieved = await memoryCache.value(for: "retained") + XCTAssertNotNil(retrieved, "Retained item should survive purgeUnowned.") + XCTAssertEqual(retrieved?.toData(), retainedItem.toData(), "Retained item data should match.") + + // Unretained item should be gone + let unretained = await memoryCache.value(for: "unretained") + XCTAssertNil(unretained, "Unretained item should be purged.") + } } // MARK: - Test Item