Skip to content

Commit 305f872

Browse files
author
Greg Soltis
authored
Wire together LRU GC (#1905)
* Wire together LRU GC * Default enabled, threshold set to `100 MB`.
1 parent b0a4b6c commit 305f872

26 files changed

+415
-58
lines changed

‎Firestore/Example/Firestore.xcodeproj/project.pbxproj

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1526,11 +1526,11 @@
15261526
);
15271527
inputPaths = (
15281528
"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS-resources.sh",
1529-
"${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates.bundle",
1529+
"${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates-Firestore.bundle",
15301530
);
15311531
name = "[CP] Copy Pods Resources";
15321532
outputPaths = (
1533-
"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates.bundle",
1533+
"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates-Firestore.bundle",
15341534
);
15351535
runOnlyForDeploymentPostprocessing = 0;
15361536
shellPath = /bin/sh;
@@ -1785,11 +1785,11 @@
17851785
);
17861786
inputPaths = (
17871787
"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS-resources.sh",
1788-
"${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates.bundle",
1788+
"${PODS_CONFIGURATION_BUILD_DIR}/FirebaseFirestore/gRPCCertificates-Firestore.bundle",
17891789
);
17901790
name = "[CP] Copy Pods Resources";
17911791
outputPaths = (
1792-
"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates.bundle",
1792+
"${TARGET_BUILD_DIR}/${UNLOCALIZED_RESOURCES_FOLDER_PATH}/gRPCCertificates-Firestore.bundle",
17931793
);
17941794
runOnlyForDeploymentPostprocessing = 0;
17951795
shellPath = /bin/sh;

‎Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,15 @@
1616

1717
#import <XCTest/XCTest.h>
1818

19+
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
20+
1921
@protocol FSTPersistence;
2022

2123
NS_ASSUME_NONNULL_BEGIN
2224

2325
@interface FSTLRUGarbageCollectorTests : XCTestCase
2426

25-
- (id<FSTPersistence>)newPersistence;
27+
- (id<FSTPersistence>)newPersistenceWithLruParams:(firebase::firestore::local::LruParams)lruParams;
2628

2729
@property(nonatomic, strong) id<FSTPersistence> persistence;
2830

‎Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040

4141
namespace testutil = firebase::firestore::testutil;
4242
using firebase::firestore::auth::User;
43+
using firebase::firestore::local::LruParams;
44+
using firebase::firestore::local::LruResults;
4345
using firebase::firestore::model::DocumentKey;
4446
using firebase::firestore::model::DocumentKeyHash;
4547
using firebase::firestore::model::DocumentKeySet;
@@ -79,9 +81,9 @@ - (BOOL)isTestBaseClass {
7981
return ([self class] == [FSTLRUGarbageCollectorTests class]);
8082
}
8183

82-
- (void)newTestResources {
84+
- (void)newTestResourcesWithLruParams:(LruParams)lruParams {
8385
HARD_ASSERT(_persistence == nil, "Persistence already created");
84-
_persistence = [self newPersistence];
86+
_persistence = [self newPersistenceWithLruParams:lruParams];
8587
_queryCache = [_persistence queryCache];
8688
_documentCache = [_persistence remoteDocumentCache];
8789
_mutationQueue = [_persistence mutationQueueForUser:_user];
@@ -93,7 +95,11 @@ - (void)newTestResources {
9395
});
9496
}
9597

