LRU tweaks (#1961)

* Add second timer setup, use NSInterval, drop post-compaction

Author: Greg Soltis

Firestore/Source/Core/FSTFirestoreClient.mm

@@ -67,9 +67,9 @@
 NS_ASSUME_NONNULL_BEGIN
 
 /** How long we wait to try running LRU GC after SDK initialization. */
-static const long FSTLruGcInitialDelay = 60 * 1000;
+static const NSTimeInterval FSTLruGcInitialDelay = 60;
 /** Minimum amount of time between GC checks, after the first one. */
-static const long FSTLruGcRegularDelay = 5 * 60 * 1000;
+static const NSTimeInterval FSTLruGcRegularDelay = 5 * 60;
 
 @interface FSTFirestoreClient () {
   DatabaseInfo _databaseInfo;
@@ -104,8 +104,8 @@ - (instancetype)initWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
 
 @implementation FSTFirestoreClient {
   std::unique_ptr<Executor> _userExecutor;
-  long _initialGcDelay;
-  long _regularGcDelay;
+  NSTimeInterval _initialGcDelay;
+  NSTimeInterval _regularGcDelay;
   BOOL _gcHasRun;
   _Nullable id<FSTLRUDelegate> _lruDelegate;
   FSTDelayedCallback *_Nullable _lruCallback;
@@ -190,7 +190,6 @@ - (void)initializeWithUser:(const User &)user settings:(FIRFirestoreSettings *)s
         [[FSTSerializerBeta alloc] initWithDatabaseID:&self.databaseInfo->database_id()];
     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
@@ -244,7 +243,7 @@ - (void)initializeWithUser:(const User &)user settings:(FIRFirestoreSettings *)s
  * run.
  */
 - (void)scheduleLruGarbageCollection {
-  long delay = _gcHasRun ? _initialGcDelay : _regularGcDelay;
+  NSTimeInterval delay = _gcHasRun ? _regularGcDelay : _initialGcDelay;
   _lruCallback =
       [_workerDispatchQueue dispatchAfterDelay:delay
                                        timerID:FSTTimerIDGarbageCollectionDelay

Firestore/Source/Local/FSTLRUGarbageCollector.h

@@ -107,14 +107,8 @@ struct LruResults {
 
 - (size_t)byteSize;
 
-/** Returns the number of targets cached. */
-- (int32_t)targetCount;
-
-/**
- * If available, persistence implementations should run any compaction that they can. This is done
- * after GC has run to allow deletes to be processed and free up space.
- */
-- (void)runPostCompaction;
+/** Returns the number of targets and orphaned documents cached. */
+- (int32_t)sequenceNumberCount;
 
 /** Access to the underlying LRU Garbage collector instance. */
 @property(strong, nonatomic, readonly) FSTLRUGarbageCollector *gc;

Firestore/Source/Local/FSTLRUGarbageCollector.mm

@@ -103,10 +103,11 @@ - (LruResults)collectWithLiveTargets:(NSDictionary<NSNumber *, FSTQueryData *> *
   size_t currentSize = [self byteSize];
   if (currentSize < _params.minBytesThreshold) {
     // Not enough on disk to warrant collection. Wait another timeout cycle.
-    LOG_DEBUG("Garbage collection skipped; Cache size %i is lower than threshold %i", currentSize,
+    LOG_DEBUG("Garbage collection skipped; Cache size %s is lower than threshold %s", currentSize,
               _params.minBytesThreshold);
     return LruResults::DidNotRun();
   } else {
+    LOG_DEBUG("Running garbage collection on cache of size: %s", currentSize);
     return [self runGCWithLiveTargets:liveTargets];
   }
 }
@@ -130,9 +131,6 @@ - (LruResults)runGCWithLiveTargets:(NSDictionary<NSNumber *, FSTQueryData *> *)l
   int numDocumentsRemoved = [self removeOrphanedDocumentsThroughSequenceNumber:upperBound];
   Timestamp removedDocuments = Timestamp::Now();
 
-  [_delegate runPostCompaction];
-  Timestamp compactedDb = Timestamp::Now();
-
   std::string desc = "LRU Garbage Collection:\n";
   absl::StrAppend(&desc, "\tCounted targets in ", millisecondsBetween(start, countedTargets),
                   "ms\n");
@@ -143,16 +141,14 @@ - (LruResults)runGCWithLiveTargets:(NSDictionary<NSNumber *, FSTQueryData *> *)l
                   millisecondsBetween(foundUpperBound, removedTargets), "ms\n");
   absl::StrAppend(&desc, "\tRemoved ", numDocumentsRemoved, " documents in ",
                   millisecondsBetween(removedTargets, removedDocuments), "ms\n");
-  absl::StrAppend(&desc, "\tCompacted leveldb database in ",
-                  millisecondsBetween(removedDocuments, compactedDb), "ms\n");
-  absl::StrAppend(&desc, "Total duration: ", millisecondsBetween(start, compactedDb), "ms");
+  absl::StrAppend(&desc, "Total duration: ", millisecondsBetween(start, removedDocuments), "ms");
   LOG_DEBUG(desc.c_str());
 
   return LruResults{/* didRun= */ true, sequenceNumbers, numTargetsRemoved, numDocumentsRemoved};
 }
 
 - (int)queryCountForPercentile:(NSUInteger)percentile {
-  int totalCount = [_delegate targetCount];
+  int totalCount = [_delegate sequenceNumberCount];
   int setSize = (int)((percentile / 100.0f) * totalCount);
   return setSize;
 }

Firestore/Source/Local/FSTLevelDB.mm

@@ -229,13 +229,13 @@ - (int)removeTargetsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
   return [queryCache removeQueriesThroughSequenceNumber:sequenceNumber liveQueries:liveQueries];
 }
 
-- (int32_t)targetCount {
-  return [_db.queryCache count];
-}
-
-- (void)runPostCompaction {
-  // Compacts the entire db
-  _db.ptr->CompactRange(NULL, NULL);
+- (int32_t)sequenceNumberCount {
+  __block int32_t totalCount = [_db.queryCache count];
+  [self enumerateMutationsUsingBlock:^(const DocumentKey &key, ListenSequenceNumber sequenceNumber,
+                                       BOOL *stop) {
+    totalCount++;
+  }];
+  return totalCount;
 }
 
 - (FSTLRUGarbageCollector *)gc {

Firestore/Source/Local/FSTMemoryPersistence.mm

@@ -244,12 +244,13 @@ - (int)removeTargetsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
                                                          liveQueries:liveQueries];
 }
 
-- (int32_t)targetCount {
-  return [_persistence.queryCache count];
-}
-
-- (void)runPostCompaction {
-  // No-op for memory persistence.
+- (int32_t)sequenceNumberCount {
+  __block int32_t totalCount = [_persistence.queryCache count];
+  [self enumerateMutationsUsingBlock:^(const DocumentKey &key, ListenSequenceNumber sequenceNumber,
+                                       BOOL *stop) {
+    totalCount++;
+  }];
+  return totalCount;
 }
 
 - (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperBound {

Firestore/Source/Util/FSTDispatchQueue.mm

@@ -72,6 +72,7 @@ + (TimerId)convertTimerId:(FSTTimerID)objcTimerID {
     case TimerId::WriteStreamIdle:
     case TimerId::WriteStreamConnectionBackoff:
     case TimerId::OnlineStateTimeout:
+    case TimerId::GarbageCollectionDelay:
       return converted;
     default:
       HARD_FAIL("Unknown value of enum FSTTimerID.");

Firestore/core/src/firebase/firestore/util/async_queue.h

@@ -54,6 +54,10 @@ enum class TimerId {
    * indefinitely for success or failure.
    */
   OnlineStateTimeout,
+  /**
+   * A timer used to periodically attempt LRU Garbage collection
+   */
+  GarbageCollectionDelay
 };
 
 // A serial queue that executes given operations asynchronously, one at a time.