From c6c2a9c01812aa18b26f5052a31c585f756554f6 Mon Sep 17 00:00:00 2001 From: Marek Gilbert Date: Thu, 28 Jun 2018 17:55:23 -0700 Subject: [PATCH 1/8] Pod updates for Cocapods 1.5.3 --- .../Firestore.xcodeproj/project.pbxproj | 96 ------------------- 1 file changed, 96 deletions(-) diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 954dbb0d224..48c63d1556d 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -1183,7 +1183,6 @@ 54C9EDEE2040E16300A969CD /* Frameworks */, 54C9EDEF2040E16300A969CD /* Resources */, EA424838F4A5DD7B337F57AB /* [CP] Embed Pods Frameworks */, - 6BD54D799442CEB09349B73E /* [CP] Copy Pods Resources */, ); buildRules = ( ); @@ -1204,7 +1203,6 @@ 6003F587195388D20070C39A /* Frameworks */, 6003F588195388D20070C39A /* Resources */, 1EE692C7509A98D7EB03CA51 /* [CP] Embed Pods Frameworks */, - A4BCE623F5E4C28728E5F17A /* [CP] Copy Pods Resources */, ); buildRules = ( ); @@ -1224,7 +1222,6 @@ 6003F5AB195388D20070C39A /* Frameworks */, 6003F5AC195388D20070C39A /* Resources */, 329C25E418360CEF62F6CB2B /* [CP] Embed Pods Frameworks */, - 263508FF7FD6CA4D6C3E685D /* [CP] Copy Pods Resources */, ); buildRules = ( ); @@ -1245,7 +1242,6 @@ 6EDD3B4520BF247500C33877 /* Frameworks */, 6EDD3B4A20BF247500C33877 /* Resources */, 6EDD3B5720BF247500C33877 /* [CP] Embed Pods Frameworks */, - F5DFA8B0274B042DC1B00837 /* [CP] Copy Pods Resources */, ); buildRules = ( ); @@ -1266,7 +1262,6 @@ DE03B2D31F2149D600A30B9C /* Frameworks */, DE03B2D81F2149D600A30B9C /* Resources */, B7923D95031DB0DA112AAE9B /* [CP] Embed Pods Frameworks */, - 5B2A669EEE88DF2205316429 /* [CP] Copy Pods Resources */, ); buildRules = ( ); @@ -1287,7 +1282,6 @@ DE0761E11F2FE611003233AF /* Frameworks */, DE0761E21F2FE611003233AF /* Resources */, 04404E0DCBB886A40E3C7175 /* [CP] Embed Pods Frameworks */, - 138396D16F5128E073E667C6 /* [CP] Copy Pods Resources */, ); buildRules = ( ); @@ -1459,21 +1453,6 @@ shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-SwiftBuildTest/Pods-Firestore_Example_iOS-SwiftBuildTest-frameworks.sh\"\n"; showEnvVarsInLog = 0; }; - 138396D16F5128E073E667C6 /* [CP] Copy Pods Resources */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputPaths = ( - ); - name = "[CP] Copy Pods Resources"; - outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-SwiftBuildTest/Pods-Firestore_Example_iOS-SwiftBuildTest-resources.sh\"\n"; - showEnvVarsInLog = 0; - }; 1EE692C7509A98D7EB03CA51 /* [CP] Embed Pods Frameworks */ = { isa = PBXShellScriptBuildPhase; buildActionMask = 2147483647; @@ -1510,21 +1489,6 @@ shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS-frameworks.sh\"\n"; showEnvVarsInLog = 0; }; - 263508FF7FD6CA4D6C3E685D /* [CP] Copy Pods Resources */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputPaths = ( - ); - name = "[CP] Copy Pods Resources"; - outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Tests_iOS/Pods-Firestore_Tests_iOS-resources.sh\"\n"; - showEnvVarsInLog = 0; - }; 329C25E418360CEF62F6CB2B /* [CP] Embed Pods Frameworks */ = { isa = PBXShellScriptBuildPhase; buildActionMask = 2147483647; @@ -1567,36 +1531,6 @@ shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n # print error to STDERR\n echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n"; showEnvVarsInLog = 0; }; - 5B2A669EEE88DF2205316429 /* [CP] Copy Pods Resources */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputPaths = ( - ); - name = "[CP] Copy Pods Resources"; - outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_IntegrationTests_iOS/Pods-Firestore_IntegrationTests_iOS-resources.sh\"\n"; - showEnvVarsInLog = 0; - }; - 6BD54D799442CEB09349B73E /* [CP] Copy Pods Resources */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputPaths = ( - ); - name = "[CP] Copy Pods Resources"; - outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS-resources.sh\"\n"; - showEnvVarsInLog = 0; - }; 6EDD3AD420BF247500C33877 /* [CP] Check Pods Manifest.lock */ = { isa = PBXShellScriptBuildPhase; buildActionMask = 2147483647; @@ -1669,21 +1603,6 @@ shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n # print error to STDERR\n echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n"; showEnvVarsInLog = 0; }; - A4BCE623F5E4C28728E5F17A /* [CP] Copy Pods Resources */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputPaths = ( - ); - name = "[CP] Copy Pods Resources"; - outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS-resources.sh\"\n"; - showEnvVarsInLog = 0; - }; A827A009A65B69DC1B80EAD4 /* [CP] Check Pods Manifest.lock */ = { isa = PBXShellScriptBuildPhase; buildActionMask = 2147483647; @@ -1774,21 +1693,6 @@ shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS/Pods-Firestore_Example_iOS-Firestore_SwiftTests_iOS-frameworks.sh\"\n"; showEnvVarsInLog = 0; }; - F5DFA8B0274B042DC1B00837 /* [CP] Copy Pods Resources */ = { - isa = PBXShellScriptBuildPhase; - buildActionMask = 2147483647; - files = ( - ); - inputPaths = ( - ); - name = "[CP] Copy Pods Resources"; - outputPaths = ( - ); - runOnlyForDeploymentPostprocessing = 0; - shellPath = /bin/sh; - shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_FuzzTests_iOS/Pods-Firestore_FuzzTests_iOS-resources.sh\"\n"; - showEnvVarsInLog = 0; - }; /* End PBXShellScriptBuildPhase section */ /* Begin PBXSourcesBuildPhase section */ From 76bf96a3d467cd9efa8b851b82609d19e1c8650f Mon Sep 17 00:00:00 2001 From: Marek Gilbert Date: Thu, 28 Jun 2018 17:55:58 -0700 Subject: [PATCH 2/8] Add allMutationsAffectingDocumentKeys --- .../Tests/Local/FSTMutationQueueTests.mm | 84 +++++++++++++++++++ .../Source/Local/FSTLevelDBMutationQueue.mm | 57 +++++++++++-- .../Source/Local/FSTMemoryMutationQueue.mm | 47 +++++++++-- Firestore/Source/Local/FSTMutationQueue.h | 14 +++- 4 files changed, 186 insertions(+), 16 deletions(-) diff --git a/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm b/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm index 5b5bdd124be..83982f2e042 100644 --- a/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm +++ b/Firestore/Example/Tests/Local/FSTMutationQueueTests.mm @@ -31,11 +31,14 @@ #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_key_set.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" namespace testutil = firebase::firestore::testutil; using firebase::firestore::auth::User; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; +using firebase::firestore::testutil::Key; NS_ASSUME_NONNULL_BEGIN @@ -315,6 +318,87 @@ - (void)testAllMutationBatchesAffectingDocumentKey { }); } +- (void)testAllMutationBatchesAffectingDocumentKeys { + if ([self isTestBaseClass]) return; + + self.persistence.run("testAllMutationBatchesAffectingDocumentKey", [&]() { + NSArray *mutations = @[ + FSTTestSetMutation(@"fob/bar", + @{ @"a" : @1 }), + FSTTestSetMutation(@"foo/bar", + @{ @"a" : @1 }), + FSTTestPatchMutation("foo/bar", + @{ @"b" : @1 }, {}), + FSTTestSetMutation(@"foo/bar/suffix/key", + @{ @"a" : @1 }), + FSTTestSetMutation(@"foo/baz", + @{ @"a" : @1 }), + FSTTestSetMutation(@"food/bar", + @{ @"a" : @1 }) + ]; + + // Store all the mutations. + NSMutableArray *batches = [NSMutableArray array]; + for (FSTMutation *mutation in mutations) { + FSTMutationBatch *batch = + [self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp] + mutations:@[ mutation ]]; + [batches addObject:batch]; + } + + DocumentKeySet keys{ + Key("foo/bar"), + Key("foo/baz"), + }; + + NSArray *expected = @[ batches[1], batches[2], batches[4] ]; + NSArray *matches = + [self.mutationQueue allMutationBatchesAffectingDocumentKeys:keys]; + + XCTAssertEqualObjects(matches, expected); + }); +} + +- (void)testAllMutationBatchesAffectingDocumentKeys_handlesOverlap { + if ([self isTestBaseClass]) return; + + self.persistence.run("testAllMutationBatchesAffectingDocumentKeys_handlesOverlap", [&]() { + NSMutableArray *batches = [NSMutableArray array]; + + NSArray *group1 = @[ + FSTTestSetMutation(@"foo/bar", + @{ @"a" : @1 }), + FSTTestSetMutation(@"foo/baz", + @{ @"a" : @1 }), + ]; + FSTMutationBatch *batch1 = + [self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp] + mutations:group1]; + + NSArray *group2 = @[ FSTTestSetMutation(@"food/bar", @{ @"a" : @1 }) ]; + [self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp] mutations:group2]; + + NSArray *group3 = @[ + FSTTestSetMutation(@"foo/bar", + @{ @"b" : @1 }), + ]; + FSTMutationBatch *batch3 = + [self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp] + mutations:group3]; + + DocumentKeySet keys{ + Key("foo/bar"), + Key("foo/baz"), + }; + + NSArray *expected = @[ batch1, batch3 ]; + NSArray *matches = + [self.mutationQueue allMutationBatchesAffectingDocumentKeys:keys]; + + XCTAssertEqualObjects(matches, expected); + }); +} + - (void)testAllMutationBatchesAffectingQuery { if ([self isTestBaseClass]) return; diff --git a/Firestore/Source/Local/FSTLevelDBMutationQueue.mm b/Firestore/Source/Local/FSTLevelDBMutationQueue.mm index 94ab8a5a6ea..8fd6675f6e5 100644 --- a/Firestore/Source/Local/FSTLevelDBMutationQueue.mm +++ b/Firestore/Source/Local/FSTLevelDBMutationQueue.mm @@ -30,7 +30,6 @@ #include "Firestore/core/src/firebase/firestore/auth/user.h" #include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h" -#include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" @@ -46,6 +45,7 @@ using Firestore::StringView; using firebase::firestore::auth::User; using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::ResourcePath; using leveldb::DB; using leveldb::Iterator; @@ -396,6 +396,39 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID return result; } +- (NSArray *)allMutationBatchesAffectingDocumentKeys: + (const DocumentKeySet &)documentKeys { + NSString *userID = self.userID; + + // Take a pass through the document keys and collect the set of unique mutation batchIDs that + // affect them all. Some batches can affect more than one key. + std::set batchIDs; + + auto indexIterator = _db.currentTransaction->NewIterator(); + FSTLevelDBDocumentMutationKey *rowKey = [[FSTLevelDBDocumentMutationKey alloc] init]; + for (const DocumentKey &documentKey : documentKeys) { + std::string indexPrefix = + [FSTLevelDBDocumentMutationKey keyPrefixWithUserID:userID resourcePath:documentKey.path()]; + indexIterator->Seek(indexPrefix); + for (; indexIterator->Valid(); indexIterator->Next()) { + // Only consider rows matching exactly the specific key of interest. Note that because we + // order by path first, and we order terminators before path separators, we'll encounter all + // the index rows for documentKey contiguously. In particular, all the rows for documentKey + // will occur before any rows for documents nested in a subcollection beneath documentKey so + // we can stop as soon as we hit any such row. + if (!absl::StartsWith(indexIterator->key(), indexPrefix) || + ![rowKey decodeKey:indexIterator->key()] || + DocumentKey{rowKey.documentKey} != documentKey) { + break; + } + + batchIDs.insert(rowKey.batchID); + } + } + + return [self allMutationBatchesWithBatchIDs:batchIDs]; +} + - (NSArray *)allMutationBatchesAffectingQuery:(FSTQuery *)query { HARD_ASSERT(![query isDocumentQuery], "Document queries shouldn't go down this path"); NSString *userID = self.userID; @@ -417,11 +450,10 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID // index for more than a single document so the associated batchIDs will be neither necessarily // unique nor in order. This means an efficient simultaneous scan isn't possible. std::string indexPrefix = - [FSTLevelDBDocumentMutationKey keyPrefixWithUserID:self.userID resourcePath:queryPath]; + [FSTLevelDBDocumentMutationKey keyPrefixWithUserID:userID resourcePath:queryPath]; auto indexIterator = _db.currentTransaction->NewIterator(); indexIterator->Seek(indexPrefix); - NSMutableArray *result = [NSMutableArray array]; FSTLevelDBDocumentMutationKey *rowKey = [[FSTLevelDBDocumentMutationKey alloc] init]; // Collect up unique batchIDs encountered during a scan of the index. Use a set to @@ -430,7 +462,7 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID // This method is faster than performing lookups of the keys with _db->Get and keeping a hash of // batchIDs that have already been looked up. The performance difference is minor for small // numbers of keys but > 30% faster for larger numbers of keys. - std::set uniqueBatchIds; + std::set uniqueBatchIDs; for (; indexIterator->Valid(); indexIterator->Next()) { if (!absl::StartsWith(indexIterator->key(), indexPrefix) || ![rowKey decodeKey:indexIterator->key()]) { @@ -444,14 +476,25 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID continue; } - uniqueBatchIds.insert(rowKey.batchID); + uniqueBatchIDs.insert(rowKey.batchID); } + return [self allMutationBatchesWithBatchIDs:uniqueBatchIDs]; +} + +/** + * Constructs an array of matching batches, sorted by batchID to ensure that multiple mutations + * affecting the same document key are applied in order. + */ +- (NSArray *)allMutationBatchesWithBatchIDs: + (const std::set &)batchIDs { + NSMutableArray *result = [NSMutableArray array]; + NSString *userID = self.userID; + // Given an ordered set of unique batchIDs perform a skipping scan over the main table to find // the mutation batches. auto mutationIterator = _db.currentTransaction->NewIterator(); - - for (FSTBatchID batchID : uniqueBatchIds) { + for (FSTBatchID batchID : batchIDs) { std::string mutationKey = [FSTLevelDBMutationKey keyWithUserID:userID batchID:batchID]; mutationIterator->Seek(mutationKey); if (!mutationIterator->Valid() || mutationIterator->key() != mutationKey) { diff --git a/Firestore/Source/Local/FSTMemoryMutationQueue.mm b/Firestore/Source/Local/FSTMemoryMutationQueue.mm index 8faf8933571..30caa9518b5 100644 --- a/Firestore/Source/Local/FSTMemoryMutationQueue.mm +++ b/Firestore/Source/Local/FSTMemoryMutationQueue.mm @@ -16,6 +16,8 @@ #import "Firestore/Source/Local/FSTMemoryMutationQueue.h" +#include + #import "Firestore/Source/Core/FSTQuery.h" #import "Firestore/Source/Local/FSTDocumentReference.h" #import "Firestore/Source/Local/FSTMemoryPersistence.h" @@ -28,6 +30,7 @@ #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" using firebase::firestore::model::DocumentKey; +using firebase::firestore::model::DocumentKeySet; using firebase::firestore::model::ResourcePath; NS_ASSUME_NONNULL_BEGIN @@ -242,6 +245,28 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID return result; } +- (NSArray *)allMutationBatchesAffectingDocumentKeys: + (const DocumentKeySet &)documentKeys { + // First find the set of affected batch IDs. + __block std::set batchIDs; + for (const DocumentKey &key : documentKeys) { + FSTDocumentReference *start = [[FSTDocumentReference alloc] initWithKey:key ID:0]; + + FSTDocumentReferenceBlock block = ^(FSTDocumentReference *reference, BOOL *stop) { + if (![key isEqualToKey:reference.key]) { + *stop = YES; + return; + } + + batchIDs.insert(reference.ID); + }; + + [self.batchesByDocumentKey enumerateObjectsFrom:start to:nil usingBlock:block]; + } + + return [self allMutationBatchesWithBatchIDs:batchIDs]; +} + - (NSArray *)allMutationBatchesAffectingQuery:(FSTQuery *)query { // Use the query path as a prefix for testing if a document matches the query. const ResourcePath &prefix = query.path; @@ -258,8 +283,7 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID [[FSTDocumentReference alloc] initWithKey:DocumentKey{startPath} ID:0]; // Find unique batchIDs referenced by all documents potentially matching the query. - __block FSTImmutableSortedSet *uniqueBatchIDs = - [FSTImmutableSortedSet setWithComparator:NumberComparator]; + __block std::set uniqueBatchIDs; FSTDocumentReferenceBlock block = ^(FSTDocumentReference *reference, BOOL *stop) { const ResourcePath &rowKeyPath = reference.key.path(); if (!prefix.IsPrefixOf(rowKeyPath)) { @@ -274,19 +298,26 @@ - (nullable FSTMutationBatch *)nextMutationBatchAfterBatchID:(FSTBatchID)batchID return; } - uniqueBatchIDs = [uniqueBatchIDs setByAddingObject:@(reference.ID)]; + uniqueBatchIDs.insert(reference.ID); }; [self.batchesByDocumentKey enumerateObjectsFrom:start to:nil usingBlock:block]; - // Construct an array of matching batches, sorted by batchID to ensure that multiple mutations - // affecting the same document key are applied in order. + return [self allMutationBatchesWithBatchIDs:uniqueBatchIDs]; +} + +/** + * Constructs an array of matching batches, sorted by batchID to ensure that multiple mutations + * affecting the same document key are applied in order. + */ +- (NSArray *)allMutationBatchesWithBatchIDs: + (const std::set &)batchIDs { NSMutableArray *result = [NSMutableArray array]; - [uniqueBatchIDs enumerateObjectsUsingBlock:^(NSNumber *batchID, BOOL *stop) { - FSTMutationBatch *batch = [self lookupMutationBatch:[batchID intValue]]; + for (FSTBatchID batchID : batchIDs) { + FSTMutationBatch *batch = [self lookupMutationBatch:batchID]; if (batch) { [result addObject:batch]; } - }]; + }; return result; } diff --git a/Firestore/Source/Local/FSTMutationQueue.h b/Firestore/Source/Local/FSTMutationQueue.h index 89b1a529fa6..7d117b50577 100644 --- a/Firestore/Source/Local/FSTMutationQueue.h +++ b/Firestore/Source/Local/FSTMutationQueue.h @@ -20,6 +20,7 @@ #import "Firestore/Source/Local/FSTGarbageCollector.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" +#include "Firestore/core/src/firebase/firestore/model/document_key_set.h" @class FSTMutation; @class FSTMutationBatch; @@ -115,10 +116,21 @@ NS_ASSUME_NONNULL_BEGIN * don't contain the document key at all if it's convenient. */ // TODO(mcg): This should really return an NSEnumerator -// also for b/32992024, all backing stores should really index by document key - (NSArray *)allMutationBatchesAffectingDocumentKey: (const firebase::firestore::model::DocumentKey &)documentKey; +/** + * Finds all mutation batches that could @em possibly affect the given document keys. Not all + * mutations in a batch will necessarily affect each key, so when looping through the batches you'll + * need to check that the mutation itself matches the key. + * + * Note that because of this requirement implementations are free to return mutation batches that + * don't contain any of the given document keys at all if it's convenient. + */ +// TODO(mcg): This should really return an NSEnumerator +- (NSArray *)allMutationBatchesAffectingDocumentKeys: + (const firebase::firestore::model::DocumentKeySet &)documentKeys; + /** * Finds all mutation batches that could affect the results for the given query. Not all * mutations in a batch will necessarily affect the query, so when looping through the batch From 36d8596e940046cd43c9e5da275a44b50c70ba90 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 9 Jul 2018 12:24:17 -0400 Subject: [PATCH 3/8] Initial --- .../Integration/API/FIRWriteBatchTests.mm | 50 +++++++++++++++++++ .../Source/Local/FSTLocalDocumentsView.mm | 46 ++++++++++++----- 2 files changed, 83 insertions(+), 13 deletions(-) diff --git a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm index 5340873212d..eb9a61f34dd 100644 --- a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm @@ -17,6 +17,7 @@ #import #import +#include #import "Firestore/Example/Tests/Util/FSTEventAccumulator.h" #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h" @@ -323,4 +324,53 @@ - (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. +long long 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 long long memoryUsedBeforeCommitMb = getCurrentMemoryUsedInMb(); + XCTAssertNotEqual(memoryUsedBeforeCommitMb, -1); + [batch commitWithCompletion:^(NSError *_Nullable error) { + XCTAssertNil(error); + const long long memoryUsedAfterCommitMb = getCurrentMemoryUsedInMb(); + XCTAssertNotEqual(memoryUsedAfterCommitMb, -1); + const long long memoryDeltaMb = memoryUsedAfterCommitMb - memoryUsedBeforeCommitMb; + // This by its nature cannot be a precise value. 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..cc5b3e9b913 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 *affectingBatches = + [self.mutationQueue allMutationBatchesAffectingDocumentKey:key]; + return [self documentForKey:key withAffectingBatches:affectingBatches]; +} + +// Internal version of documentForKey: which allows reusing `affectingBatches`. +- (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key + withAffectingBatches:(NSArray *)affectingBatches { FSTMaybeDocument *_Nullable remoteDoc = [self.remoteDocumentCache entryForKey:key]; - return [self localDocument:remoteDoc key:key]; + return [self localDocument:remoteDoc key:key affectingBatches:affectingBatches]; } - (FSTMaybeDocumentDictionary *)documentsForKeys:(const DocumentKeySet &)keys { FSTMaybeDocumentDictionary *results = [FSTMaybeDocumentDictionary maybeDocumentDictionary]; + NSArray *affectingBatches = + [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 withAffectingBatches:affectingBatches]; // 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,10 +163,9 @@ - (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]; - for (FSTMutationBatch *batch in batches) { + key:(const DocumentKey &)documentKey + affectingBatches:(NSArray *)affectingBatches { + for (FSTMutationBatch *batch in affectingBatches) { document = [batch applyTo:document documentKey:documentKey]; } @@ -169,10 +180,19 @@ - (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 *affectingBatches = + [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 affectingBatches:affectingBatches]; if ([mutatedDoc isKindOfClass:[FSTDeletedDocument class]]) { result = [result dictionaryByRemovingObjectForKey:key]; } else if ([mutatedDoc isKindOfClass:[FSTDocument class]]) { From b70157cdfeb5bc318541ac5e6910e4f989e7f4ec Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 9 Jul 2018 18:36:39 -0400 Subject: [PATCH 4/8] Review feedback --- .../Firestore.xcodeproj/project.pbxproj | 6 +++-- .../Integration/API/FIRWriteBatchTests.mm | 13 ++++++----- .../Source/Local/FSTLocalDocumentsView.mm | 22 +++++++++---------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 48c63d1556d..c1a0d6c0182 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -283,7 +283,7 @@ 132E36BB104830BD806351AC /* FSTLevelDBTransactionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLevelDBTransactionTests.mm; sourceTree = ""; }; 2A0CF41BA5AED6049B0BEB2C /* type_traits_apple_test.mm */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.objcpp; path = type_traits_apple_test.mm; sourceTree = ""; }; 2B50B3A0DF77100EEE887891 /* Pods_Firestore_Tests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Tests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; - 358C3B5FE573B1D60A4F7592 /* strerror_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = strerror_test.cc; sourceTree = ""; }; + 358C3B5FE573B1D60A4F7592 /* strerror_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = strerror_test.cc; sourceTree = ""; }; 379B34A1536045869826D82A /* Pods_Firestore_Example_iOS_SwiftBuildTest.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Example_iOS_SwiftBuildTest.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = remote_store_spec_test.json; sourceTree = ""; }; 3C81DE3772628FE297055662 /* Pods-Firestore_Example_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS.debug.xcconfig"; sourceTree = ""; }; @@ -517,7 +517,7 @@ E592181BFD7C53C305123739 /* Pods-Firestore_Tests_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Tests_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Tests_iOS/Pods-Firestore_Tests_iOS.debug.xcconfig"; sourceTree = ""; }; ECEBABC7E7B693BE808A1052 /* Pods_Firestore_IntegrationTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_IntegrationTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; F354C0FE92645B56A6C6FD44 /* Pods-Firestore_IntegrationTests_iOS.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_IntegrationTests_iOS.release.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_IntegrationTests_iOS/Pods-Firestore_IntegrationTests_iOS.release.xcconfig"; sourceTree = ""; }; - F8043813A5D16963EC02B182 /* local_serializer_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = local_serializer_test.cc; sourceTree = ""; }; + F8043813A5D16963EC02B182 /* local_serializer_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = local_serializer_test.cc; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -2124,6 +2124,7 @@ isa = XCBuildConfiguration; baseConfigurationReference = 3C81DE3772628FE297055662 /* Pods-Firestore_Example_iOS.debug.xcconfig */; buildSettings = { + ARCHS = "$(ARCHS_STANDARD)"; ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; GCC_PRECOMPILE_PREFIX_HEADER = YES; GCC_PREFIX_HEADER = ""; @@ -2148,6 +2149,7 @@ isa = XCBuildConfiguration; baseConfigurationReference = 3F0992A4B83C60841C52E960 /* Pods-Firestore_Example_iOS.release.xcconfig */; buildSettings = { + ARCHS = "$(ARCHS_STANDARD)"; ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; GCC_PRECOMPILE_PREFIX_HEADER = YES; GCC_PREFIX_HEADER = ""; diff --git a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm index eb9a61f34dd..05710aa059d 100644 --- a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm @@ -19,6 +19,8 @@ #import #include +#include + #import "Firestore/Example/Tests/Util/FSTEventAccumulator.h" #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h" @@ -328,7 +330,7 @@ - (void)testUpdateNestedFields { // 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. -long long getCurrentMemoryUsedInMb() { +std::int64_t GetCurrentMemoryUsedInMb() { mach_task_basic_info taskInfo; mach_msg_type_number_t taskInfoSize = MACH_TASK_BASIC_INFO_COUNT; const auto errorCode = @@ -358,14 +360,15 @@ - (void)testReasonableMemoryUsageForLotsOfMutations { forDocument:nestedDoc]; } - const long long memoryUsedBeforeCommitMb = getCurrentMemoryUsedInMb(); + const std::int64_t memoryUsedBeforeCommitMb = GetCurrentMemoryUsedInMb(); XCTAssertNotEqual(memoryUsedBeforeCommitMb, -1); [batch commitWithCompletion:^(NSError *_Nullable error) { XCTAssertNil(error); - const long long memoryUsedAfterCommitMb = getCurrentMemoryUsedInMb(); + const std::int64_t memoryUsedAfterCommitMb = GetCurrentMemoryUsedInMb(); XCTAssertNotEqual(memoryUsedAfterCommitMb, -1); - const long long memoryDeltaMb = memoryUsedAfterCommitMb - memoryUsedBeforeCommitMb; - // This by its nature cannot be a precise value. A regression would be on the scale of 500Mb. + const std::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]; diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index cc5b3e9b913..f44da9db7b0 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -62,25 +62,25 @@ - (instancetype)initWithRemoteDocumentCache:(id)remoteDo } - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key { - NSArray *affectingBatches = + NSArray *batches = [self.mutationQueue allMutationBatchesAffectingDocumentKey:key]; - return [self documentForKey:key withAffectingBatches:affectingBatches]; + return [self documentForKey:key inBatches:batches]; } -// Internal version of documentForKey: which allows reusing `affectingBatches`. +// Internal version of documentForKey: which allows reusing `batches`. - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key - withAffectingBatches:(NSArray *)affectingBatches { + inBatches:(NSArray *)batches { FSTMaybeDocument *_Nullable remoteDoc = [self.remoteDocumentCache entryForKey:key]; - return [self localDocument:remoteDoc key:key affectingBatches:affectingBatches]; + return [self localDocument:remoteDoc key:key inBatches:batches]; } - (FSTMaybeDocumentDictionary *)documentsForKeys:(const DocumentKeySet &)keys { FSTMaybeDocumentDictionary *results = [FSTMaybeDocumentDictionary maybeDocumentDictionary]; - NSArray *affectingBatches = + 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 withAffectingBatches:affectingBatches]; + 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()]; @@ -164,8 +164,8 @@ - (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { */ - (nullable FSTMaybeDocument *)localDocument:(nullable FSTMaybeDocument *)document key:(const DocumentKey &)documentKey - affectingBatches:(NSArray *)affectingBatches { - for (FSTMutationBatch *batch in affectingBatches) { + inBatches:(NSArray *)batches { + for (FSTMutationBatch *batch in batches) { document = [batch applyTo:document documentKey:documentKey]; } @@ -185,14 +185,14 @@ - (FSTDocumentDictionary *)localDocuments:(FSTDocumentDictionary *)documents { enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTDocument *doc, BOOL *stop) { keySet = keySet.insert(doc.key); }]; - NSArray *affectingBatches = + 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 affectingBatches:affectingBatches]; + [self localDocument:remoteDocument key:key inBatches:batches]; if ([mutatedDoc isKindOfClass:[FSTDeletedDocument class]]) { result = [result dictionaryByRemovingObjectForKey:key]; } else if ([mutatedDoc isKindOfClass:[FSTDocument class]]) { From 19d862aa51b684910870b9fef68dda095a4b2eea Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 9 Jul 2018 18:37:20 -0400 Subject: [PATCH 5/8] style.sh --- Firestore/Source/Local/FSTLocalDocumentsView.mm | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Firestore/Source/Local/FSTLocalDocumentsView.mm b/Firestore/Source/Local/FSTLocalDocumentsView.mm index f44da9db7b0..43378790a81 100644 --- a/Firestore/Source/Local/FSTLocalDocumentsView.mm +++ b/Firestore/Source/Local/FSTLocalDocumentsView.mm @@ -69,7 +69,7 @@ - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key { // Internal version of documentForKey: which allows reusing `batches`. - (nullable FSTMaybeDocument *)documentForKey:(const DocumentKey &)key - inBatches:(NSArray *)batches { + inBatches:(NSArray *)batches { FSTMaybeDocument *_Nullable remoteDoc = [self.remoteDocumentCache entryForKey:key]; return [self localDocument:remoteDoc key:key inBatches:batches]; } @@ -164,7 +164,7 @@ - (FSTDocumentDictionary *)documentsMatchingCollectionQuery:(FSTQuery *)query { */ - (nullable FSTMaybeDocument *)localDocument:(nullable FSTMaybeDocument *)document key:(const DocumentKey &)documentKey - inBatches:(NSArray *)batches { + inBatches:(NSArray *)batches { for (FSTMutationBatch *batch in batches) { document = [batch applyTo:document documentKey:documentKey]; } @@ -191,8 +191,7 @@ - (FSTDocumentDictionary *)localDocuments:(FSTDocumentDictionary *)documents { __block FSTDocumentDictionary *result = documents; [documents enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey *key, FSTDocument *remoteDocument, BOOL *stop) { - FSTMaybeDocument *mutatedDoc = - [self localDocument:remoteDocument key:key inBatches:batches]; + FSTMaybeDocument *mutatedDoc = [self localDocument:remoteDocument key:key inBatches:batches]; if ([mutatedDoc isKindOfClass:[FSTDeletedDocument class]]) { result = [result dictionaryByRemovingObjectForKey:key]; } else if ([mutatedDoc isKindOfClass:[FSTDocument class]]) { From 8283a7b8f9bbcc4ef428282e95d603d05954b31b Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 9 Jul 2018 18:38:53 -0400 Subject: [PATCH 6/8] undo change to project file --- Firestore/Example/Firestore.xcodeproj/project.pbxproj | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index c1a0d6c0182..d15e5906a2a 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -165,6 +165,7 @@ 6EDD3B4820BF247500C33877 /* UIKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6003F591195388D20070C39A /* UIKit.framework */; }; 6EDD3B4920BF247500C33877 /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 6003F5AF195388D20070C39A /* XCTest.framework */; }; 6EDD3B6020BF25AE00C33877 /* FSTFuzzTestsPrincipal.mm in Sources */ = {isa = PBXBuildFile; fileRef = 6EDD3B5E20BF24D000C33877 /* FSTFuzzTestsPrincipal.mm */; }; + 6F3CAC76D918D6B0917EDF92 /* query_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B9C261C26C5D311E1E3C0CB9 /* query_test.cc */; }; 71719F9F1E33DC2100824A3D /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 71719F9D1E33DC2100824A3D /* LaunchScreen.storyboard */; }; 7346E61D20325C6900FD6CEF /* FSTDispatchQueueTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 7346E61C20325C6900FD6CEF /* FSTDispatchQueueTests.mm */; }; 73866AA12082B0A5009BB4FF /* FIRArrayTransformTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 73866A9F2082B069009BB4FF /* FIRArrayTransformTests.mm */; }; @@ -283,7 +284,7 @@ 132E36BB104830BD806351AC /* FSTLevelDBTransactionTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLevelDBTransactionTests.mm; sourceTree = ""; }; 2A0CF41BA5AED6049B0BEB2C /* type_traits_apple_test.mm */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.objcpp; path = type_traits_apple_test.mm; sourceTree = ""; }; 2B50B3A0DF77100EEE887891 /* Pods_Firestore_Tests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Tests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; - 358C3B5FE573B1D60A4F7592 /* strerror_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = strerror_test.cc; sourceTree = ""; }; + 358C3B5FE573B1D60A4F7592 /* strerror_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = strerror_test.cc; sourceTree = ""; }; 379B34A1536045869826D82A /* Pods_Firestore_Example_iOS_SwiftBuildTest.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Example_iOS_SwiftBuildTest.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = remote_store_spec_test.json; sourceTree = ""; }; 3C81DE3772628FE297055662 /* Pods-Firestore_Example_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS.debug.xcconfig"; sourceTree = ""; }; @@ -496,6 +497,7 @@ B6FB4689208F9B9100554BA2 /* executor_libdispatch_test.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = executor_libdispatch_test.mm; sourceTree = ""; }; B6FB468A208F9B9100554BA2 /* executor_test.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = executor_test.h; sourceTree = ""; }; B79CA87A1A01FC5329031C9B /* Pods_Firestore_FuzzTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_FuzzTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; + B9C261C26C5D311E1E3C0CB9 /* query_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = query_test.cc; sourceTree = ""; }; BB92EB03E3F92485023F64ED /* Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; D3CC3DC5338DCAF43A211155 /* README.md */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = net.daringfireball.markdown; name = README.md; path = ../README.md; sourceTree = ""; }; DE03B2E91F2149D600A30B9C /* Firestore_IntegrationTests_iOS.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = Firestore_IntegrationTests_iOS.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -517,7 +519,7 @@ E592181BFD7C53C305123739 /* Pods-Firestore_Tests_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Tests_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Tests_iOS/Pods-Firestore_Tests_iOS.debug.xcconfig"; sourceTree = ""; }; ECEBABC7E7B693BE808A1052 /* Pods_Firestore_IntegrationTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_IntegrationTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; F354C0FE92645B56A6C6FD44 /* Pods-Firestore_IntegrationTests_iOS.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_IntegrationTests_iOS.release.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_IntegrationTests_iOS/Pods-Firestore_IntegrationTests_iOS.release.xcconfig"; sourceTree = ""; }; - F8043813A5D16963EC02B182 /* local_serializer_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = local_serializer_test.cc; sourceTree = ""; }; + F8043813A5D16963EC02B182 /* local_serializer_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = local_serializer_test.cc; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -958,6 +960,7 @@ isa = PBXGroup; children = ( AB38D92E20235D22000A432D /* database_info_test.cc */, + B9C261C26C5D311E1E3C0CB9 /* query_test.cc */, AB380CF82019382300D97691 /* target_id_generator_test.cc */, ); path = core; @@ -1824,6 +1827,7 @@ 5A080105CCBFDB6BF3F3772D /* path_test.cc in Sources */, 549CCA5920A36E1F00BCEB75 /* precondition_test.cc in Sources */, 618BBEAB20B89AAC00B5BCE7 /* query.pb.cc in Sources */, + 6F3CAC76D918D6B0917EDF92 /* query_test.cc in Sources */, B686F2B22025000D0028D6BE /* resource_path_test.cc in Sources */, 54740A571FC914BA00713A1A /* secure_random_test.cc in Sources */, 61F72C5620BC48FD001A68CB /* serializer_test.cc in Sources */, @@ -2124,7 +2128,6 @@ isa = XCBuildConfiguration; baseConfigurationReference = 3C81DE3772628FE297055662 /* Pods-Firestore_Example_iOS.debug.xcconfig */; buildSettings = { - ARCHS = "$(ARCHS_STANDARD)"; ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; GCC_PRECOMPILE_PREFIX_HEADER = YES; GCC_PREFIX_HEADER = ""; @@ -2149,7 +2152,6 @@ isa = XCBuildConfiguration; baseConfigurationReference = 3F0992A4B83C60841C52E960 /* Pods-Firestore_Example_iOS.release.xcconfig */; buildSettings = { - ARCHS = "$(ARCHS_STANDARD)"; ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; GCC_PRECOMPILE_PREFIX_HEADER = YES; GCC_PREFIX_HEADER = ""; From 7b366a14a5c509428a5cd7eae12aff7aa8967ee1 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 9 Jul 2018 19:06:20 -0400 Subject: [PATCH 7/8] Add to system directories --- scripts/cpplint.py | 1 + 1 file changed, 1 insertion(+) 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', ]) From e29283faa4759a2e11382a7fe1da129d8cfaf6bd Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 9 Jul 2018 21:30:17 -0400 Subject: [PATCH 8/8] Review feedback --- .../Example/Tests/Integration/API/FIRWriteBatchTests.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm index 05710aa059d..63616aad9fd 100644 --- a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm @@ -330,7 +330,7 @@ - (void)testUpdateNestedFields { // 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. -std::int64_t GetCurrentMemoryUsedInMb() { +int64_t GetCurrentMemoryUsedInMb() { mach_task_basic_info taskInfo; mach_msg_type_number_t taskInfoSize = MACH_TASK_BASIC_INFO_COUNT; const auto errorCode = @@ -360,13 +360,13 @@ - (void)testReasonableMemoryUsageForLotsOfMutations { forDocument:nestedDoc]; } - const std::int64_t memoryUsedBeforeCommitMb = GetCurrentMemoryUsedInMb(); + const int64_t memoryUsedBeforeCommitMb = GetCurrentMemoryUsedInMb(); XCTAssertNotEqual(memoryUsedBeforeCommitMb, -1); [batch commitWithCompletion:^(NSError *_Nullable error) { XCTAssertNil(error); - const std::int64_t memoryUsedAfterCommitMb = GetCurrentMemoryUsedInMb(); + const int64_t memoryUsedAfterCommitMb = GetCurrentMemoryUsedInMb(); XCTAssertNotEqual(memoryUsedAfterCommitMb, -1); - const std::int64_t memoryDeltaMb = memoryUsedAfterCommitMb - memoryUsedBeforeCommitMb; + 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);