96-
- (id<FSTPersistence>)newPersistence {
98+
- (void)newTestResources {
99+
[self newTestResourcesWithLruParams:LruParams::Default()];
100+
}
101+
102+
- (id<FSTPersistence>)newPersistenceWithLruParams:(LruParams)lruParams {
97103
@throw FSTAbstractMethodException(); // NOLINT
98104
}
99105

@@ -630,6 +636,7 @@ - (void)testRemoveTargetsThenGC {
630636
// Finally, do the garbage collection, up to but not including the removal of middleTarget
631637
NSDictionary<NSNumber *, FSTQueryData *> *liveQueries =
632638
@{@(oldestTarget.targetID) : oldestTarget};
639+
633640
int queriesRemoved = [self removeQueriesThroughSequenceNumber:upperBound liveQueries:liveQueries];
634641
XCTAssertEqual(1, queriesRemoved, @"Expected to remove newest target");
635642
int docsRemoved = [self removeOrphanedDocumentsThroughSequenceNumber:upperBound];
@@ -672,6 +679,86 @@ - (void)testGetsSize {
672679
[_persistence shutdown];
673680
}
674681

682+
- (void)testDisabled {
683+
if ([self isTestBaseClass]) return;
684+
685+
LruParams params = LruParams::Disabled();
686+
[self newTestResourcesWithLruParams:params];
687+
688+
_persistence.run("fill cache", [&]() {
689+
// Simulate a bunch of ack'd mutations
690+
for (int i = 0; i < 500; i++) {
691+
FSTDocument *doc = [self cacheADocumentInTransaction];
692+
[self markDocumentEligibleForGCInTransaction:doc.key];
693+
}
694+
});
695+
696+
LruResults results =
697+
_persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:@{}]; });
698+
XCTAssertFalse(results.didRun);
699+
700+
[_persistence shutdown];
701+
}
702+
703+
- (void)testCacheTooSmall {
704+
if ([self isTestBaseClass]) return;
705+
706+
LruParams params = LruParams::Default();
707+
[self newTestResourcesWithLruParams:params];
708+
709+
_persistence.run("fill cache", [&]() {
710+
// Simulate a bunch of ack'd mutations
711+
for (int i = 0; i < 50; i++) {
712+
FSTDocument *doc = [self cacheADocumentInTransaction];
713+
[self markDocumentEligibleForGCInTransaction:doc.key];
714+
}
715+
});
716+
717+
int cacheSize = (int)[_gc byteSize];
718+
// Verify that we don't have enough in our cache to warrant collection
719+
XCTAssertLessThan(cacheSize, params.minBytesThreshold);
720+
721+
// Try collection and verify that it didn't run
722+
LruResults results =
723+
_persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:@{}]; });
724+
XCTAssertFalse(results.didRun);
725+
726+
[_persistence shutdown];
727+
}
728+
729+
- (void)testGCRan {
730+
if ([self isTestBaseClass]) return;
731+
732+
LruParams params = LruParams::Default();
733+
// Set a low threshold so we will definitely run
734+
params.minBytesThreshold = 100;
735+
[self newTestResourcesWithLruParams:params];
736+
737+
// Add 100 targets and 10 documents to each
738+
for (int i = 0; i < 100; i++) {
739+
// Use separate transactions so that each target and associated documents get their own
740+
// sequence number.
741+
_persistence.run("Add a target and some documents", [&]() {
742+
FSTQueryData *queryData = [self addNextQueryInTransaction];
743+
for (int j = 0; j < 10; j++) {
744+
FSTDocument *doc = [self cacheADocumentInTransaction];
745+
[self addDocument:doc.key toTarget:queryData.targetID];
746+
}
747+
});
748+
}
749+
750+
// Mark nothing as live, so everything is eligible.
751+
LruResults results =
752+
_persistence.run("GC", [&]() -> LruResults { return [_gc collectWithLiveTargets:@{}]; });
753+
754+
// By default, we collect 10% of the sequence numbers. Since we added 100 targets,
755+
// that should be 10 targets with 10 documents each, for a total of 100 documents.
756+
XCTAssertTrue(results.didRun);
757+
XCTAssertEqual(10, results.targetsRemoved);
758+
XCTAssertEqual(100, results.documentsRemoved);
759+
[_persistence shutdown];
760+
}
761+
675762
@end
676763

677764
NS_ASSUME_NONNULL_END

‎Firestore/Example/Tests/Local/FSTLevelDBLRUGarbageCollectorTests.mm

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,25 @@
1919
#import "Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h"
2020

2121
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
22+
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
2223
#import "Firestore/Source/Local/FSTLevelDB.h"
2324
#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h"
2425
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
2526

2627
using firebase::firestore::local::LevelDbDocumentTargetKey;
2728
using firebase::firestore::model::DocumentKey;
2829

30+
using firebase::firestore::local::LruParams;
31+
2932
NS_ASSUME_NONNULL_BEGIN
3033

3134
@interface FSTLevelDBLRUGarbageCollectorTests : FSTLRUGarbageCollectorTests
3235
@end
3336

3437
@implementation FSTLevelDBLRUGarbageCollectorTests
3538

