Skip to content

Commit c0a86af

Browse files
Use approximate FieldValue size for MemoryLRU
1 parent dac1757 commit c0a86af

File tree

6 files changed

+131
-27
lines changed

6 files changed

+131
-27
lines changed

packages/firestore/src/local/memory_persistence.ts

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@
1616
*/
1717

1818
import { User } from '../auth/user';
19-
import { MaybeDocument } from '../model/document';
19+
import { Document, MaybeDocument } from '../model/document';
2020
import { DocumentKey } from '../model/document_key';
21-
import { JsonProtoSerializer } from '../remote/serializer';
2221
import { fail } from '../util/assert';
2322
import { debug } from '../util/log';
2423
import * as obj from '../util/obj';
2524
import { ObjectMap } from '../util/obj_map';
2625
import { encode } from './encoded_resource_path';
27-
import { LocalSerializer } from './local_serializer';
2826
import {
2927
ActiveTargets,
3028
LruDelegate,
@@ -77,11 +75,10 @@ export class MemoryPersistence implements Persistence {
7775

7876
static createLruPersistence(
7977
clientId: ClientId,
80-
serializer: JsonProtoSerializer,
8178
params: LruParams
8279
): MemoryPersistence {
8380
const factory = (p: MemoryPersistence): MemoryLruDelegate =>
84-
new MemoryLruDelegate(p, new LocalSerializer(serializer), params);
81+
new MemoryLruDelegate(p, params);
8582
return new MemoryPersistence(clientId, factory);
8683
}
8784

@@ -334,7 +331,6 @@ export class MemoryLruDelegate implements ReferenceDelegate, LruDelegate {
334331

335332
constructor(
336333
private readonly persistence: MemoryPersistence,
337-
private readonly serializer: LocalSerializer,
338334
lruParams: LruParams
339335
) {
340336
this.garbageCollector = new LruGarbageCollector(this, lruParams);
@@ -471,21 +467,11 @@ export class MemoryLruDelegate implements ReferenceDelegate, LruDelegate {
471467
}
472468

473469
documentSize(maybeDoc: MaybeDocument): number {
474-
const remoteDocument = this.serializer.toDbRemoteDocument(
475-
maybeDoc,
476-
maybeDoc.version
477-
);
478-
let value: unknown;
479-
if (remoteDocument.document) {
480-
value = remoteDocument.document;
481-
} else if (remoteDocument.unknownDocument) {
482-
value = remoteDocument.unknownDocument;
483-
} else if (remoteDocument.noDocument) {
484-
value = remoteDocument.noDocument;
485-
} else {
486-
throw fail('Unknown remote document type');
470+
let documentSize = maybeDoc.key.toString().length;
471+
if (maybeDoc instanceof Document) {
472+
documentSize += maybeDoc.data().byteSize();
487473
}
488-
return JSON.stringify(value).length;
474+
return documentSize;
489475
}
490476

491477
private isPinned(

packages/firestore/src/model/field_value.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,12 @@ export abstract class FieldValue {
126126
abstract isEqual(other: FieldValue): boolean;
127127
abstract compareTo(other: FieldValue): number;
128128

129+
/**
130+
* Returns the approximate (and wildly inaccurate) in-memory size of the field
131+
* value.
132+
*/
133+
abstract byteSize(): number;
134+
129135
toString(): string {
130136
const val = this.value();
131137
return val === null ? 'null' : val.toString();
@@ -167,6 +173,10 @@ export class NullValue extends FieldValue {
167173
return this.defaultCompareTo(other);
168174
}
169175

176+
byteSize(): number {
177+
return 1;
178+
}
179+
170180
static INSTANCE = new NullValue();
171181
}
172182

@@ -195,6 +205,10 @@ export class BooleanValue extends FieldValue {
195205
return this.defaultCompareTo(other);
196206
}
197207

208+
byteSize(): number {
209+
return 1;
210+
}
211+
198212
static of(value: boolean): BooleanValue {
199213
return value ? BooleanValue.TRUE : BooleanValue.FALSE;
200214
}
@@ -221,6 +235,10 @@ export abstract class NumberValue extends FieldValue {
221235
}
222236
return this.defaultCompareTo(other);
223237
}
238+
239+
byteSize(): number {
240+
return 4;
241+
}
224242
}
225243

226244
/** Utility function to compare doubles (using Firestore semantics for NaN). */
@@ -313,6 +331,13 @@ export class StringValue extends FieldValue {
313331
}
314332
return this.defaultCompareTo(other);
315333
}
334+
335+
byteSize(): number {
336+
// See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures:
337+
// "JavaScript's String type is [...] a set of elements of 16-bit unsigned
338+
// integer values"
339+
return this.internalValue.length * 2;
340+
}
316341
}
317342

318343
export class TimestampValue extends FieldValue {
@@ -347,6 +372,11 @@ export class TimestampValue extends FieldValue {
347372
return this.defaultCompareTo(other);
348373
}
349374
}
375+
376+
byteSize(): number {
377+
// Timestamps are made up of two distinct numbers (seconds/nanoseconds)
378+
return 8;
379+
}
350380
}
351381

352382
/**
@@ -410,6 +440,13 @@ export class ServerTimestampValue extends FieldValue {
410440
toString(): string {
411441
return '<ServerTimestamp localTime=' + this.localWriteTime.toString() + '>';
412442
}
443+
444+
byteSize(): number {
445+
return (
446+
/* localWriteTime */ 8 +
447+
(this.previousValue ? this.previousValue.byteSize() : 0)
448+
);
449+
}
413450
}
414451

