Migrate FSTLRUGarbageCollector to C++ LruGarbageCollector (#3685)

* Add ReferenceDelegate

* Remove Objective-C crutches from model/types.h and query_cache.h

* Add C++ LruGarbageCollector

* Add delegate bridges for LruDelegate and ReferenceDelegate

* Migrate from FSTLRUGarbageCollector to C++ LruGarbageCollector

Author: wilhuff

Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h

@@ -18,6 +18,16 @@
 
 #import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 
+namespace firebase {
+namespace firestore {
+namespace local {
+
+class LruParams;
+
+}  // namespace local
+}  // namespace firestore
+}  // namespace firebase
+
 @protocol FSTPersistence;
 
 NS_ASSUME_NONNULL_BEGIN

Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm

@@ -24,13 +24,13 @@
 #include <vector>
 
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
-#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #import "Firestore/Source/Local/FSTPersistence.h"
 #import "Firestore/Source/Util/FSTClasses.h"
 
 #include "Firestore/core/include/firebase/firestore/timestamp.h"
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
 #include "Firestore/core/src/firebase/firestore/core/query.h"
+#include "Firestore/core/src/firebase/firestore/local/lru_garbage_collector.h"
 #include "Firestore/core/src/firebase/firestore/local/mutation_queue.h"
 #include "Firestore/core/src/firebase/firestore/local/query_cache.h"
 #include "Firestore/core/src/firebase/firestore/local/query_data.h"
@@ -48,6 +48,7 @@
 namespace testutil = firebase::firestore::testutil;
 using firebase::Timestamp;
 using firebase::firestore::auth::User;
+using firebase::firestore::local::LruGarbageCollector;
 using firebase::firestore::local::LruParams;
 using firebase::firestore::local::LruResults;
 using firebase::firestore::local::MutationQueue;
@@ -84,7 +85,7 @@ @implementation FSTLRUGarbageCollectorTests {
   RemoteDocumentCache *_documentCache;
   MutationQueue *_mutationQueue;
   id<FSTLRUDelegate> _lruDelegate;
-  FSTLRUGarbageCollector *_gc;
+  LruGarbageCollector *_gc;
   ListenSequenceNumber _initialSequenceNumber;
   User _user;
   ReferenceSet _additionalReferences;
@@ -140,28 +141,26 @@ - (void)expectSentinelRemoved:(const DocumentKey &)key {
 
 - (ListenSequenceNumber)sequenceNumberForQueryCount:(int)queryCount {
   return _persistence.run(
-      "gc", [&]() -> ListenSequenceNumber { return [_gc sequenceNumberForQueryCount:queryCount]; });
+      "gc", [&]() -> ListenSequenceNumber { return _gc->SequenceNumberForQueryCount(queryCount); });
 }
 
 - (int)queryCountForPercentile:(int)percentile {
   return _persistence.run("query count",
-                          [&]() -> int { return [_gc queryCountForPercentile:percentile]; });
+                          [&]() -> int { return _gc->QueryCountForPercentile(percentile); });
 }
 
 - (int)removeQueriesThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
                               liveQueries:
                                   (const std::unordered_map<TargetId, QueryData> &)liveQueries {
-  return _persistence.run("gc", [&]() -> int {
-    return [_gc removeQueriesUpThroughSequenceNumber:sequenceNumber liveQueries:liveQueries];
-  });
+  return _persistence.run("gc",
+                          [&]() -> int { return _gc->RemoveTargets(sequenceNumber, liveQueries); });
 }
 
 // Removes documents that are not part of a target or a mutation and have a sequence number
 // less than or equal to the given sequence number.
 - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber {
-  return _persistence.run("gc", [&]() -> int {
-    return [_gc removeOrphanedDocumentsThroughSequenceNumber:sequenceNumber];
-  });
+  return _persistence.run("gc",
+                          [&]() -> int { return _gc->RemoveOrphanedDocuments(sequenceNumber); });
 }
 
 - (QueryData)nextTestQuery {
@@ -284,7 +283,7 @@ - (void)testSequenceNumberNoQueries {
 
   // No queries... should get invalid sequence number (-1)
   [self newTestResources];
-  XCTAssertEqual(kFSTListenSequenceNumberInvalid, [self sequenceNumberForQueryCount:0]);
+  XCTAssertEqual(local::kListenSequenceNumberInvalid, [self sequenceNumberForQueryCount:0]);
   [_persistence shutdown];
 }
 
@@ -672,7 +671,7 @@ - (void)testGetsSize {
 
   [self newTestResources];
 
-  size_t initialSize = [_gc byteSize];
+  size_t initialSize = _gc->CalculateByteSize();
 
   _persistence.run("fill cache", [&]() {
     // Simulate a bunch of ack'd mutations
@@ -682,7 +681,7 @@ - (void)testGetsSize {
     }
   });
 
-  size_t finalSize = [_gc byteSize];
+  size_t finalSize = _gc->CalculateByteSize();
   XCTAssertGreaterThan(finalSize, initialSize);
 
   [_persistence shutdown];
@@ -702,9 +701,8 @@ - (void)testDisabled {
     }
   });
 
-  LruResults results =
-      _persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:{}]; });
-  XCTAssertFalse(results.didRun);
+  LruResults results = _persistence.run("GC", [&]() -> LruResults { return _gc->Collect({}); });
+  XCTAssertFalse(results.did_run);
 
   [_persistence shutdown];
 }
