Skip to content

Commit aa67e19

Browse files
committed
Change FServerValues to just-in-time read from SyncTrees. (#5183)
* Change FServerValues to just-in-time read from SyncTrees. Based on fix firebase/firebase-js-sdk#2499 to bug firebase/firebase-js-sdk#2487 Micro-benchmarking showed that N writes in succession led to N^2 performance when calcuating the resolved write tree for those writes (thankfully only at the subpath in this SDK). This change matches the optimization in JS that mitigated a 20% performance regression. * Rename types to match JS sdk
1 parent ddb1177 commit aa67e19

File tree

5 files changed

+158
-55
lines changed

5 files changed

+158
-55
lines changed

Example/Database/Tests/Integration/FData.m

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,7 +2874,15 @@ - (void)testServerTimestampTransactionLocalEvents {
28742874
@"Number should be no more than 2 seconds ago");
28752875
}
28762876

2877-
- (void)testServerIncrementOverwritesExistingData {
2877+
- (void)testServerIncrementOverwritesExistingDataOnline {
2878+
[self checkServerIncrementOverwritesExistingDataWhileOnline:true];
2879+
}
2880+
2881+
- (void)testServerIncrementOverwritesExistingDataOffline {
2882+
[self checkServerIncrementOverwritesExistingDataWhileOnline:false];
2883+
}
2884+
2885+
- (void)checkServerIncrementOverwritesExistingDataWhileOnline:(BOOL)online {
28782886
FIRDatabaseReference *ref = [FTestHelpers getRandomNode];
28792887
__block NSMutableArray *found = [NSMutableArray new];
28802888
NSMutableArray *expected = [NSMutableArray new];
@@ -2884,7 +2892,9 @@ - (void)testServerIncrementOverwritesExistingData {
28842892
}];
28852893

28862894
// Going offline ensures that local events get queued up before server events
2887-
[ref.repo interrupt];
2895+
if (!online) {
2896+
[ref.repo interrupt];
2897+
}
28882898

28892899
// null + incr
28902900
[ref setValue:[FIRServerValue increment:@1]];
@@ -2912,11 +2922,25 @@ - (void)testServerIncrementOverwritesExistingData {
29122922
return found.count == expected.count;
29132923
}];
29142924
XCTAssertEqualObjects(expected, found);
2915-
[ref.repo resume];
2925+
2926+
if (!online) {
2927+
[ref.repo resume];
2928+
}
29162929
}
29172930

