Wire together LRU GC

Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h

@@ -16,14 +16,15 @@
 
 #import <XCTest/XCTest.h>
 
+#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
+
 @protocol FSTPersistence;
-struct FSTLruGcParams;
 
 NS_ASSUME_NONNULL_BEGIN
 
 @interface FSTLRUGarbageCollectorTests : XCTestCase
 
-- (id<FSTPersistence>)newPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams;
+- (id<FSTPersistence>)newPersistenceWithLruParams:(firebase::firestore::local::LruParams)lruParams;
 
 @end
 

Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm

@@ -40,6 +40,8 @@
 
 namespace testutil = firebase::firestore::testutil;
 using firebase::firestore::auth::User;
+using firebase::firestore::local::LruParams;
+using firebase::firestore::local::LruResults;
 using firebase::firestore::model::DocumentKey;
 using firebase::firestore::model::DocumentKeyHash;
 using firebase::firestore::model::DocumentKeySet;
@@ -82,9 +84,9 @@ - (FSTLRUGarbageCollector *)garbageCollectorFromPersistence:(id<FSTPersistence>)
   return ((id<FSTLRUDelegate>)persistence.referenceDelegate).gc;
 }
 
-- (void)newTestResourcesWithLruGcParams:(FSTLruGcParams)lruGcParams {
+- (void)newTestResourcesWithLruParams:(LruParams)lruParams {
   HARD_ASSERT(_persistence == nil, "Persistence already created");
-  _persistence = [self newPersistenceWithLruGcParams:lruGcParams];
+  _persistence = [self newPersistenceWithLruParams:lruParams];
   _queryCache = [_persistence queryCache];
   _documentCache = [_persistence remoteDocumentCache];
   _mutationQueue = [_persistence mutationQueueForUser:_user];
@@ -96,10 +98,10 @@ - (void)newTestResourcesWithLruGcParams:(FSTLruGcParams)lruGcParams {
 }
 
 - (void)newTestResources {
-  [self newTestResourcesWithLruGcParams:FSTLruGcParams::Default()];
+  [self newTestResourcesWithLruParams:LruParams::Default()];
 }
 
-- (id<FSTPersistence>)newPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams {
+- (id<FSTPersistence>)newPersistenceWithLruParams:(LruParams)lruParams {
   @throw FSTAbstractMethodException();  // NOLINT
 }
 
@@ -673,8 +675,8 @@ - (void)testGetsSize {
 - (void)testDisabled {
   if ([self isTestBaseClass]) return;
 
-  FSTLruGcParams params = FSTLruGcParams::Disabled();
-  [self newTestResourcesWithLruGcParams:params];
+  LruParams params = LruParams::Disabled();
+  [self newTestResourcesWithLruParams:params];
 
   _persistence.run("fill cache", [&]() {
     // Simulate a bunch of ack'd mutations
@@ -684,7 +686,7 @@ - (void)testDisabled {
     }
   });
 
-  FSTLruGcResults results = [_gc tryRunGcWithLiveTargets:@{}];
+  LruResults results = [_gc collectWithLiveTargets:@{}];
   XCTAssertFalse(results.didRun);
 
   [_persistence shutdown];
@@ -693,8 +695,8 @@ - (void)testDisabled {
 - (void)testCacheTooSmall {
   if ([self isTestBaseClass]) return;
 
-  FSTLruGcParams params = FSTLruGcParams::Disabled();
-  [self newTestResourcesWithLruGcParams:params];
+  LruParams params = LruParams::Disabled();
+  [self newTestResourcesWithLruParams:params];
 
   _persistence.run("fill cache", [&]() {
     // Simulate a bunch of ack'd mutations
@@ -709,7 +711,7 @@ - (void)testCacheTooSmall {
   XCTAssertLessThan(cacheSize, params.minBytesThreshold);
 
   // Try collection and verify that it didn't run
-  FSTLruGcResults results = [_gc tryRunGcWithLiveTargets:@{}];
+  LruResults results = [_gc collectWithLiveTargets:@{}];
   XCTAssertFalse(results.didRun);
 
   [_persistence shutdown];
@@ -718,10 +720,10 @@ - (void)testCacheTooSmall {
 - (void)testGCRan {
   if ([self isTestBaseClass]) return;
 
-  FSTLruGcParams params = FSTLruGcParams::Default();
+  LruParams params = LruParams::Default();
   // Set a low threshold so we will definitely run
   params.minBytesThreshold = 100;
-  [self newTestResourcesWithLruGcParams:params];
+  [self newTestResourcesWithLruParams:params];
 
   // Add 100 targets and 10 documents to each
   for (int i = 0; i < 100; i++) {
@@ -737,8 +739,8 @@ - (void)testGCRan {
   }
 
   // Mark nothing as live, so everything is eligible.
-  FSTLruGcResults results = _persistence.run(
-      "GC", [&]() -> FSTLruGcResults { return [_gc tryRunGcWithLiveTargets:@{}]; });
+  LruResults results =
+      _persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:@{}]; });
 
   // 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.

Firestore/Example/Tests/Local/FSTLevelDBLRUGarbageCollectorTests.mm

@@ -16,17 +16,20 @@
 
 #import "Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h"
 #import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
+#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #import "Firestore/Source/Local/FSTLevelDB.h"
 
+using firebase::firestore::local::LruParams;
+
 NS_ASSUME_NONNULL_BEGIN
 
 @interface FSTLevelDBLRUGarbageCollectorTests : FSTLRUGarbageCollectorTests
 @end
 
 @implementation FSTLevelDBLRUGarbageCollectorTests
 
-- (id<FSTPersistence>)newPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams {
-  return [FSTPersistenceTestHelpers levelDBPersistenceWithLruGcParams:lruGcParams];
+- (id<FSTPersistence>)newPersistenceWithLruParams:(LruParams)lruParams {
+  return [FSTPersistenceTestHelpers levelDBPersistenceWithLruParams:lruParams];
 }
 
 @end

Firestore/Example/Tests/Local/FSTMemoryLRUGarbageCollectorTests.mm

@@ -17,17 +17,20 @@
 #import "Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h"
 
 #import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
+#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #import "Firestore/Source/Local/FSTMemoryPersistence.h"
 
+using firebase::firestore::local::LruParams;
+
 NS_ASSUME_NONNULL_BEGIN
 
 @interface FSTMemoryLRUGarbageCollectionTests : FSTLRUGarbageCollectorTests
 @end
 
 @implementation FSTMemoryLRUGarbageCollectionTests
 
-- (id<FSTPersistence>)newPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams {
-  return [FSTPersistenceTestHelpers lruMemoryPersistenceWithLruGcParams:lruGcParams];
+- (id<FSTPersistence>)newPersistenceWithLruParams:(LruParams)lruParams {
+  return [FSTPersistenceTestHelpers lruMemoryPersistenceWithLruParams:lruParams];
 }
 
 @end

Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h

@@ -16,11 +16,11 @@
 
 #import <Foundation/Foundation.h>
 
+#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #include "Firestore/core/src/firebase/firestore/util/path.h"
 
 @class FSTLevelDB;
 @class FSTMemoryPersistence;
-struct FSTLruGcParams;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -55,14 +55,15 @@ NS_ASSUME_NONNULL_BEGIN
  *
  * Sets up the LRU garbage collection to use the provided params.
  */
-+ (FSTLevelDB *)levelDBPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams;
++ (FSTLevelDB *)levelDBPersistenceWithLruParams:(firebase::firestore::local::LruParams)lruParams;
 
 /** Creates and starts a new FSTMemoryPersistence instance for testing. */
 + (FSTMemoryPersistence *)eagerGCMemoryPersistence;
 
 + (FSTMemoryPersistence *)lruMemoryPersistence;
 
-+ (FSTMemoryPersistence *)lruMemoryPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams;
++ (FSTMemoryPersistence *)lruMemoryPersistenceWithLruParams:
+    (firebase::firestore::local::LruParams)lruParams;
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.mm

@@ -18,6 +18,7 @@
 
 #include <utility>
 
+#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
 #import "Firestore/Source/Local/FSTLevelDB.h"
 #import "Firestore/Source/Local/FSTLocalSerializer.h"
 #import "Firestore/Source/Local/FSTMemoryPersistence.h"
@@ -30,6 +31,7 @@
 #include "Firestore/core/src/firebase/firestore/util/string_apple.h"
 
 namespace util = firebase::firestore::util;
+using firebase::firestore::local::LruParams;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::util::Path;
 using firebase::firestore::util::Status;
@@ -61,14 +63,13 @@ + (Path)levelDBDir {
 }
 
 + (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir {
-  return [self levelDBPersistenceWithDir:dir lruGcParams:FSTLruGcParams::Default()];
+  return [self levelDBPersistenceWithDir:dir lruParams:LruParams::Default()];
 }
 
-+ (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir lruGcParams:(FSTLruGcParams)params {
++ (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir lruParams:(LruParams)params {
   FSTLocalSerializer *serializer = [self localSerializer];
-  FSTLevelDB *db = [[FSTLevelDB alloc] initWithDirectory:std::move(dir)
-                                              serializer:serializer
-                                             lruGcParams:params];
+  FSTLevelDB *db =
+      [[FSTLevelDB alloc] initWithDirectory:std::move(dir) serializer:serializer lruParams:params];
   Status status = [db start];
   if (!status.ok()) {
     [NSException raise:NSInternalInconsistencyException
@@ -78,8 +79,8 @@ + (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir lruGcParams:(FSTLruGcParams)
   return db;
 }
 
-+ (FSTLevelDB *)levelDBPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams {
-  return [self levelDBPersistenceWithDir:[self levelDBDir] lruGcParams:lruGcParams];
++ (FSTLevelDB *)levelDBPersistenceWithLruParams:(LruParams)lruParams {
+  return [self levelDBPersistenceWithDir:[self levelDBDir] lruParams:lruParams];
 }
 
 + (FSTLevelDB *)levelDBPersistence {
@@ -98,13 +99,13 @@ + (FSTMemoryPersistence *)eagerGCMemoryPersistence {
 }
 
 + (FSTMemoryPersistence *)lruMemoryPersistence {
-  return [self lruMemoryPersistenceWithLruGcParams:FSTLruGcParams::Default()];
+  return [self lruMemoryPersistenceWithLruParams:LruParams::Default()];
 }
 
-+ (FSTMemoryPersistence *)lruMemoryPersistenceWithLruGcParams:(FSTLruGcParams)lruGcParams {
++ (FSTMemoryPersistence *)lruMemoryPersistenceWithLruParams:(LruParams)lruParams {
   FSTLocalSerializer *serializer = [self localSerializer];
   FSTMemoryPersistence *persistence =
-      [FSTMemoryPersistence persistenceWithLruGcParams:lruGcParams serializer:serializer];
+      [FSTMemoryPersistence persistenceWithLruParams:lruParams serializer:serializer];
   Status status = [persistence start];
   if (!status.ok()) {
     [NSException raise:NSInternalInconsistencyException

Firestore/Source/Core/FSTFirestoreClient.mm

@@ -55,6 +55,7 @@
 using firebase::firestore::auth::CredentialsProvider;
 using firebase::firestore::auth::User;
 using firebase::firestore::core::DatabaseInfo;
+using firebase::firestore::local::LruParams;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::DocumentKeySet;
 using firebase::firestore::model::OnlineState;
@@ -191,7 +192,7 @@ - (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistenc
     // TODO(gsoltis): enable LRU GC once API is finalized
     FSTLevelDB *ldb = [[FSTLevelDB alloc] initWithDirectory:std::move(dir)
                                                  serializer:serializer
-                                                lruGcParams:FSTLruGcParams::Disabled()];
+                                                  lruParams:LruParams::Disabled()];
     _lruDelegate = ldb.referenceDelegate;
     _persistence = ldb;
     [self scheduleLruGarbageCollection];
@@ -242,16 +243,14 @@ - (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistenc
  */
 - (void)scheduleLruGarbageCollection {
   long delay = _gcHasRun ? _initialGcDelay : _regularGcDelay;
-  _lruCallback = [_workerDispatchQueue
-      dispatchAfterDelay:delay
-                 timerID:FSTLruGCTimer
-                   block:^{
-                     FSTLruGcResults results =
-                         [self->_localStore tryLruGarbageCollection:self->_lruDelegate.gc];
-                     self->_gcHasRun = YES;
-                     LOG_DEBUG(results.ToString().c_str());
-                     [self scheduleLruGarbageCollection];
-                   }];
+  _lruCallback =
+      [_workerDispatchQueue dispatchAfterDelay:delay
+                                       timerID:FSTTimerIDGarbageCollectionDelay
+                                         block:^{
+                                           [self->_localStore collectGarbage:self->_lruDelegate.gc];
+                                           self->_gcHasRun = YES;
+                                           [self scheduleLruGarbageCollection];
+                                         }];
 }
 
 - (void)credentialDidChangeWithUser:(const User &)user {

Firestore/Source/Local/FSTLRUGarbageCollector.h

@@ -29,22 +29,39 @@
 
 extern const firebase::firestore::model::ListenSequenceNumber kFSTListenSequenceNumberInvalid;
 
-struct FSTLruGcParams {
-  static FSTLruGcParams Default() {
-    return FSTLruGcParams{.minBytesThreshold = 100 * 1024 * 1024,
-                          .percentileToCollect = 10,
-                          .maximumSequenceNumbersToCollect = 1000};
+namespace firebase {
+namespace firestore {
+namespace local {
+
+struct LruParams {
+  static LruParams Default() {
+    return LruParams{100 * 1024 * 1024, 10, 1000};
   }
 
-  static FSTLruGcParams Disabled() {
-    return FSTLruGcParams{.minBytesThreshold = kFIRFirestorePersistenceCacheSizeUnlimited};
+  static LruParams Disabled() {
+    return LruParams{kFIRFirestorePersistenceCacheSizeUnlimited, 0, 0};
   }
 
   long minBytesThreshold;
   int percentileToCollect;
   int maximumSequenceNumbersToCollect;
 };
 
+struct LruResults {
+  static LruResults DidNotRun() {
+    return LruResults{NO, 0, 0, 0};
+  }
+
+  BOOL didRun;
+  int sequenceNumbersCollected;
+  int targetsRemoved;
+  int documentsRemoved;
+};
+
+}  // namespace local
+}  // namespace firestore
+}  // namespace firebase
+
 /**
  * Persistence layers intending to use LRU Garbage collection should implement this protocol. This
  * protocol defines the operations that the LRU garbage collector needs from the persistence layer.
@@ -96,38 +113,17 @@ struct FSTLruGcParams {
 
 @end
 
-struct FSTLruGcResults {
-  static FSTLruGcResults DidNotRun() {
-    return FSTLruGcResults{.didRun = NO};
-  }
-
-  std::string ToString() const;
-
-  long total_duration() const {
-    return targetCountDurationMs + upperBoundDurationMs + removedTargetsDurationMs +
-           removedDocumentsDurationMs + dbCompactionDurationMs;
-  }
-
-  BOOL didRun;
-
-  long targetCountDurationMs;
-  long upperBoundDurationMs;
-  long removedTargetsDurationMs;
-  long removedDocumentsDurationMs;
-  long dbCompactionDurationMs;
-
-  int sequenceNumbersCollected;
-  int targetsRemoved;
-  int documentsRemoved;
-};
-
 /**
  * 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:(FSTLruGcParams)params;
+- (instancetype)initWithDelegate:(id<FSTLRUDelegate>)delegate
+                          params:(firebase::firestore::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
@@ -159,6 +155,7 @@ struct FSTLruGcResults {
 
 - (size_t)byteSize;
 
-- (FSTLruGcResults)tryRunGcWithLiveTargets:(NSDictionary<NSNumber *, FSTQueryData *> *)liveTargets;
+- (firebase::firestore::local::LruResults)collectWithLiveTargets:
+    (NSDictionary<NSNumber *, FSTQueryData *> *)liveTargets;
 
 @end