@@ -723,14 +721,13 @@ - (void)testCacheTooSmall {
     }
   });
 
-  int cacheSize = (int)[_gc byteSize];
+  int cacheSize = (int)_gc->CalculateByteSize();
   // Verify that we don't have enough in our cache to warrant collection
-  XCTAssertLessThan(cacheSize, params.minBytesThreshold);
+  XCTAssertLessThan(cacheSize, params.min_bytes_threshold);
 
   // Try collection and verify that it didn't run
-  LruResults results =
-      _persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:{}]; });
-  XCTAssertFalse(results.didRun);
+  LruResults results = _persistence.run("GC", [&]() -> LruResults { return _gc->Collect({}); });
+  XCTAssertFalse(results.did_run);
 
   [_persistence shutdown];
 }
@@ -740,7 +737,7 @@ - (void)testGCRan {
 
   LruParams params = LruParams::Default();
   // Set a low threshold so we will definitely run
-  params.minBytesThreshold = 100;
+  params.min_bytes_threshold = 100;
   [self newTestResourcesWithLruParams:params];
 
   // Add 100 targets and 10 documents to each
@@ -757,14 +754,13 @@ - (void)testGCRan {
   }
 
   // Mark nothing as live, so everything is eligible.
-  LruResults results =
-      _persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:{}]; });
+  LruResults results = _persistence.run("GC", [&]() -> LruResults { return _gc->Collect({}); });
 
   // By default, we collect 10% of the sequence numbers. Since we added 100 targets,
   // that should be 10 targets with 10 documents each, for a total of 100 documents.
-  XCTAssertTrue(results.didRun);
-  XCTAssertEqual(10, results.targetsRemoved);
-  XCTAssertEqual(100, results.documentsRemoved);
+  XCTAssertTrue(results.did_run);
+  XCTAssertEqual(10, results.targets_removed);
+  XCTAssertEqual(100, results.documents_removed);
   [_persistence shutdown];
 }
 

Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h

@@ -16,12 +16,21 @@
 
 #import <Foundation/Foundation.h>
 
-#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #include "Firestore/core/src/firebase/firestore/util/path.h"
 
 @class FSTLevelDB;
 @class FSTMemoryPersistence;
 
+namespace firebase {
+namespace firestore {
+namespace local {
+
+class LruParams;
+
+}  // namespace local
+}  // namespace firestore
+}  // namespace firebase
+
 NS_ASSUME_NONNULL_BEGIN
 
 @interface FSTPersistenceTestHelpers : NSObject

Firestore/Example/Tests/Util/FSTHelpers.h