2918-
- (void)testServerIncrementPriority {
2931+
- (void)testServerIncrementPriorityOnline {
2932+
[self checkServerIncrementPriorityWhileOnline:true];
2933+
}
2934+
2935+
- (void)testServerIncrementPriorityOffline {
2936+
[self checkServerIncrementPriorityWhileOnline:false];
2937+
}
2938+
2939+
- (void)checkServerIncrementPriorityWhileOnline:(BOOL)online {
29192940
FIRDatabaseReference *ref = [FTestHelpers getRandomNode];
2941+
if (!online) {
2942+
[ref.repo interrupt];
2943+
}
29202944
__block NSMutableArray *found = [NSMutableArray new];
29212945
NSMutableArray *expected = [NSMutableArray new];
29222946
[ref observeEventType:FIRDataEventTypeValue
@@ -2926,7 +2950,9 @@ - (void)testServerIncrementPriority {
29262950

29272951
// Going offline ensures that local events get queued up before server events
29282952
// Also necessary because increment may not be live yet in the server.
2929-
[ref.repo interrupt];
2953+
if (!online) {
2954+
[ref.repo interrupt];
2955+
}
29302956

29312957
// null + incr
29322958
[ref setValue:@0 andPriority:[FIRServerValue increment:@1]];
@@ -2938,7 +2964,10 @@ - (void)testServerIncrementPriority {
29382964
return found.count == expected.count;
29392965
}];
29402966
XCTAssertEqualObjects(expected, found);
2941-
[ref.repo resume];
2967+
2968+
if (!online) {
2969+
[ref.repo resume];
2970+
}
29422971
}
29432972

29442973
- (void)testServerIncrementOverflowAndTypeCoercion {

Firebase/Database/Core/FRepo.m

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,7 @@ - (void)restoreWrites {
238238
}
239239
lastWriteId = writeId;
240240
self.writeIdCounter = writeId + 1;
241-
id<FNode> existing =
242-
[self.serverSyncTree calcCompleteEventCacheAtPath:write.path
243-
excludeWriteIds:@[]];
241+
244242
if ([write isOverwrite]) {
245243
FFLog(@"I-RDB038001", @"Restoring overwrite with id %ld",
246244
(long)write.writeId);
@@ -250,7 +248,8 @@ - (void)restoreWrites {
250248
withCallback:callback];
251249
id<FNode> resolved =
252250
[FServerValues resolveDeferredValueSnapshot:write.overwrite
253-
withExisting:existing
251+
withSyncTree:self.serverSyncTree
252+
atPath:write.path
254253
serverValues:serverValues];
255254
[self.serverSyncTree applyUserOverwriteAtPath:write.path
256255
newData:resolved
@@ -262,10 +261,11 @@ - (void)restoreWrites {
262261
[self.connection mergeData:[write.merge valForExport:YES]
263262
forPath:[write.path toString]
264263
withCallback:callback];
265-
FCompoundWrite *resolved =
266-
[FServerValues resolveDeferredValueCompoundWrite:write.merge
267-
withExisting:existing
268-
serverValues:serverValues];
264+
FCompoundWrite *resolved = [FServerValues
265+
resolveDeferredValueCompoundWrite:write.merge
266+
withSyncTree:self.serverSyncTree
267+
atPath:write.path
268+
serverValues:serverValues];
269269
[self.serverSyncTree applyUserMergeAtPath:write.path
270270
changedChildren:resolved
271271
writeId:writeId];
@@ -366,11 +366,10 @@ - (void)update:(FPath *)path
366366
[values description]);
367367
NSDictionary *serverValues =
368368
[FServerValues generateServerValues:self.serverClock];
369-
id<FNode> existing = [self.serverSyncTree calcCompleteEventCacheAtPath:path
370-
excludeWriteIds:@[]];
371369
FCompoundWrite *resolved =
372370
[FServerValues resolveDeferredValueCompoundWrite:nodes
373-
withExisting:existing
371+
withSyncTree:self.serverSyncTree
372+
atPath:path
374373
serverValues:serverValues];
375374

376375
if (!resolved.isEmpty) {

Firebase/Database/Core/FServerValues.h

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,23 @@
1818
#import "FCompoundWrite.h"
1919
#import "FNode.h"
2020
#import "FSparseSnapshotTree.h"
21+
#import "FSyncTree.h"
2122
#import <Foundation/Foundation.h>
2223

2324
@interface FServerValues : NSObject
2425

2526
+ (NSDictionary *)generateServerValues:(id<FClock>)clock;
26-
+ (id)resolveDeferredValueCompoundWrite:(FCompoundWrite *)write
27-
withExisting:(id<FNode>)existing
28-
serverValues:(NSDictionary *)serverValues;
27+
28+
+ (FCompoundWrite *)resolveDeferredValueCompoundWrite:(FCompoundWrite *)write
29+
withSyncTree:(FSyncTree *)tree
30+
atPath:(FPath *)path
31+
serverValues:
32+
(NSDictionary *)serverValues;
33+
+ (id<FNode>)resolveDeferredValueSnapshot:(id<FNode>)node
34+
withSyncTree:(FSyncTree *)existing
35+
atPath:(FPath *)path
36+
serverValues:(NSDictionary *)serverValues;
2937
+ (id<FNode>)resolveDeferredValueSnapshot:(id<FNode>)node
3038
withExisting:(id<FNode>)existing
3139
serverValues:(NSDictionary *)serverValues;
32-
+ (id)resolveDeferredValueTree:(FSparseSnapshotTree *)tree
33-
withExisting:(id<FNode>)node
34-
serverValues:(NSDictionary *)serverValues;
35-
3640
@end

Firebase/Database/Core/FServerValues.m

Lines changed: 98 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,77 @@ BOOL canBeRepresentedAsLong(NSNumber *num) {
3838
return YES;
3939
}
4040

41+
// Running through CompoundWrites for all update paths has been shown to
42+
// be a 20% pessimization in microbenchmarks. This is because it slows
43+
// down by O(N) of the write queue length. To eliminate the performance
44+
// hit, we wrap around existing data of either snapshot or CompoundWrite
45+
// (allowing us to share code) and read from the CompoundWrite only when/where
46+
// we need to calculate an incremented value's prior state.
47+
@protocol ValueProvider <NSObject>
48+
- (id<ValueProvider>)getChild:(NSString *)pathSegment;
49+
- (id<FNode>)value;
50+
@end
51+
52+
@interface DeferredValueProvider : NSObject <ValueProvider>
53+
- (instancetype)initWithSyncTree:(FSyncTree *)tree atPath:(FPath *)path;
54+
- (id<ValueProvider>)getChild:(NSString *)pathSegment;
55+
- (id<FNode>)value;
56+
@property FPath *path;
57+
@property FSyncTree *tree;
58+
@end
59+
60+
@interface ExistingValueProvider : NSObject <ValueProvider>
61+
- (instancetype)initWithSnapshot:(id<FNode>)snapshot;
62+
- (id<ValueProvider>)getChild:(NSString *)pathSegment;
63+
- (id<FNode>)value;
64+
@property id<FNode> snapshot;
65+
@end
66+
67+
@implementation DeferredValueProvider
68+
- (instancetype)initWithSyncTree:(FSyncTree *)tree atPath:(FPath *)path {
69+
self.tree = tree;
70+
self.path = path;
71+
return self;
72+
}
73+
74+
- (id<ValueProvider>)getChild:(NSString *)pathSegment {
75+
FPath *child = [self.path childFromString:pathSegment];
76+
return [[DeferredValueProvider alloc] initWithSyncTree:self.tree
77+
atPath:child];
78+
}
79+
80+
- (id<FNode>)value {
81+
return [self.tree calcCompleteEventCacheAtPath:self.path
82+
excludeWriteIds:@[]];
83+
}
84+
@end
85+
86+
@implementation ExistingValueProvider
87+
- (instancetype)initWithSnapshot:(id<FNode>)snapshot {
88+
self.snapshot = snapshot;
89+
return self;
90+
}
91+
92+
- (id<ValueProvider>)getChild:(NSString *)pathSegment {
93+
return [[ExistingValueProvider alloc]
94+
initWithSnapshot:[self.snapshot getImmediateChild:pathSegment]];
95+
}
96+
97+
- (id<FNode>)value {
98+
return self.snapshot;
99+
}
100+
@end
101+
41102
@interface FServerValues ()
42103
+ (id)resolveScalarServerOp:(NSString *)op
43104
withServerValues:(NSDictionary *)serverValues;
44105
+ (id)resolveComplexServerOp:(NSDictionary *)op
45-
withExisting:(id<FNode>)existing
106+
withExisting:(id<ValueProvider>)existing
46107
serverValues:(NSDictionary *)serverValues;
108+
+ (id<FNode>)resolveDeferredValueSnapshot:(id<FNode>)node
109+
withJITExisting:(id<ValueProvider>)existing
110+
serverValues:(NSDictionary *)serverValues;
111+
47112
@end
48113

49114
@implementation FServerValues
@@ -54,7 +119,7 @@ + (NSDictionary *)generateServerValues:(id<FClock>)clock {
54119
}
55120

56121
+ (id)resolveDeferredValue:(id)val
57-
withExisting:(id<FNode>)existing
122+
withExisting:(id<ValueProvider>)existing
58123
serverValues:(NSDictionary *)serverValues {
59124
if (![val isKindOfClass:[NSDictionary class]]) {
60125
return val;
@@ -81,7 +146,7 @@ + (id)resolveScalarServerOp:(NSString *)op
81146
}
82147

83148
+ (id)resolveComplexServerOp:(NSDictionary *)op
84-
withExisting:(id<FNode>)existing
149+
withExisting:(id<ValueProvider>)jitExisting
85150
serverValues:(NSDictionary *)serverValues {
86151
// Only increment is supported as of now
87152
if (op[kIncrement] == nil) {
@@ -90,6 +155,7 @@ + (id)resolveComplexServerOp:(NSDictionary *)op
90155

91156
// Incrementing a non-number sets the value to the incremented amount
92157
NSNumber *delta = op[kIncrement];
158+
id<FNode> existing = jitExisting.value;
93159
if (![existing isLeafNode]) {
94160
return delta;
95161
}
@@ -117,49 +183,54 @@ + (id)resolveComplexServerOp:(NSDictionary *)op
117183
}
118184

119185
+ (FCompoundWrite *)resolveDeferredValueCompoundWrite:(FCompoundWrite *)write
120-
withExisting:(id<FNode>)existing
186+
withSyncTree:(FSyncTree *)tree
187+
atPath:(FPath *)path
121188
serverValues:
122189
(NSDictionary *)serverValues {
123190
__block FCompoundWrite *resolved = write;
124-
[write enumerateWrites:^(FPath *path, id<FNode> node, BOOL *stop) {
191+
[write enumerateWrites:^(FPath *subPath, id<FNode> node, BOOL *stop) {
192+
id<ValueProvider> existing =
193+
[[DeferredValueProvider alloc] initWithSyncTree:tree
194+
atPath:[path child:subPath]];
125195
id<FNode> resolvedNode =
126196
[FServerValues resolveDeferredValueSnapshot:node
127-
withExisting:existing
197+
withJITExisting:existing
128198
serverValues:serverValues];
129199
// Node actually changed, use pointer inequality here
130200
if (resolvedNode != node) {
131-
resolved = [resolved addWrite:resolvedNode atPath:path];
201+
resolved = [resolved addWrite:resolvedNode atPath:subPath];
132202
}
133203
}];
134204
return resolved;
135205
}
136206

137-
+ (id)resolveDeferredValueTree:(FSparseSnapshotTree *)tree
138-
withExisting:(id<FNode>)existing
139-
serverValues:(NSDictionary *)serverValues {
140-
FSparseSnapshotTree *resolvedTree = [[FSparseSnapshotTree alloc] init];
141-
[tree
142-
forEachTreeAtPath:[FPath empty]
143-
do:^(FPath *path, id<FNode> node) {
144-
[resolvedTree
145-
rememberData:
146-
[FServerValues
147-
resolveDeferredValueSnapshot:node
148-
withExisting:
149-
[existing
150-
getChild:path]
151-
serverValues:serverValues]
152-
onPath:path];
153-
}];
154-
return resolvedTree;
207+
+ (id<FNode>)resolveDeferredValueSnapshot:(id<FNode>)node
208+
withSyncTree:(FSyncTree *)tree
209+
atPath:(FPath *)path
210+
serverValues:(NSDictionary *)serverValues {
211+
id<ValueProvider> jitExisting =
212+
[[DeferredValueProvider alloc] initWithSyncTree:tree atPath:path];
213+
return [FServerValues resolveDeferredValueSnapshot:node
214+
withJITExisting:jitExisting
215+
serverValues:serverValues];
155216
}
156217

157218
+ (id<FNode>)resolveDeferredValueSnapshot:(id<FNode>)node
158219
withExisting:(id<FNode>)existing
159220
serverValues:(NSDictionary *)serverValues {
221+
id<ValueProvider> jitExisting =
222+
[[ExistingValueProvider alloc] initWithSnapshot:existing];
223+
return [FServerValues resolveDeferredValueSnapshot:node
224+
withJITExisting:jitExisting
225+
serverValues:serverValues];
226+
}
227+
228+
+ (id<FNode>)resolveDeferredValueSnapshot:(id<FNode>)node
229+
withJITExisting:(id<ValueProvider>)existing
230+
serverValues:(NSDictionary *)serverValues {
160231
id priorityVal =
161232
[FServerValues resolveDeferredValue:[[node getPriority] val]
162-
withExisting:existing.getPriority
233+
withExisting:[existing getChild:@".priority"]
163234
serverValues:serverValues];
164235
id<FNode> priority = [FSnapshotUtilities nodeFrom:priorityVal];
165236

@@ -184,7 +255,7 @@ + (id)resolveDeferredValueTree:(FSparseSnapshotTree *)tree
184255
id<FNode> childNode, BOOL *stop) {
185256
id newChildNode = [FServerValues
186257
resolveDeferredValueSnapshot:childNode
187-
withExisting:[existing getImmediateChild:childKey]
258+
withJITExisting:[existing getChild:childKey]
188259
serverValues:serverValues];
189260
if (![newChildNode isEqual:childNode]) {
190261
newNode = [newNode updateImmediateChild:childKey

Firebase/Database/Core/FSyncTree.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,19 +231,19 @@ - (NSArray *)ackUserWriteWithWriteId:(NSInteger)writeId
231231
if (!revert) {
232232
NSDictionary *serverValues =
233233
[FServerValues generateServerValues:clock];
234-
id<FNode> existing = [self calcCompleteEventCacheAtPath:write.path
235-
excludeWriteIds:@[]];
236234
if ([write isOverwrite]) {
237235
id<FNode> resolvedNode =
238236
[FServerValues resolveDeferredValueSnapshot:write.overwrite
239-
withExisting:existing
237+
withSyncTree:self
238+
atPath:write.path
240239
serverValues:serverValues];
241240
[self.persistenceManager applyUserWrite:resolvedNode
242241
toServerCacheAtPath:write.path];
243242
} else {
244243
FCompoundWrite *resolvedMerge = [FServerValues
245244
resolveDeferredValueCompoundWrite:write.merge
246-
withExisting:existing
245+
withSyncTree:self
246+
atPath:write.path
247247
serverValues:serverValues];
248248
[self.persistenceManager applyUserMerge:resolvedMerge
249249
toServerCacheAtPath:write.path];

0 commit comments

Comments
 (0)