Skip to content

Commit 8a276f2

Browse files
Don't deserialize full Document proto for Query execution
1 parent 2708307 commit 8a276f2

File tree

13 files changed

+173
-65
lines changed

13 files changed

+173
-65
lines changed

packages/firestore/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
21
# Unreleased
2+
- [changed] Improved performance for queries with filters that only return a
3+
small subset of the documents in a collection.
34
- [fixed] Fixed an internal assertion that was triggered when an update
45
with a `FieldValue.serverTimestamp()` and an update with a
56
`FieldValue.increment()` were pending for the same document.

packages/firestore/src/api/database.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
13111311
return !this._document
13121312
? undefined
13131313
: this.convertObject(
1314-
this._document.data,
1314+
this._document.data(),
13151315
FieldValueOptions.fromSnapshotOptions(
13161316
options,
13171317
this._firestore._areTimestampsInSnapshotsEnabled()
@@ -1326,9 +1326,9 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
13261326
validateBetweenNumberOfArgs('DocumentSnapshot.get', arguments, 1, 2);
13271327
options = validateSnapshotOptions('DocumentSnapshot.get', options);
13281328
if (this._document) {
1329-
const value = this._document.data.field(
1330-
fieldPathFromArgument('DocumentSnapshot.get', fieldPath)
1331-
);
1329+
const value = this._document
1330+
.data()
1331+
.field(fieldPathFromArgument('DocumentSnapshot.get', fieldPath));
13321332
if (value !== null) {
13331333
return this.convertValue(
13341334
value,

packages/firestore/src/core/query.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ export class Bound {
759759
} else {
760760
const docValue = doc.field(orderByComponent.field);
761761
assert(
762-
docValue !== undefined,
762+
docValue !== null,
763763
'Field should exist since document matched the orderBy already.'
764764
);
765765
comparison = component.compareTo(docValue!);

packages/firestore/src/core/view.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ export class View {
169169

170170
// Calculate change
171171
if (oldDoc && newDoc) {
172-
const docsEqual = oldDoc.data.isEqual(newDoc.data);
172+
const docsEqual = oldDoc.data().isEqual(newDoc.data());
173173
if (!docsEqual) {
174174
if (!this.shouldWaitForSyncedDocument(oldDoc, newDoc)) {
175175
changeSet.track({

packages/firestore/src/model/document.ts

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@
1616
*/
1717

1818
import { SnapshotVersion } from '../core/snapshot_version';
19-
import { fail } from '../util/assert';
19+
import { assert, fail } from '../util/assert';
2020

2121
import { DocumentKey } from './document_key';
2222
import { FieldValue, JsonObject, ObjectValue } from './field_value';
2323
import { FieldPath } from './path';
2424

2525
import * as api from '../protos/firestore_proto_api';
26+
import * as obj from '../util/obj';
2627

2728
export interface DocumentOptions {
2829
hasLocalMutations?: boolean;
@@ -59,43 +60,111 @@ export class Document extends MaybeDocument {
5960
readonly hasLocalMutations: boolean;
6061
readonly hasCommittedMutations: boolean;
6162

63+
/**
64+
* A cache of canonicalized FieldPaths to FieldValues that have already been
65+
* deserialized in `getField()`.
66+
*/
67+
private fieldValueCache?: Map<string, FieldValue | null>;
68+
6269
constructor(
6370
key: DocumentKey,
6471
version: SnapshotVersion,
65-
readonly data: ObjectValue,
6672
options: DocumentOptions,
67-
/**
68-
* Memoized serialized form of the document for optimization purposes (avoids repeated
69-
* serialization). Might be undefined.
70-
*/
71-
readonly proto?: api.Document
73+
private objectValue?: ObjectValue,
74+
readonly proto?: api.Document,
75+
private readonly converter?: (value: api.Value) => FieldValue
7276
) {
7377
super(key, version);
7478
this.hasLocalMutations = !!options.hasLocalMutations;
7579
this.hasCommittedMutations = !!options.hasCommittedMutations;
7680
}
7781

7882
field(path: FieldPath): FieldValue | null {
79-
return this.data.field(path);
83+
if (this.objectValue) {
84+
return this.objectValue.field(path);
85+
} else {
86+
assert(
87+
this.proto !== undefined && this.converter !== undefined,
88+
'Expected proto and converter to be defined.'
89+
);
90+
91+
if (!this.fieldValueCache) {
92+
// TODO(b/136090445): Remove the cache when `getField` is no longer
93+
// called during Query ordering.
94+
this.fieldValueCache = new Map<string, FieldValue>();
95+
}
96+
97+
const canonicalPath = path.canonicalString();
98+
99+
let fieldValue = this.fieldValueCache.get(canonicalPath);
100+
101+
if (fieldValue === undefined) {
102+
// Instead of deserializing the full Document proto, we only
103+
// deserialize the value at the requested field path. This speeds up
104+
// Query execution as query filters can discard documents based on a
105+
// single field.
106+
let protoValue: api.Value | undefined = this.proto!.fields[
107+
path.firstSegment()
108+
];
109+
for (let i = 1; protoValue !== undefined && i < path.length; ++i) {
110+
if (protoValue.mapValue) {
111+
protoValue = protoValue.mapValue.fields[path.get(i)];
112+
} else {
113+
protoValue = undefined;
114+
}
115+
}
116+
117+
if (protoValue === undefined) {
118+
fieldValue = null;
119+
} else {
120+
fieldValue = this.converter!(protoValue);
121+
}
122+
123+
this.fieldValueCache.set(canonicalPath, fieldValue);
124+
}
125+
126+
return fieldValue!;
127+
}
80128
}
81129

82130
fieldValue(path: FieldPath): unknown {
83131
const field = this.field(path);
84132
return field ? field.value() : undefined;
85133
}
86134

135+
data(): ObjectValue {
136+
if (!this.objectValue) {
137+
assert(
138+
this.proto !== undefined && this.converter !== undefined,
139+
'Expected proto and converter to be defined.'
140+
);
141+
142+
let result = ObjectValue.EMPTY;
143+
obj.forEach(this.proto!.fields, (key: string, value: api.Value) => {
144+
result = result.set(new FieldPath([key]), this.converter!(value));
145+
});
146+
this.objectValue = result;
147+
148+
// Once objectValue is computed, values inside the fieldValueCache are no
149+
// longer accessed.
150+
this.fieldValueCache = undefined;
151+
}
152+
153+
return this.objectValue;
154+
}
155+
87156
value(): JsonObject<unknown> {
88-
return this.data.value();
157+
return this.data().value();
89158
}
90159

91160
isEqual(other: MaybeDocument | null | undefined): boolean {
92161
return (
93162
other instanceof Document &&
94163
this.key.isEqual(other.key) &&
95164
this.version.isEqual(other.version) &&
96-
this.data.isEqual(other.data) &&
97165
this.hasLocalMutations === other.hasLocalMutations &&
98-
this.hasCommittedMutations === other.hasCommittedMutations
166+
this.hasCommittedMutations === other.hasCommittedMutations &&
167+
this.data().isEqual(other.data())
99168
);
100169
}
101170

packages/firestore/src/model/mutation.ts

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,14 @@ export class SetMutation extends Mutation {
353353
// have held.
354354

355355
const version = mutationResult.version;
356-
return new Document(this.key, version, this.value, {
357-
hasCommittedMutations: true
358-
});
356+
return new Document(
357+
this.key,
358+
version,
359+
{
360+
hasCommittedMutations: true
361+
},
362+
this.value
363+
);
359364
}
360365

361366
applyToLocalView(
@@ -370,9 +375,14 @@ export class SetMutation extends Mutation {
370375
}
371376

372377
const version = Mutation.getPostMutationVersion(maybeDoc);
373-
return new Document(this.key, version, this.value, {
374-
hasLocalMutations: true
375-
});
378+
return new Document(
379+
this.key,
380+
version,
381+
{
382+
hasLocalMutations: true
383+
},
384+
this.value
385+
);
376386
}
377387

378388
extractBaseValue(maybeDoc: MaybeDocument | null): null {
@@ -434,9 +444,14 @@ export class PatchMutation extends Mutation {
434444
}
435445

436446
const newData = this.patchDocument(maybeDoc);
437-
return new Document(this.key, mutationResult.version, newData, {
438-
hasCommittedMutations: true
439-
});
447+
return new Document(
448+
this.key,
449+
mutationResult.version,
450+
{
451+
hasCommittedMutations: true
452+
},
453+
newData
454+
);
440455
}
441456

442457
applyToLocalView(
@@ -452,9 +467,14 @@ export class PatchMutation extends Mutation {
452467

453468
const version = Mutation.getPostMutationVersion(maybeDoc);
454469
const newData = this.patchDocument(maybeDoc);
455-
return new Document(this.key, version, newData, {
456-
hasLocalMutations: true
457-
});
470+
return new Document(
471+
this.key,
472+
version,
473+
{
474+
hasLocalMutations: true
475+
},
476+
newData
477+
);
458478
}
459479

460480
extractBaseValue(maybeDoc: MaybeDocument | null): null {
@@ -478,7 +498,7 @@ export class PatchMutation extends Mutation {
478498
private patchDocument(maybeDoc: MaybeDocument | null): ObjectValue {
479499
let data: ObjectValue;
480500
if (maybeDoc instanceof Document) {
481-
data = maybeDoc.data;
501+
data = maybeDoc.data();
482502
} else {
483503
data = ObjectValue.EMPTY;
484504
}
@@ -550,10 +570,15 @@ export class TransformMutation extends Mutation {
550570
);
551571

552572
const version = mutationResult.version;
553-
const newData = this.transformObject(doc.data, transformResults);
554-
return new Document(this.key, version, newData, {
555-
hasCommittedMutations: true
556-
});
573+
const newData = this.transformObject(doc.data(), transformResults);
574+
return new Document(
575+
this.key,
576+
version,
577+
{
578+
hasCommittedMutations: true
579+
},
580+
newData
581+
);
557582
}
558583

559584
applyToLocalView(
@@ -573,10 +598,15 @@ export class TransformMutation extends Mutation {
573598
maybeDoc,
574599
baseDoc
575600
);
576-
const newData = this.transformObject(doc.data, transformResults);
577-
return new Document(this.key, doc.version, newData, {
578-
hasLocalMutations: true
579-
});
601+
const newData = this.transformObject(doc.data(), transformResults);
602+
return new Document(
603+
this.key,
604+
doc.version,
605+
{
606+
hasLocalMutations: true
607+
},
608+
newData
609+
);
580610
}
581611

582612
extractBaseValue(maybeDoc: MaybeDocument | null): ObjectValue | null {

packages/firestore/src/remote/serializer.ts

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ export class JsonProtoSerializer {
519519
);
520520
return {
521521
name: this.toName(document.key),
522-
fields: this.toFields(document.data),
522+
fields: this.toFields(document.data()),
523523
updateTime: this.toTimestamp(document.version.toTimestamp())
524524
};
525525
}
@@ -528,11 +528,15 @@ export class JsonProtoSerializer {
528528
document: api.Document,
529529
hasCommittedMutations?: boolean
530530
): Document {
531+
const key = this.fromName(document.name!);
532+
const version = this.fromVersion(document.updateTime!);
531533
return new Document(
532-
this.fromName(document.name!),
533-
this.fromVersion(document.updateTime!),
534-
this.fromFields(document.fields || {}),
535-
{ hasCommittedMutations: !!hasCommittedMutations }
534+
key,
535+
version,
536+
{ hasCommittedMutations: !!hasCommittedMutations },
537+
undefined,
538+
document,
539+
v => this.fromValue(v)
536540
);
537541
}
538542

@@ -577,8 +581,9 @@ export class JsonProtoSerializer {
577581
assertPresent(doc.found!.updateTime, 'doc.found.updateTime');
578582
const key = this.fromName(doc.found!.name!);
579583
const version = this.fromVersion(doc.found!.updateTime!);
580-
const fields = this.fromFields(doc.found!.fields || {});
581-
return new Document(key, version, fields, {}, doc.found!);
584+
return new Document(key, version, {}, undefined, doc.found!, v =>
585+
this.fromValue(v)
586+
);
582587
}
583588

584589
private fromMissing(result: api.BatchGetDocumentsResponse): NoDocument {
@@ -639,7 +644,7 @@ export class JsonProtoSerializer {
639644
documentChange: {
640645
document: {
641646
name: this.toName(doc.key),
642-
fields: this.toFields(doc.data),
647+
fields: this.toFields(doc.data()),
643648
updateTime: this.toVersion(doc.version)
644649
},
645650
targetIds: watchChange.updatedTargetIds,
@@ -718,15 +723,13 @@ export class JsonProtoSerializer {
718723
const entityChange = change.documentChange!;
719724
const key = this.fromName(entityChange.document!.name!);
720725
const version = this.fromVersion(entityChange.document!.updateTime!);
721-
const fields = this.fromFields(entityChange.document!.fields || {});
722-
// The document may soon be re-serialized back to protos in order to store it in local
723-
// persistence. Memoize the encoded form to avoid encoding it again.
724726
const doc = new Document(
725727
key,
726728
version,
727-
fields,
728729
{},
729-
entityChange.document!
730+
undefined,
731+
entityChange.document!,
732+
v => this.fromValue(v)
730733
);
731734
const updatedTargetIds = entityChange.targetIds || [];
732735
const removedTargetIds = entityChange.removedTargetIds || [];

packages/firestore/test/unit/local/lru_garbage_collector.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,8 @@ function genericLruGarbageCollectorTests(
243243
return new Document(
244244
key,
245245
SnapshotVersion.fromMicroseconds(1000),
246-
wrapObject({ foo: 3, bar: false }),
247-
{}
246+
{},
247+
wrapObject({ foo: 3, bar: false })
248248
);
249249
}
250250

@@ -774,8 +774,8 @@ function genericLruGarbageCollectorTests(
774774
const doc = new Document(
775775
middleDocToUpdate,
776776
SnapshotVersion.fromMicroseconds(2000),
777-
wrapObject({ foo: 4, bar: true }),
778-
{}
777+
{},
778+
wrapObject({ foo: 4, bar: true })
779779
);
780780
return saveDocument(txn, doc).next(() => {
781781
return updateTargetInTransaction(txn, middleTarget);

0 commit comments

Comments
 (0)