@@ -182,7 +182,7 @@ class TestTargetMetadataProvider : public TargetMetadataProvider {
       const std::vector<model::TargetId> &limbo_targets);
 
   /**
-   * Creates an `TestTargetMetadataProvider` that behaves as if there's an established listen for
+   * Creates a `TestTargetMetadataProvider` that behaves as if there's an established listen for
    * each of the given targets, where each target has not seen any previous document.
    *
    * Internally this means that the `GetRemoteKeysForTarget` callback for these targets will return

Firestore/Source/Local/FSTLRUGarbageCollector.h

@@ -25,53 +25,18 @@
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 #include "Firestore/core/src/firebase/firestore/model/types.h"
 
-@class FSTLRUGarbageCollector;
-
-namespace local = firebase::firestore::local;
-namespace model = firebase::firestore::model;
-
-extern const model::ListenSequenceNumber kFSTListenSequenceNumberInvalid;
-
 namespace firebase {
 namespace firestore {
 namespace local {
 
-struct LruParams {
-  static LruParams Default() {
-    return LruParams{100 * 1024 * 1024, 10, 1000};
-  }
-
-  static LruParams Disabled() {
-    return LruParams{api::Settings::CacheSizeUnlimited, 0, 0};
-  }
-
-  static LruParams WithCacheSize(int64_t cacheSize) {
-    LruParams params = Default();
-    params.minBytesThreshold = cacheSize;
-    return params;
-  }
-
-  int64_t minBytesThreshold;
-  int percentileToCollect;
-  int maximumSequenceNumbersToCollect;
-};
-
-struct LruResults {
-  static LruResults DidNotRun() {
-    return LruResults{/* didRun= */ false, 0, 0, 0};
-  }
-
-  bool didRun;
-  int sequenceNumbersCollected;
-  int targetsRemoved;
-  int documentsRemoved;
-};
+class LruGarbageCollector;
 
 }  // namespace local
 }  // namespace firestore
 }  // namespace firebase
 
 namespace local = firebase::firestore::local;
+namespace model = firebase::firestore::model;
 
 /**
  * Persistence layers intending to use LRU Garbage collection should implement this protocol. This
@@ -111,51 +76,6 @@ namespace local = firebase::firestore::local;
 - (size_t)sequenceNumberCount;
 
 /** Access to the underlying LRU Garbage collector instance. */
-@property(strong, nonatomic, readonly) FSTLRUGarbageCollector *gc;
-
-@end
-
-/**
- * FSTLRUGarbageCollector defines the LRU algorithm used to clean up old documents and targets. It
- * is persistence-agnostic, as long as proper delegate is provided.
- */
-@interface FSTLRUGarbageCollector : NSObject
-
-- (instancetype)initWithDelegate:(id<FSTLRUDelegate>)delegate
-                          params:(local::LruParams)params NS_DESIGNATED_INITIALIZER;
-
-- (instancetype)init NS_UNAVAILABLE;
-
-/**
- * Given a target percentile, return the number of queries that make up that percentage of the
- * queries that are cached. For instance, if 20 queries are cached, and the percentile is 40, the
- * result will be 8.
- */
-- (int)queryCountForPercentile:(NSUInteger)percentile;
-
-/**
- * Given a number of queries n, return the nth sequence number in the cache.
- */
-- (model::ListenSequenceNumber)sequenceNumberForQueryCount:(NSUInteger)queryCount;
-
-/**
- * Removes queries that are not currently live (as indicated by presence in the liveQueries map) and
- * have a sequence number less than or equal to the given sequence number.
- */
-- (int)removeQueriesUpThroughSequenceNumber:(model::ListenSequenceNumber)sequenceNumber
-                                liveQueries:
-                                    (const std::unordered_map<model::TargetId, local::QueryData> &)
-                                        liveQueries;
-
-/**
- * Removes all unreferenced documents from the cache that have a sequence number less than or equal
- * to the given sequence number. Returns the number of documents removed.
- */
-- (int)removeOrphanedDocumentsThroughSequenceNumber:(model::ListenSequenceNumber)sequenceNumber;
-
-- (size_t)byteSize;
-
-- (local::LruResults)collectWithLiveTargets:
-    (const std::unordered_map<model::TargetId, local::QueryData> &)liveTargets;
+- (local::LruGarbageCollector *)gc;
 
 @end