Wire up LRU GC (#1925)

* Wire up LRU GC

Author: Greg Soltis

Firestore/Source/API/FIRFirestore.mm

@@ -258,7 +258,7 @@ const DatabaseInfo database_info(*self.databaseID, util::MakeString(_persistence
           absl::make_unique<ExecutorLibdispatch>(_settings.dispatchQueue);
 
       _client = [FSTFirestoreClient clientWithDatabaseInfo:database_info
-                                            usePersistence:_settings.persistenceEnabled
+                                                  settings:_settings
                                        credentialsProvider:_credentialsProvider.get()
                                               userExecutor:std::move(userExecutor)
                                        workerDispatchQueue:_workerDispatchQueue];

Firestore/Source/API/FIRFirestoreSettings.mm

@@ -23,6 +23,8 @@
 static NSString *const kDefaultHost = @"firestore.googleapis.com";
 static const BOOL kDefaultSSLEnabled = YES;
 static const BOOL kDefaultPersistenceEnabled = YES;
+static const int64_t kDefaultCacheSizeBytes = 100 * 1024 * 1024;
+static const int64_t kMinimumCacheSizeBytes = 1 * 1024 * 1024;
 // TODO(b/73820332): flip the default.
 static const BOOL kDefaultTimestampsInSnapshotsEnabled = NO;
 
@@ -35,6 +37,7 @@ - (instancetype)init {
     _dispatchQueue = dispatch_get_main_queue();
     _persistenceEnabled = kDefaultPersistenceEnabled;
     _timestampsInSnapshotsEnabled = kDefaultTimestampsInSnapshotsEnabled;
+    _cacheSizeBytes = kDefaultCacheSizeBytes;
   }
   return self;
 }
@@ -51,7 +54,8 @@ - (BOOL)isEqual:(id)other {
          self.isSSLEnabled == otherSettings.isSSLEnabled &&
          self.dispatchQueue == otherSettings.dispatchQueue &&
          self.isPersistenceEnabled == otherSettings.isPersistenceEnabled &&
-         self.timestampsInSnapshotsEnabled == otherSettings.timestampsInSnapshotsEnabled;
+         self.timestampsInSnapshotsEnabled == otherSettings.timestampsInSnapshotsEnabled &&
+         self.cacheSizeBytes == otherSettings.cacheSizeBytes;
 }
 
 - (NSUInteger)hash {
@@ -60,6 +64,7 @@ - (NSUInteger)hash {
   // Ignore the dispatchQueue to avoid having to deal with sizeof(dispatch_queue_t).
   result = 31 * result + (self.isPersistenceEnabled ? 1231 : 1237);
   result = 31 * result + (self.timestampsInSnapshotsEnabled ? 1231 : 1237);
+  result = 31 * result + (NSUInteger)self.cacheSizeBytes;
   return result;
 }
 
@@ -70,6 +75,7 @@ - (id)copyWithZone:(nullable NSZone *)zone {
   copy.dispatchQueue = _dispatchQueue;
   copy.persistenceEnabled = _persistenceEnabled;
   copy.timestampsInSnapshotsEnabled = _timestampsInSnapshotsEnabled;
+  copy.cacheSizeBytes = _cacheSizeBytes;
   return copy;
 }
 
@@ -93,6 +99,14 @@ - (void)setDispatchQueue:(dispatch_queue_t)dispatchQueue {
   _dispatchQueue = dispatchQueue;
 }
 
+- (void)setCacheSizeBytes:(int64_t)cacheSizeBytes {
+  if (cacheSizeBytes != kFIRFirestoreCacheSizeUnlimited &&
+      cacheSizeBytes < kMinimumCacheSizeBytes) {
+    FSTThrowInvalidArgument(@"Cache size must be set to at least %i bytes", kMinimumCacheSizeBytes);
+  }
+  _cacheSizeBytes = cacheSizeBytes;
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Source/Core/FSTFirestoreClient.h

@@ -29,6 +29,7 @@
 
 @class FIRDocumentReference;
 @class FIRDocumentSnapshot;
+@class FIRFirestoreSettings;
 @class FIRQuery;
 @class FIRQuerySnapshot;
 @class FSTDatabaseID;
@@ -56,7 +57,7 @@ NS_ASSUME_NONNULL_BEGIN
  * All callbacks and events will be triggered on the provided userExecutor.
  */
 + (instancetype)clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo
-                        usePersistence:(BOOL)usePersistence
+                              settings:(FIRFirestoreSettings *)settings
                    credentialsProvider:(firebase::firestore::auth::CredentialsProvider *)
                                            credentialsProvider  // no passing ownership
                           userExecutor:

Firestore/Source/Core/FSTFirestoreClient.mm

@@ -21,6 +21,7 @@
 #include <utility>
 
 #import "FIRFirestoreErrors.h"
+#import "FIRFirestoreSettings.h"
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
 #import "Firestore/Source/API/FIRQuery+Internal.h"
@@ -75,7 +76,7 @@ @interface FSTFirestoreClient () {
 }
 
 - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
-                      usePersistence:(BOOL)usePersistence
+                            settings:(FIRFirestoreSettings *)settings
                  credentialsProvider:
                      (CredentialsProvider *)credentialsProvider  // no passing ownership
                         userExecutor:(std::unique_ptr<Executor>)userExecutor
@@ -115,20 +116,20 @@ - (Executor *)userExecutor {
 }
 
 + (instancetype)clientWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
-                        usePersistence:(BOOL)usePersistence
+                              settings:(FIRFirestoreSettings *)settings
                    credentialsProvider:
                        (CredentialsProvider *)credentialsProvider  // no passing ownership
                           userExecutor:(std::unique_ptr<Executor>)userExecutor
                    workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue {
   return [[FSTFirestoreClient alloc] initWithDatabaseInfo:databaseInfo
-                                           usePersistence:usePersistence
+                                                 settings:settings
                                       credentialsProvider:credentialsProvider
                                              userExecutor:std::move(userExecutor)
                                       workerDispatchQueue:workerDispatchQueue];
 }
 
 - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
-                      usePersistence:(BOOL)usePersistence
+                            settings:(FIRFirestoreSettings *)settings
                  credentialsProvider:
                      (CredentialsProvider *)credentialsProvider  // no passing ownership
                         userExecutor:(std::unique_ptr<Executor>)userExecutor
@@ -167,21 +168,21 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
     // before any subsequently queued work runs.
     [_workerDispatchQueue dispatchAsync:^{
       User user = userPromise->get_future().get();
-      [self initializeWithUser:user usePersistence:usePersistence];
+      [self initializeWithUser:user settings:settings];
     }];
   }
   return self;
 }
 
-- (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistence {
+- (void)initializeWithUser:(const User &)user settings:(FIRFirestoreSettings *)settings {
   // Do all of our initialization on our own dispatch queue.
   [self.workerDispatchQueue verifyIsCurrentQueue];
   LOG_DEBUG("Initializing. Current user: %s", user.uid());
 
   // Note: The initialization work must all be synchronous (we can't dispatch more work) since
   // external write/listen operations could get queued to run before that subsequent work
   // completes.
-  if (usePersistence) {
+  if (settings.isPersistenceEnabled) {
     Path dir = [FSTLevelDB storageDirectoryForDatabaseInfo:*self.databaseInfo
                                         documentsDirectory:[FSTLevelDB documentsDirectory]];
 
@@ -190,9 +191,10 @@ - (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistenc
     FSTLocalSerializer *serializer =
         [[FSTLocalSerializer alloc] initWithRemoteSerializer:remoteSerializer];
     // TODO(gsoltis): enable LRU GC once API is finalized
-    FSTLevelDB *ldb = [[FSTLevelDB alloc] initWithDirectory:std::move(dir)
-                                                 serializer:serializer
-                                                  lruParams:LruParams::Disabled()];
+    FSTLevelDB *ldb =
+        [[FSTLevelDB alloc] initWithDirectory:std::move(dir)
+                                   serializer:serializer
+                                    lruParams:LruParams::WithCacheSize(settings.cacheSizeBytes)];
     _lruDelegate = ldb.referenceDelegate;
     _persistence = ldb;
     [self scheduleLruGarbageCollection];

Firestore/Source/Local/FSTLRUGarbageCollector.h

@@ -44,6 +44,12 @@ struct LruParams {
     return LruParams{kFIRFirestoreCacheSizeUnlimited, 0, 0};
   }
 
+  static LruParams WithCacheSize(int64_t cacheSize) {
+    LruParams params = Default();
+    params.minBytesThreshold = cacheSize;
+    return params;
+  }
+
   int64_t minBytesThreshold;
   int percentileToCollect;
   int maximumSequenceNumbersToCollect;

Firestore/Source/Public/FIRFirestoreSettings.h

@@ -19,7 +19,7 @@
 NS_ASSUME_NONNULL_BEGIN
 
 /** Used to set on-disk cache size to unlimited. Garbage collection will not run. */
-extern const int64_t kFIRFirestoreCacheSizeUnlimited;
+extern const int64_t kFIRFirestoreCacheSizeUnlimited NS_SWIFT_NAME(FirestoreCacheSizeUnlimited);
 
 /** Settings used to configure a `FIRFirestore` instance. */
 NS_SWIFT_NAME(FirestoreSettings)
@@ -64,6 +64,15 @@ NS_SWIFT_NAME(FirestoreSettings)
  */
 @property(nonatomic, getter=areTimestampsInSnapshotsEnabled) BOOL timestampsInSnapshotsEnabled;
 
+/**
+ * Sets the cache size threshold above which the SDK will attempt to collect least-recently-used
+ * documents. The size is not a guarantee that the cache will stay below that size, only that if
+ * the cache exceeds the given size, cleanup will be attempted. Cannot be set lower than 1MB.
+ *
+ * Set to kFIRFirestoreCacheSizeUnlimited to disable garbage collection entirely.
+ */
+@property(nonatomic, assign) int64_t cacheSizeBytes;
+
 @end
 
 NS_ASSUME_NONNULL_END

Firestore/Swift/Tests/API/BasicCompileTests.swift

@@ -64,6 +64,7 @@ func initializeDb() -> Firestore {
   settings.host = "localhost"
   settings.isPersistenceEnabled = true
   settings.areTimestampsInSnapshotsEnabled = true
+  settings.cacheSizeBytes = FirestoreCacheSizeUnlimited
   firestore.settings = settings
 
   return firestore