415452
export class BlobValue extends FieldValue {
@@ -436,6 +473,10 @@ export class BlobValue extends FieldValue {
436473
}
437474
return this.defaultCompareTo(other);
438475
}
476+
477+
byteSize(): number {
478+
return this.internalValue.toUint8Array().byteLength;
479+
}
439480
}
440481

441482
export class RefValue extends FieldValue {
@@ -466,6 +507,14 @@ export class RefValue extends FieldValue {
466507
}
467508
return this.defaultCompareTo(other);
468509
}
510+
511+
byteSize(): number {
512+
return (
513+
this.databaseId.projectId.length +
514+
this.databaseId.database.length +
515+
this.key.toString().length
516+
);
517+
}
469518
}
470519

471520
export class GeoPointValue extends FieldValue {
@@ -492,6 +541,11 @@ export class GeoPointValue extends FieldValue {
492541
}
493542
return this.defaultCompareTo(other);
494543
}
544+
545+
byteSize(): number {
546+
// GeoPoints are made up of two distinct numbers (latitude/longitude)
547+
return 8;
548+
}
495549
}
496550

497551
export class ObjectValue extends FieldValue {
@@ -634,6 +688,14 @@ export class ObjectValue extends FieldValue {
634688
return FieldMask.fromSet(fields);
635689
}
636690

691+
byteSize(): number {
692+
let size = 0;
693+
this.internalValue.inorderTraversal((key, val) => {
694+
size += key.length + val.byteSize();
695+
});
696+
return size;
697+
}
698+
637699
toString(): string {
638700
return this.internalValue.toString();
639701
}
@@ -720,6 +782,13 @@ export class ArrayValue extends FieldValue {
720782
}
721783
}
722784

