Skip to content

In FSTLocalDocumentsView, search for all batches affecting a set of keys in one go #1505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 10, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 0 additions & 96 deletions Firestore/Example/Firestore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,6 @@
54C9EDEE2040E16300A969CD /* Frameworks */,
54C9EDEF2040E16300A969CD /* Resources */,
EA424838F4A5DD7B337F57AB /* [CP] Embed Pods Frameworks */,
6BD54D799442CEB09349B73E /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand All @@ -1204,7 +1203,6 @@
6003F587195388D20070C39A /* Frameworks */,
6003F588195388D20070C39A /* Resources */,
1EE692C7509A98D7EB03CA51 /* [CP] Embed Pods Frameworks */,
A4BCE623F5E4C28728E5F17A /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand All @@ -1224,7 +1222,6 @@
6003F5AB195388D20070C39A /* Frameworks */,
6003F5AC195388D20070C39A /* Resources */,
329C25E418360CEF62F6CB2B /* [CP] Embed Pods Frameworks */,
263508FF7FD6CA4D6C3E685D /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand All @@ -1245,7 +1242,6 @@
6EDD3B4520BF247500C33877 /* Frameworks */,
6EDD3B4A20BF247500C33877 /* Resources */,
6EDD3B5720BF247500C33877 /* [CP] Embed Pods Frameworks */,
F5DFA8B0274B042DC1B00837 /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand All @@ -1266,7 +1262,6 @@
DE03B2D31F2149D600A30B9C /* Frameworks */,
DE03B2D81F2149D600A30B9C /* Resources */,
B7923D95031DB0DA112AAE9B /* [CP] Embed Pods Frameworks */,
5B2A669EEE88DF2205316429 /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand All @@ -1287,7 +1282,6 @@
DE0761E11F2FE611003233AF /* Frameworks */,
DE0761E21F2FE611003233AF /* Resources */,
04404E0DCBB886A40E3C7175 /* [CP] Embed Pods Frameworks */,
138396D16F5128E073E667C6 /* [CP] Copy Pods Resources */,
);
buildRules = (
);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down
50 changes: 50 additions & 0 deletions Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#import <FirebaseFirestore/FirebaseFirestore.h>

#import <XCTest/XCTest.h>
#include <mach/mach.h>

#import "Firestore/Example/Tests/Util/FSTEventAccumulator.h"
#import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
Expand Down Expand Up @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid long long. Probably this can be int64_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: there is also something like performance tests in XCTest. I originally had that here as well, but eventually dropped it for this case at least. It seems like a very limited and frankly a clunky feature. It's only possible to measure execution speed, the test is run 10 times which I think is impossible to configure, and to top it all, expected time can only be set in the UI (it is then stored in a file similar to a scheme).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, free functions should start with an initial capital.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the same for both 32 and 64-bit builds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it using two simulators in debug mode:

  • iPhone 8 Plus (64 bit) -- I see increases around 90 MB;
  • iPhone 4S with iOS 9.3 (presumably 32bit) -- I see increases around 50 MB.

I don't have a real device handy, but I can try to find one if you'd like.


[expectation fulfill];
}];
[self awaitExpectations];
}

@end
84 changes: 84 additions & 0 deletions Firestore/Example/Tests/Local/FSTMutationQueueTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -315,6 +318,87 @@ - (void)testAllMutationBatchesAffectingDocumentKey {
});
}

- (void)testAllMutationBatchesAffectingDocumentKeys {
if ([self isTestBaseClass]) return;

self.persistence.run("testAllMutationBatchesAffectingDocumentKey", [&]() {
NSArray<FSTMutation *> *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<FSTMutationBatch *> *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<FSTMutationBatch *> *expected = @[ batches[1], batches[2], batches[4] ];
NSArray<FSTMutationBatch *> *matches =
[self.mutationQueue allMutationBatchesAffectingDocumentKeys:keys];

XCTAssertEqualObjects(matches, expected);
});
}

- (void)testAllMutationBatchesAffectingDocumentKeys_handlesOverlap {
if ([self isTestBaseClass]) return;

self.persistence.run("testAllMutationBatchesAffectingDocumentKeys_handlesOverlap", [&]() {
NSMutableArray<FSTMutationBatch *> *batches = [NSMutableArray array];

NSArray<FSTMutation *> *group1 = @[
FSTTestSetMutation(@"foo/bar",
@{ @"a" : @1 }),
FSTTestSetMutation(@"foo/baz",
@{ @"a" : @1 }),
];
FSTMutationBatch *batch1 =
[self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp]
mutations:group1];

NSArray<FSTMutation *> *group2 = @[ FSTTestSetMutation(@"food/bar", @{ @"a" : @1 }) ];
[self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp] mutations:group2];

NSArray<FSTMutation *> *group3 = @[
FSTTestSetMutation(@"foo/bar",
@{ @"b" : @1 }),
];
FSTMutationBatch *batch3 =
[self.mutationQueue addMutationBatchWithWriteTime:[FIRTimestamp timestamp]
mutations:group3];

DocumentKeySet keys{
Key("foo/bar"),
Key("foo/baz"),
};

NSArray<FSTMutationBatch *> *expected = @[ batch1, batch3 ];
NSArray<FSTMutationBatch *> *matches =
[self.mutationQueue allMutationBatchesAffectingDocumentKeys:keys];

XCTAssertEqualObjects(matches, expected);
});
}

- (void)testAllMutationBatchesAffectingQuery {
if ([self isTestBaseClass]) return;

Expand Down
Loading