Skip to content

Commit

Permalink
perf: migrate rbtree to js-sdsl
Browse files Browse the repository at this point in the history
  • Loading branch information
ZLY201 committed Sep 23, 2022
1 parent b6e4751 commit 84c087e
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 21 deletions.
1 change: 0 additions & 1 deletion dev/src/external-modules.d.ts
Expand Up @@ -15,5 +15,4 @@
*/

// TODO(mrschmidt): Come up with actual definitions for these modules.
declare module 'functional-red-black-tree';
declare module 'length-prefixed-json-stream';
5 changes: 0 additions & 5 deletions dev/src/types.ts
Expand Up @@ -103,11 +103,6 @@ export type UnaryMethod<Req, Resp> = (
callOptions: CallOptions
) => Promise<[Resp, unknown, unknown]>;

// We don't have type information for the npm package
// `functional-red-black-tree`.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type RBTree = any;

/**
* A default converter to use when none is provided.
*
Expand Down
29 changes: 15 additions & 14 deletions dev/src/watch.ts
Expand Up @@ -17,7 +17,7 @@
import * as firestore from '@google-cloud/firestore';

import * as assert from 'assert';
import * as rbtree from 'functional-red-black-tree';
import {OrderedSet} from "js-sdsl";
import {GoogleError, Status} from 'google-gax';
import {Duplex} from 'stream';

Expand All @@ -29,7 +29,7 @@ import {DocumentReference, Firestore, Query} from './index';
import {logger} from './logger';
import {QualifiedResourcePath} from './path';
import {Timestamp} from './timestamp';
import {defaultConverter, RBTree} from './types';
import {defaultConverter} from './types';
import {requestTag} from './util';

import api = google.firestore.v1;
Expand Down Expand Up @@ -183,7 +183,7 @@ abstract class Watch<T = firestore.DocumentData> {
* @private
* @internal
*/
private docTree: RBTree | undefined;
private docTree: OrderedSet<QueryDocumentSnapshot<T>> | undefined;

/**
* We need this to track whether we've pushed an initial set of changes,
Expand Down Expand Up @@ -271,7 +271,7 @@ abstract class Watch<T = firestore.DocumentData> {
);
this.onNext = onNext;
this.onError = onError;
this.docTree = rbtree(this.getComparator());
this.docTree = new OrderedSet([], this.getComparator(), true);

this.initStream();

Expand Down Expand Up @@ -334,7 +334,7 @@ abstract class Watch<T = firestore.DocumentData> {
this.changeMap.clear();
this.resumeToken = undefined;

this.docTree.forEach((snapshot: QueryDocumentSnapshot) => {
this.docTree!.forEach((snapshot: QueryDocumentSnapshot<T>) => {
// Mark each document as deleted. If documents are not deleted, they
// will be send again by the server.
this.changeMap.set(
Expand Down Expand Up @@ -650,14 +650,15 @@ abstract class Watch<T = firestore.DocumentData> {
this.requestTag,
'Sending snapshot with %d changes and %d documents',
String(appliedChanges.length),
this.docTree.length
this.docTree!.size()
);
// We pass the current set of changes, even if `docTree` is modified later.
const currentTree = this.docTree;
const snapshots: QueryDocumentSnapshot<T>[] = [];
this.docTree!.forEach(key => snapshots.push(key));
this.onNext(
readTime,
currentTree.length,
() => currentTree.keys,
this.docTree!.size(),
() => snapshots,
() => appliedChanges
);
this.hasPushed = true;
Expand All @@ -676,9 +677,9 @@ abstract class Watch<T = firestore.DocumentData> {
private deleteDoc(name: string): DocumentChange<T> {
assert(this.docMap.has(name), 'Document to delete does not exist');
const oldDocument = this.docMap.get(name)!;
const existing = this.docTree.find(oldDocument);
const existing = this.docTree!.find(oldDocument);
const oldIndex = existing.index;
this.docTree = existing.remove();
this.docTree!.eraseElementByIterator(existing);
this.docMap.delete(name);
return new DocumentChange(ChangeType.removed, oldDocument, oldIndex, -1);
}
Expand All @@ -692,8 +693,8 @@ abstract class Watch<T = firestore.DocumentData> {
private addDoc(newDocument: QueryDocumentSnapshot<T>): DocumentChange<T> {
const name = newDocument.ref.path;
assert(!this.docMap.has(name), 'Document to add already exists');
this.docTree = this.docTree.insert(newDocument, null);
const newIndex = this.docTree.find(newDocument).index;
this.docTree!.insert(newDocument);
const newIndex = this.docTree!.find(newDocument).index;
this.docMap.set(name, newDocument);
return new DocumentChange(ChangeType.added, newDocument, -1, newIndex);
}
Expand Down Expand Up @@ -764,7 +765,7 @@ abstract class Watch<T = firestore.DocumentData> {
});

assert(
this.docTree.length === this.docMap.size,
this.docTree!.size() === this.docMap.size,
'The update document ' +
'tree and document map should have the same number of entries.'
);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -53,8 +53,8 @@
},
"dependencies": {
"fast-deep-equal": "^3.1.1",
"functional-red-black-tree": "^1.0.1",
"google-gax": "^3.5.1",
"js-sdsl": "^4.1.5-beta.1",
"protobufjs": "^7.0.0"
},
"devDependencies": {
Expand Down

0 comments on commit 84c087e

Please sign in to comment.