diff --git a/bitchat/Services/MessageDeduplicationService.swift b/bitchat/Services/MessageDeduplicationService.swift index 693365ca..c41b942e 100644 --- a/bitchat/Services/MessageDeduplicationService.swift +++ b/bitchat/Services/MessageDeduplicationService.swift @@ -15,9 +15,21 @@ import Foundation /// Thread-safe via @MainActor - all callers are already on main actor. @MainActor final class LRUDeduplicationCache { - private var map: [String: Value] = [:] - private var order: [String] = [] + private struct StoredEntry { + var value: Value + let generation: UInt64 + } + + private struct OrderEntry { + let key: String + let generation: UInt64 + } + + private var map: [String: StoredEntry] = [:] + private var order: [OrderEntry] = [] private var head: Int = 0 + private var staleNodeCount: Int = 0 + private var nextGeneration: UInt64 = 0 private let capacity: Int /// Creates a new LRU cache with the specified capacity. @@ -29,9 +41,14 @@ final class LRUDeduplicationCache { /// Number of active entries in the cache var count: Int { - order.count - head + map.count } + #if DEBUG + /// Order-node storage exposed only for bounded-storage regression tests. + var _orderStorageCountForTesting: Int { order.count } + #endif + /// Checks if a key exists in the cache func contains(_ key: String) -> Bool { map[key] != nil @@ -39,22 +56,29 @@ final class LRUDeduplicationCache { /// Gets the value for a key, or nil if not present func value(for key: String) -> Value? { - map[key] + map[key]?.value } /// Records a key-value pair, updating if exists or inserting if new func record(_ key: String, value: Value) { - if map[key] == nil { - order.append(key) + if let existing = map[key] { + map[key] = StoredEntry(value: value, generation: existing.generation) + } else { + nextGeneration &+= 1 + let generation = nextGeneration + map[key] = StoredEntry(value: value, generation: generation) + order.append(OrderEntry(key: key, generation: generation)) } - map[key] = value + trimIfNeeded() + compactIfNeeded() } /// Removes a specific key from the cache func remove(_ key: String) { - map.removeValue(forKey: key) - // Note: key remains in order array but will be skipped during eviction + guard map.removeValue(forKey: key) != nil else { return } + staleNodeCount += 1 + compactIfNeeded() } /// Clears all entries from the cache @@ -62,39 +86,53 @@ final class LRUDeduplicationCache { map.removeAll() order.removeAll() head = 0 + staleNodeCount = 0 + nextGeneration = 0 } // MARK: - Private private func trimIfNeeded() { - let activeCount = order.count - head - guard activeCount > capacity else { return } - - let overflow = activeCount - capacity - for _ in 0.. capacity { + guard popOldest() else { break } } } - private func popOldest() -> String? { - // Skip keys that were already removed from map + private func popOldest() -> Bool { while head < order.count { - let key = order[head] + let candidate = order[head] head += 1 - // Periodically compact the backing storage - if head >= 32 && head * 2 >= order.count { - order.removeFirst(head) - head = 0 + guard let liveEntry = map[candidate.key], + liveEntry.generation == candidate.generation else { + if staleNodeCount > 0 { + staleNodeCount -= 1 + } + continue } - // Only return if key is still in map - if map[key] != nil { - return key - } + map.removeValue(forKey: candidate.key) + return true } - return nil + return false + } + + private func compactIfNeeded() { + let unconsumedCount = order.count - head + let shouldDropConsumedPrefix = head >= 32 && head * 2 >= order.count + let shouldRemoveStaleNodes = staleNodeCount >= 32 && staleNodeCount * 2 >= unconsumedCount + + guard shouldDropConsumedPrefix || shouldRemoveStaleNodes else { return } + + if shouldRemoveStaleNodes { + order = order[head...].filter { candidate in + map[candidate.key]?.generation == candidate.generation + } + staleNodeCount = 0 + } else { + order.removeFirst(head) + } + head = 0 } } diff --git a/bitchatTests/MessageDeduplicationServiceTests.swift b/bitchatTests/MessageDeduplicationServiceTests.swift index 971c17f0..3d5f0bc4 100644 --- a/bitchatTests/MessageDeduplicationServiceTests.swift +++ b/bitchatTests/MessageDeduplicationServiceTests.swift @@ -122,22 +122,73 @@ struct LRUDeduplicationCacheTests { #expect(cache.contains("b")) } - @Test func eviction_skipsRemovedKeys() { + @Test func removal_thenReplacementPreservesLiveCapacity() { let cache = LRUDeduplicationCache(capacity: 3) cache.record("a", value: 1) cache.record("b", value: 2) cache.record("c", value: 3) - // Remove "a" manually cache.remove("a") - - // Add new entry - should evict "b" (next oldest still in map) cache.record("d", value: 4) - // Cache should have b, c, d (a was removed) - // Actually after eviction it should have c, d and maybe b depending on implementation + #expect(cache.count == 3) #expect(!cache.contains("a")) - #expect(cache.count <= 3) + #expect(cache.contains("b")) + #expect(cache.contains("c")) + #expect(cache.contains("d")) + } + + @Test func removal_thenReinsertionKeepsNewGenerationLive() { + let cache = LRUDeduplicationCache(capacity: 3) + cache.record("a", value: 1) + cache.record("b", value: 2) + cache.record("c", value: 3) + + cache.remove("a") + cache.record("a", value: 4) + + #expect(cache.count == 3) + #expect(cache.value(for: "a") == 4) + #expect(cache.contains("b")) + #expect(cache.contains("c")) + } + + @Test func updatingLiveKeyDoesNotRefreshInsertionOrder() { + let cache = LRUDeduplicationCache(capacity: 2) + cache.record("a", value: 1) + cache.record("b", value: 2) + cache.record("a", value: 3) + + cache.record("c", value: 4) + + #expect(!cache.contains("a")) + #expect(cache.contains("b")) + #expect(cache.contains("c")) + } + + @Test func repeatedRemovalAndReinsertionCompactsStaleNodes() { + let cache = LRUDeduplicationCache(capacity: 3) + cache.record("a", value: 0) + + for value in 1...1_001 { + cache.remove("a") + cache.record("a", value: value) + } + + #expect(cache.count == 1) + #expect(cache.value(for: "a") == 1_001) + #if DEBUG + #expect(cache._orderStorageCountForTesting <= 32) + #endif + + cache.record("b", value: 1) + cache.record("c", value: 2) + cache.record("d", value: 3) + + #expect(!cache.contains("a")) + #expect(cache.contains("b")) + #expect(cache.contains("c")) + #expect(cache.contains("d")) } // MARK: - Edge Cases @@ -323,6 +374,24 @@ struct MessageDeduplicationServiceTests { #expect(service.contentTimestamp(for: content) == nil) } + @Test func forgetContent_thenRerecordKeepsReplacementDeduplicatedAtCapacity() { + let service = MessageDeduplicationService(contentCapacity: 3, nostrEventCapacity: 3) + let first = Date(timeIntervalSince1970: 1_000) + let replacement = Date(timeIntervalSince1970: 2_000) + service.recordContent("content a", timestamp: first) + service.recordContent("content b", timestamp: first) + service.recordContent("content c", timestamp: first) + + service.forgetContent("content a", ifRecordedAt: first) + service.recordContent("content a", timestamp: replacement) + service.recordContent("content d", timestamp: replacement) + + #expect(service.contentTimestamp(for: "content a") == replacement) + #expect(service.contentTimestamp(for: "content b") == nil) + #expect(service.contentTimestamp(for: "content c") == first) + #expect(service.contentTimestamp(for: "content d") == replacement) + } + @Test func forgetContent_doesNotEraseNewerSameContentMarker() { let service = MessageDeduplicationService(contentCapacity: 100, nostrEventCapacity: 100) let content = "repeated payload"