diff --git a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm index 5340873212d..63616aad9fd 100644 --- a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm @@ -17,6 +17,9 @@ #import #import +#include + +#include #import "Firestore/Example/Tests/Util/FSTEventAccumulator.h" #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h" @@ -323,4 +326,54 @@ - (void)testUpdateNestedFields { [self awaitExpectations]; } +// Returns how much memory the test application is currently using, in megabytes (fractional part is +// truncated), or -1 if the OS call fails. +// TODO(varconst): move the helper function and the test into a new test target for performance +// testing. +int64_t GetCurrentMemoryUsedInMb() { + mach_task_basic_info taskInfo; + mach_msg_type_number_t taskInfoSize = MACH_TASK_BASIC_INFO_COUNT; + const auto errorCode = + task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&taskInfo, &taskInfoSize); + if (errorCode == KERN_SUCCESS) { + const int bytesInMegabyte = 1000 * 1000; + return taskInfo.resident_size / bytesInMegabyte; + } + return -1; +} + +- (void)testReasonableMemoryUsageForLotsOfMutations { + XCTestExpectation *expectation = + [self expectationWithDescription:@"testReasonableMemoryUsageForLotsOfMutations"]; + + FIRDocumentReference *mainDoc = [self documentRef]; + FIRWriteBatch *batch = [mainDoc.firestore batch]; + + // >= 500 mutations will be rejected, so use 500-1 mutations + for (int i = 0; i != 500 - 1; ++i) { + FIRDocumentReference *nestedDoc = [[mainDoc collectionWithPath:@"nested"] documentWithAutoID]; + // The exact data doesn't matter; what is important is the large number of mutations. + [batch setData:@{ + @"a" : @"foo", + @"b" : @"bar", + } + forDocument:nestedDoc]; + } + + const int64_t memoryUsedBeforeCommitMb = GetCurrentMemoryUsedInMb(); + XCTAssertNotEqual(memoryUsedBeforeCommitMb, -1); + [batch commitWithCompletion:^(NSError *_Nullable error) { + XCTAssertNil(error); + const int64_t memoryUsedAfterCommitMb = GetCurrentMemoryUsedInMb(); + XCTAssertNotEqual(memoryUsedAfterCommitMb, -1); + const int64_t memoryDeltaMb = memoryUsedAfterCommitMb - memoryUsedBeforeCommitMb; + // This by its nature cannot be a precise value. In debug mode, the increase on simulator + // seems to be around 90 MB. A regression would be on the scale of 500Mb. + XCTAssertLessThan(memoryDeltaMb, 150); + + [expectation fulfill]; + }]; + [self awaitExpectations]; +} + @end diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index 48c963e876d..43378790a81 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -62,21 +62,32 @@ - (instancetype)initWithRemoteDocumentCache:(id)remoteDo } - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key { + NSArray *batches = + [self.mutationQueue allMutationBatchesAffectingDocumentKey:key]; + return [self documentForKey:key inBatches:batches]; +} + +// Internal version of documentForKey: which allows reusing `batches`. +- (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key + inBatches:(NSArray *)batches { FSTMaybeDocument *_Nullable remoteDoc = [self.remoteDocumentCache entryForKey:key]; - return [self localDocument:remoteDoc key:key]; + return [self localDocument:remoteDoc key:key inBatches:batches]; } - (FSTMaybeDocumentDictionary *)documentsForKeys:(const DocumentKeySet &)keys { FSTMaybeDocumentDictionary *results = [FSTMaybeDocumentDictionary maybeDocumentDictionary]; + NSArray *batches = + [self.mutationQueue allMutationBatchesAffectingDocumentKeys:keys]; for (const DocumentKey &key : keys) { // TODO(mikelehen): PERF: Consider fetching all remote documents at once rather than one-by-one. - FSTMaybeDocument *maybeDoc = [self documentForKey:key]; + FSTMaybeDocument *maybeDoc = [self documentForKey:key inBatches:batches]; // TODO(http://b/32275378): Don't conflate missing / deleted. if (!maybeDoc) { maybeDoc = [FSTDeletedDocument documentWithKey:key version:SnapshotVersion::None()]; } results = [results dictionaryBySettingObject:maybeDoc forKey:key]; } + return results; } @@ -122,12 +133,13 @@ - (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { } // Now add in results for the matchingKeys. - for (const DocumentKey &key : matchingKeys) { - FSTMaybeDocument *doc = [self documentForKey:key]; - if ([doc isKindOfClass:[FSTDocument class]]) { - results = [results dictionaryBySettingObject:(FSTDocument *)doc forKey:key]; - } - } + FSTMaybeDocumentDictionary *matchingKeysDocs = [self documentsForKeys:matchingKeys]; + [matchingKeysDocs + enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTMaybeDocument *doc, BOOL *stop) { + if ([doc isKindOfClass:[FSTDocument class]]) { + results = [results dictionaryBySettingObject:(FSTDocument *)doc forKey:key]; + } + }]; // Finally, filter out any documents that don't actually match the query. Note that the extra // reference here prevents ARC from deallocating the initial unfiltered results while we're @@ -151,9 +163,8 @@ - (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { * @param documentKey The key of the document (necessary when remoteDocument is nil). */ - (nullable FSTMaybeDocument *)localDocument:(nullable FSTMaybeDocument *)document - key:(const DocumentKey &)documentKey { - NSArray *batches = - [self.mutationQueue allMutationBatchesAffectingDocumentKey:documentKey]; + key:(const DocumentKey &)documentKey + inBatches:(NSArray *)batches { for (FSTMutationBatch *batch in batches) { document = [batch applyTo:document documentKey:documentKey]; } @@ -169,10 +180,18 @@ - (nullable FSTMaybeDocument *)localDocument:(nullable FSTMaybeDocument *)docume * @return The local view of the documents. */ - (FSTDocumentDictionary *)localDocuments:(FSTDocumentDictionary *)documents { + __block DocumentKeySet keySet; + [documents + enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTDocument *doc, BOOL *stop) { + keySet = keySet.insert(doc.key); + }]; + NSArray *batches = + [self.mutationQueue allMutationBatchesAffectingDocumentKeys:keySet]; + __block FSTDocumentDictionary *result = documents; [documents enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTDocument *remoteDocument, BOOL *stop) { - FSTMaybeDocument *mutatedDoc = [self localDocument:remoteDocument key:key]; + FSTMaybeDocument *mutatedDoc = [self localDocument:remoteDocument key:key inBatches:batches]; if ([mutatedDoc isKindOfClass:[FSTDeletedDocument class]]) { result = [result dictionaryByRemovingObjectForKey:key]; } else if ([mutatedDoc isKindOfClass:[FSTDocument class]]) { diff --git a/scripts/cpplint.py b/scripts/cpplint.py index 28f37613d0b..f796ede0d4e 100644 --- a/scripts/cpplint.py +++ b/scripts/cpplint.py @@ -416,6 +416,7 @@ _C_SYSTEM_DIRECTORIES = frozenset([ 'libkern', + 'mach', 'sys', ])