785+
byteSize(): number {
786+
return this.internalValue.reduce(
787+
(previousSize, value) => previousSize + value.byteSize(),
788+
0
789+
);
790+
}
791+
723792
toString(): string {
724793
const descriptions = this.internalValue.map(v => v.toString());
725794
return `[${descriptions.join(',')}]`;

packages/firestore/test/integration/api/database.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,7 @@ apiDescribe('Database', (persistence: boolean) => {
814814
// eslint-disable-next-line no-restricted-properties
815815
describe.skip('Listens are rejected remotely:', () => {
816816
//eslint-disable-next-line @typescript-eslint/no-explicit-any
817-
const queryForRejection : firestore.Query = null as any;
817+
const queryForRejection: firestore.Query = null as any;
818818

819819
it('will reject listens', () => {
820820
const deferred = new Deferred();

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,7 @@ export async function testMemoryEagerPersistence(): Promise<MemoryPersistence> {
128128
export async function testMemoryLruPersistence(
129129
params: LruParams = LruParams.DEFAULT
130130
): Promise<MemoryPersistence> {
131-
return MemoryPersistence.createLruPersistence(
132-
AutoId.newId(),
133-
JSON_SERIALIZER,
134-
params
135-
);
131+
return MemoryPersistence.createLruPersistence(AutoId.newId(), params);
136132
}
137133

138134
/** Clears the persistence in tests */

packages/firestore/test/unit/model/field_value.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { expect } from 'chai';
1919
import { GeoPoint } from '../../../src/api/geo_point';
2020
import { Timestamp } from '../../../src/api/timestamp';
2121
import * as fieldValue from '../../../src/model/field_value';
22+
import { primitiveComparator } from '../../../src/util/misc';
2223
import * as typeUtils from '../../../src/util/types';
2324
import {
2425
blob,
@@ -485,4 +486,57 @@ describe('FieldValue', () => {
485486
}
486487
);
487488
});
489+
490+
it('estimates size correctly for fixed sized values', () => {
491+
// This test verifies that each member of a group takes up the same amount
492+
// of space in memory (based on its estimated in-memory size).
493+
const equalityGroups = [
494+
[wrap(null), wrap(false), wrap(true)],
495+
[wrap(blob(0, 1)), wrap(blob(128, 129))],
496+
[wrap(NaN), wrap(Infinity), wrap(1), wrap(1.1)],
497+
[wrap(new GeoPoint(0, 0)), wrap(new GeoPoint(0, 0))],
498+
[wrap(Timestamp.fromMillis(100)), wrap(Timestamp.now())],
499+
[
500+
new fieldValue.ServerTimestampValue(Timestamp.fromMillis(100), null),
501+
new fieldValue.ServerTimestampValue(Timestamp.now(), null)
502+
]
503+
];
504+
505+
for (const group of equalityGroups) {
506+
const expectedItemSize = group[0].byteSize();
507+
for (const element of group) {
508+
expect(element.byteSize()).to.equal(expectedItemSize);
509+
}
510+
}
511+
});
512+
513+
it('estimates size correctly for relatively sized values', () => {
514+
// This test verifies for each group that the estimated size increases
515+
// as the size of the underlying data grows.
516+
const relativeGroups = [
517+
[wrap(blob(0)), wrap(blob(0, 1))],
518+
[
519+
new fieldValue.ServerTimestampValue(Timestamp.fromMillis(100), null),
520+
new fieldValue.ServerTimestampValue(Timestamp.now(), wrap(null))
521+
],
522+
[
523+
new fieldValue.RefValue(dbId('p1', 'd1'), key('c1/doc1')),
524+
new fieldValue.RefValue(dbId('p1', 'd1'), key('c1/doc1/c2/doc2'))
525+
],
526+
[wrap('foo'), wrap('foobar')],
527+
[wrap(['a', 'b']), wrap(['a', 'bc'])],
528+
[wrap(['a', 'b']), wrap(['a', 'b', 'c'])],
529+
[wrap({ a: 'a', b: 'b' }), wrap({ a: 'a', b: 'bc' })],
530+
[wrap({ a: 'a', b: 'b' }), wrap({ a: 'a', bc: 'b' })],
531+
[wrap({ a: 'a', b: 'b' }), wrap({ a: 'a', b: 'b', c: 'c' })]
532+
];
533+
534+
for (const group of relativeGroups) {
535+
const expectedOrder = group;
536+
const actualOrder = group
537+
.slice()
538+
.sort((l, r) => primitiveComparator(l.byteSize(), r.byteSize()));
539+
expect(expectedOrder).to.deep.equal(actualOrder);
540+
}
541+
});
488542
});

packages/firestore/test/unit/specs/spec_test_runner.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1238,7 +1238,6 @@ class MemoryTestRunner extends TestRunner {
12381238
? MemoryPersistence.createEagerPersistence(this.clientId)
12391239
: MemoryPersistence.createLruPersistence(
12401240
this.clientId,
1241-
serializer,
12421241
LruParams.DEFAULT
12431242
)
12441243
);

0 commit comments

Comments
 (0)