36-
- (id<FSTPersistence>)newPersistence {
37-
return [FSTPersistenceTestHelpers levelDBPersistence];
39+
- (id<FSTPersistence>)newPersistenceWithLruParams:(LruParams)lruParams {
40+
return [FSTPersistenceTestHelpers levelDBPersistenceWithLruParams:lruParams];
3841
}
3942

4043
- (BOOL)sentinelExists:(const DocumentKey &)key {

‎Firestore/Example/Tests/Local/FSTMemoryLRUGarbageCollectorTests.mm

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,23 @@
1717
#import "Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.h"
1818

1919
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
20+
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
2021
#import "Firestore/Source/Local/FSTMemoryPersistence.h"
2122
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
2223

2324
using firebase::firestore::model::DocumentKey;
2425

26+
using firebase::firestore::local::LruParams;
27+
2528
NS_ASSUME_NONNULL_BEGIN
2629

2730
@interface FSTMemoryLRUGarbageCollectionTests : FSTLRUGarbageCollectorTests
2831
@end
2932

3033
@implementation FSTMemoryLRUGarbageCollectionTests
3134

32-
- (id<FSTPersistence>)newPersistence {
33-
return [FSTPersistenceTestHelpers lruMemoryPersistence];
35+
- (id<FSTPersistence>)newPersistenceWithLruParams:(LruParams)lruParams {
36+
return [FSTPersistenceTestHelpers lruMemoryPersistenceWithLruParams:lruParams];
3437
}
3538

3639
- (BOOL)sentinelExists:(const DocumentKey &)key {

‎Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#import <Foundation/Foundation.h>
1818

19+
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
1920
#include "Firestore/core/src/firebase/firestore/util/path.h"
2021

2122
@class FSTLevelDB;
@@ -48,10 +49,21 @@ NS_ASSUME_NONNULL_BEGIN
4849
*/
4950
+ (FSTLevelDB *)levelDBPersistenceWithDir:(firebase::firestore::util::Path)dir;
5051

52+
/**
53+
* Creates and starts a new FSTLevelDB instance for testing, destroying any previous contents
54+
* if they existed.
55+
*
56+
* Sets up the LRU garbage collection to use the provided params.
57+
*/
58+
+ (FSTLevelDB *)levelDBPersistenceWithLruParams:(firebase::firestore::local::LruParams)lruParams;
59+
5160
/** Creates and starts a new FSTMemoryPersistence instance for testing. */
5261
+ (FSTMemoryPersistence *)eagerGCMemoryPersistence;
5362

5463
+ (FSTMemoryPersistence *)lruMemoryPersistence;
64+
65+
+ (FSTMemoryPersistence *)lruMemoryPersistenceWithLruParams:
66+
(firebase::firestore::local::LruParams)lruParams;
5567
@end
5668

5769
NS_ASSUME_NONNULL_END

‎Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.mm

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include <utility>
2020

21+
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
2122
#import "Firestore/Source/Local/FSTLevelDB.h"
2223
#import "Firestore/Source/Local/FSTLocalSerializer.h"
2324
#import "Firestore/Source/Local/FSTMemoryPersistence.h"
@@ -30,6 +31,7 @@
3031
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
3132

3233
namespace util = firebase::firestore::util;
34+
using firebase::firestore::local::LruParams;
3335
using firebase::firestore::model::DatabaseId;
3436
using firebase::firestore::util::Path;
3537
using firebase::firestore::util::Status;
@@ -61,8 +63,13 @@ + (Path)levelDBDir {
6163
}
6264

6365
+ (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir {
66+
return [self levelDBPersistenceWithDir:dir lruParams:LruParams::Default()];
67+
}
68+
69+
+ (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir lruParams:(LruParams)params {
6470
FSTLocalSerializer *serializer = [self localSerializer];
65-
FSTLevelDB *db = [[FSTLevelDB alloc] initWithDirectory:std::move(dir) serializer:serializer];
71+
FSTLevelDB *db =
72+
[[FSTLevelDB alloc] initWithDirectory:std::move(dir) serializer:serializer lruParams:params];
6673
Status status = [db start];
6774
if (!status.ok()) {
6875
[NSException raise:NSInternalInconsistencyException
@@ -72,6 +79,10 @@ + (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir {
7279
return db;
7380
}
7481

82+
+ (FSTLevelDB *)levelDBPersistenceWithLruParams:(LruParams)lruParams {
83+
return [self levelDBPersistenceWithDir:[self levelDBDir] lruParams:lruParams];
84+
}
85+
7586
+ (FSTLevelDB *)levelDBPersistence {
7687
return [self levelDBPersistenceWithDir:[self levelDBDir]];
7788
}
@@ -88,9 +99,13 @@ + (FSTMemoryPersistence *)eagerGCMemoryPersistence {
8899
}
89100

90101
+ (FSTMemoryPersistence *)lruMemoryPersistence {
102+
return [self lruMemoryPersistenceWithLruParams:LruParams::Default()];
103+
}
104+
105+
+ (FSTMemoryPersistence *)lruMemoryPersistenceWithLruParams:(LruParams)lruParams {
91106
FSTLocalSerializer *serializer = [self localSerializer];
92107
FSTMemoryPersistence *persistence =
93-
[FSTMemoryPersistence persistenceWithLRUGCAndSerializer:serializer];
108+
[FSTMemoryPersistence persistenceWithLruParams:lruParams serializer:serializer];
94109
Status status = [persistence start];
95110
if (!status.ok()) {
96111
[NSException raise:NSInternalInconsistencyException

‎Firestore/Source/API/FIRFirestore.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ const DatabaseInfo database_info(*self.databaseID, util::MakeString(_persistence
265265

266266
HARD_ASSERT(_workerQueue, "Expected non-null _workerQueue");
267267
_client = [FSTFirestoreClient clientWithDatabaseInfo:database_info
268-
usePersistence:_settings.persistenceEnabled
268+
settings:_settings
269269
credentialsProvider:_credentialsProvider.get()
270270
userExecutor:std::move(userExecutor)
271271
workerQueue:std::move(_workerQueue)];

‎Firestore/Source/API/FIRFirestoreSettings.mm

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
static NSString *const kDefaultHost = @"firestore.googleapis.com";
2424
static const BOOL kDefaultSSLEnabled = YES;
2525
static const BOOL kDefaultPersistenceEnabled = YES;
26+
static const int64_t kDefaultCacheSizeBytes = 100 * 1024 * 1024;
27+
static const int64_t kMinimumCacheSizeBytes = 1 * 1024 * 1024;
2628
// TODO(b/73820332): flip the default.
2729
static const BOOL kDefaultTimestampsInSnapshotsEnabled = NO;
2830

@@ -35,6 +37,7 @@ - (instancetype)init {
3537
_dispatchQueue = dispatch_get_main_queue();
3638
_persistenceEnabled = kDefaultPersistenceEnabled;
3739
_timestampsInSnapshotsEnabled = kDefaultTimestampsInSnapshotsEnabled;
40+
_cacheSizeBytes = kDefaultCacheSizeBytes;
3841
}
3942
return self;
4043
}
@@ -51,7 +54,8 @@ - (BOOL)isEqual:(id)other {
5154
self.isSSLEnabled == otherSettings.isSSLEnabled &&
5255
self.dispatchQueue == otherSettings.dispatchQueue &&
5356
self.isPersistenceEnabled == otherSettings.isPersistenceEnabled &&
54-
self.timestampsInSnapshotsEnabled == otherSettings.timestampsInSnapshotsEnabled;
57+
self.timestampsInSnapshotsEnabled == otherSettings.timestampsInSnapshotsEnabled &&
58+
self.cacheSizeBytes == otherSettings.cacheSizeBytes;
5559
}
5660

5761
- (NSUInteger)hash {
@@ -60,6 +64,7 @@ - (NSUInteger)hash {
6064
// Ignore the dispatchQueue to avoid having to deal with sizeof(dispatch_queue_t).
6165
result = 31 * result + (self.isPersistenceEnabled ? 1231 : 1237);
6266
result = 31 * result + (self.timestampsInSnapshotsEnabled ? 1231 : 1237);
67+
result = 31 * result + (NSUInteger)self.cacheSizeBytes;
6368
return result;
6469
}
6570

@@ -70,6 +75,7 @@ - (id)copyWithZone:(nullable NSZone *)zone {
7075
copy.dispatchQueue = _dispatchQueue;
7176
copy.persistenceEnabled = _persistenceEnabled;
7277
copy.timestampsInSnapshotsEnabled = _timestampsInSnapshotsEnabled;
78+
copy.cacheSizeBytes = _cacheSizeBytes;
7379
return copy;
7480
}
7581

@@ -93,6 +99,14 @@ - (void)setDispatchQueue:(dispatch_queue_t)dispatchQueue {
9399
_dispatchQueue = dispatchQueue;
94100
}
95101

102+
- (void)setCacheSizeBytes:(int64_t)cacheSizeBytes {
103+
if (cacheSizeBytes != kFIRFirestoreCacheSizeUnlimited &&
104+
cacheSizeBytes < kMinimumCacheSizeBytes) {
105+
FSTThrowInvalidArgument(@"Cache size must be set to at least %i bytes", kMinimumCacheSizeBytes);
106+
}
107+
_cacheSizeBytes = cacheSizeBytes;
108+
}
109+
96110
@end
97111

98112
NS_ASSUME_NONNULL_END

‎Firestore/Source/Core/FSTFirestoreClient.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
@class FIRDocumentReference;
3232
@class FIRDocumentSnapshot;
33+
@class FIRFirestoreSettings;
3334
@class FIRQuery;
3435
@class FIRQuerySnapshot;
3536
@class FSTDatabaseID;
@@ -57,7 +58,7 @@ NS_ASSUME_NONNULL_BEGIN
5758
*/
5859
+ (instancetype)
5960
clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo
60-
usePersistence:(BOOL)usePersistence
61+
settings:(FIRFirestoreSettings *)settings
6162
credentialsProvider:(firebase::firestore::auth::CredentialsProvider *)
6263
credentialsProvider // no passing ownership
6364
userExecutor:(std::unique_ptr<firebase::firestore::util::Executor>)userExecutor

0 commit comments

Comments
 (0)