From 4f8cfcf92feecbe1e6f4b1f434b94aac580705f9 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 8 Mar 2021 09:15:50 -0700 Subject: [PATCH] Treeshake Tree (#4582) --- packages/database/src/core/Repo.ts | 80 ++-- packages/database/src/core/util/Tree.ts | 358 +++++++++--------- .../database/src/core/view/ViewProcessor.ts | 2 +- 3 files changed, 219 insertions(+), 221 deletions(-) diff --git a/packages/database/src/core/Repo.ts b/packages/database/src/core/Repo.ts index 0906e2ff62e..99f719a7f4b 100644 --- a/packages/database/src/core/Repo.ts +++ b/packages/database/src/core/Repo.ts @@ -38,15 +38,15 @@ import { import { SyncTree, syncTreeAckUserWrite, + syncTreeAddEventRegistration, + syncTreeApplyServerMerge, syncTreeApplyServerOverwrite, syncTreeApplyTaggedQueryMerge, syncTreeApplyTaggedQueryOverwrite, - syncTreeApplyServerMerge, - syncTreeGetServerValue, - syncTreeCalcCompleteEventCache, - syncTreeApplyUserOverwrite, syncTreeApplyUserMerge, - syncTreeAddEventRegistration, + syncTreeApplyUserOverwrite, + syncTreeCalcCompleteEventCache, + syncTreeGetServerValue, syncTreeRemoveEventRegistration } from './SyncTree'; import { SnapshotHolder } from './SnapshotHolder'; @@ -92,7 +92,17 @@ import { StatsCollection } from './stats/StatsCollection'; import { Event } from './view/Event'; import { Node } from './snap/Node'; import { Indexable } from './util/misc'; -import { Tree } from './util/Tree'; +import { + Tree, + treeForEachAncestor, + treeForEachChild, + treeForEachDescendant, + treeGetPath, + treeGetValue, + treeHasChildren, + treeSetValue, + treeSubTree +} from './util/Tree'; import { isValidPriority, validateFirebaseData } from './util/validation'; import { ChildrenNode } from './snap/ChildrenNode'; import { PRIORITY_INDEX } from './snap/indexes/PriorityIndex'; @@ -937,11 +947,11 @@ export function repoStartTransaction( // Mark as run and add to our queue. transaction.status = TransactionStatus.RUN; - const queueNode = repo.transactionQueueTree_.subTree(path); - const nodeQueue = queueNode.getValue() || []; + const queueNode = treeSubTree(repo.transactionQueueTree_, path); + const nodeQueue = treeGetValue(queueNode) || []; nodeQueue.push(transaction); - queueNode.setValue(nodeQueue); + treeSetValue(queueNode, nodeQueue); // Update visibleData and raise events // Note: We intentionally raise events after updating all of our @@ -1023,7 +1033,7 @@ function repoSendReadyTransactions( repoPruneCompletedTransactionsBelowNode(repo, node); } - if (node.getValue() !== null) { + if (treeGetValue(node)) { const queue = repoBuildTransactionQueue(repo, node); assert(queue.length > 0, 'Sending zero length transaction queue'); @@ -1033,10 +1043,10 @@ function repoSendReadyTransactions( // If they're all run (and not sent), we can send them. Else, we must wait. if (allRun) { - repoSendTransactionQueue(repo, node.path(), queue); + repoSendTransactionQueue(repo, treeGetPath(node), queue); } - } else if (node.hasChildren()) { - node.forEachChild(childNode => { + } else if (treeHasChildren(node)) { + treeForEachChild(node, childNode => { repoSendReadyTransactions(repo, childNode); }); } @@ -1117,7 +1127,7 @@ function repoSendTransactionQueue( // Now remove the completed transactions. repoPruneCompletedTransactionsBelowNode( repo, - repo.transactionQueueTree_.subTree(path) + treeSubTree(repo.transactionQueueTree_, path) ); // There may be pending transactions that we can now send. repoSendReadyTransactions(repo, repo.transactionQueueTree_); @@ -1171,7 +1181,7 @@ function repoRerunTransactions(repo: Repo, changedPath: Path): Path { repo, changedPath ); - const path = rootMostTransactionNode.path(); + const path = treeGetPath(rootMostTransactionNode); const queue = repoBuildTransactionQueue(repo, rootMostTransactionNode); repoRerunTransactionQueue(repo, queue, path); @@ -1360,8 +1370,8 @@ function repoGetAncestorTransactionNode( // find a node with pending transactions. let transactionNode = repo.transactionQueueTree_; front = pathGetFront(path); - while (front !== null && transactionNode.getValue() === null) { - transactionNode = transactionNode.subTree(front); + while (front !== null && treeGetValue(transactionNode) === undefined) { + transactionNode = treeSubTree(transactionNode, front); path = pathPopFront(path); front = pathGetFront(path); } @@ -1389,9 +1399,7 @@ function repoBuildTransactionQueue( ); // Sort them by the order the transactions were created. - transactionQueue.sort((a, b) => { - return a.order - b.order; - }); + transactionQueue.sort((a, b) => a.order - b.order); return transactionQueue; } @@ -1401,14 +1409,14 @@ function repoAggregateTransactionQueuesForNode( node: Tree, queue: Transaction[] ): void { - const nodeQueue = node.getValue(); - if (nodeQueue !== null) { + const nodeQueue = treeGetValue(node); + if (nodeQueue) { for (let i = 0; i < nodeQueue.length; i++) { queue.push(nodeQueue[i]); } } - node.forEachChild(child => { + treeForEachChild(node, child => { repoAggregateTransactionQueuesForNode(repo, child, queue); }); } @@ -1420,7 +1428,7 @@ function repoPruneCompletedTransactionsBelowNode( repo: Repo, node: Tree ): void { - const queue = node.getValue(); + const queue = treeGetValue(node); if (queue) { let to = 0; for (let from = 0; from < queue.length; from++) { @@ -1430,10 +1438,10 @@ function repoPruneCompletedTransactionsBelowNode( } } queue.length = to; - node.setValue(queue.length > 0 ? queue : null); + treeSetValue(node, queue.length > 0 ? queue : undefined); } - node.forEachChild(childNode => { + treeForEachChild(node, childNode => { repoPruneCompletedTransactionsBelowNode(repo, childNode); }); } @@ -1446,17 +1454,17 @@ function repoPruneCompletedTransactionsBelowNode( * @param path Path for which we want to abort related transactions. */ function repoAbortTransactions(repo: Repo, path: Path): Path { - const affectedPath = repoGetAncestorTransactionNode(repo, path).path(); + const affectedPath = treeGetPath(repoGetAncestorTransactionNode(repo, path)); - const transactionNode = repo.transactionQueueTree_.subTree(path); + const transactionNode = treeSubTree(repo.transactionQueueTree_, path); - transactionNode.forEachAncestor((node: Tree) => { + treeForEachAncestor(transactionNode, (node: Tree) => { repoAbortTransactionsOnNode(repo, node); }); repoAbortTransactionsOnNode(repo, transactionNode); - transactionNode.forEachDescendant((node: Tree) => { + treeForEachDescendant(transactionNode, (node: Tree) => { repoAbortTransactionsOnNode(repo, node); }); @@ -1472,8 +1480,8 @@ function repoAbortTransactionsOnNode( repo: Repo, node: Tree ): void { - const queue = node.getValue(); - if (queue !== null) { + const queue = treeGetValue(node); + if (queue) { // Queue up the callbacks and fire them after cleaning up all of our // transaction state, since the callback could trigger more transactions // or sets. @@ -1519,14 +1527,18 @@ function repoAbortTransactionsOnNode( } if (lastSent === -1) { // We're not waiting for any sent transactions. We can clear the queue. - node.setValue(null); + treeSetValue(node, undefined); } else { // Remove the transactions we aborted. queue.length = lastSent + 1; } // Now fire the callbacks. - eventQueueRaiseEventsForChangedPath(repo.eventQueue_, node.path(), events); + eventQueueRaiseEventsForChangedPath( + repo.eventQueue_, + treeGetPath(node), + events + ); for (let i = 0; i < callbacks.length; i++) { exceptionGuard(callbacks[i]); } diff --git a/packages/database/src/core/util/Tree.ts b/packages/database/src/core/util/Tree.ts index c014f04fd9e..b61c0b09d44 100644 --- a/packages/database/src/core/util/Tree.ts +++ b/packages/database/src/core/util/Tree.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { assert, contains, safeGet } from '@firebase/util'; +import { contains, safeGet } from '@firebase/util'; import { Path, pathGetFront, pathPopFront } from './Path'; import { each } from './util'; @@ -23,12 +23,12 @@ import { each } from './util'; /** * Node in a Tree. */ -export class TreeNode { +export interface TreeNode { // TODO: Consider making accessors that create children and value lazily or // separate Internal / Leaf 'types'. - children: { [name: string]: TreeNode } = {}; - childCount = 0; - value: T | null = null; + children: Record>; + childCount: number; + value?: T; } /** @@ -38,209 +38,195 @@ export class TreeNode { */ export class Tree { /** - * @param name_ Optional name of the node. - * @param parent_ Optional parent node. - * @param node_ Optional node to wrap. + * @param name Optional name of the node. + * @param parent Optional parent node. + * @param node Optional node to wrap. */ constructor( - private name_: string = '', - private parent_: Tree | null = null, - private node_: TreeNode = new TreeNode() + readonly name: string = '', + readonly parent: Tree | null = null, + public node: TreeNode = { children: {}, childCount: 0 } ) {} +} - /** - * Returns a sub-Tree for the given path. - * - * @param pathObj Path to look up. - * @return Tree for path. - */ - subTree(pathObj: string | Path): Tree { - // TODO: Require pathObj to be Path? - let path = pathObj instanceof Path ? pathObj : new Path(pathObj); - let child = this as Tree, - next = pathGetFront(path); - while (next !== null) { - const childNode = safeGet(child.node_.children, next) || new TreeNode(); - child = new Tree(next, child, childNode); - path = pathPopFront(path); - next = pathGetFront(path); - } - - return child; - } - - /** - * Returns the data associated with this tree node. - * - * @return The data or null if no data exists. - */ - getValue(): T | null { - return this.node_.value; - } - - /** - * Sets data to this tree node. - * - * @param value Value to set. - */ - setValue(value: T) { - assert(typeof value !== 'undefined', 'Cannot set value to undefined'); - this.node_.value = value; - this.updateParents_(); +/** + * Returns a sub-Tree for the given path. + * + * @param pathObj Path to look up. + * @return Tree for path. + */ +export function treeSubTree(tree: Tree, pathObj: string | Path): Tree { + // TODO: Require pathObj to be Path? + let path = pathObj instanceof Path ? pathObj : new Path(pathObj); + let child = tree, + next = pathGetFront(path); + while (next !== null) { + const childNode = safeGet(child.node.children, next) || { + children: {}, + childCount: 0 + }; + child = new Tree(next, child, childNode); + path = pathPopFront(path); + next = pathGetFront(path); } - /** - * Clears the contents of the tree node (its value and all children). - */ - clear() { - this.node_.value = null; - this.node_.children = {}; - this.node_.childCount = 0; - this.updateParents_(); - } + return child; +} - /** - * @return Whether the tree has any children. - */ - hasChildren(): boolean { - return this.node_.childCount > 0; - } +/** + * Returns the data associated with this tree node. + * + * @return The data or null if no data exists. + */ +export function treeGetValue(tree: Tree): T | undefined { + return tree.node.value; +} - /** - * @return Whethe rthe tree is empty (no value or children). - */ - isEmpty(): boolean { - return this.getValue() === null && !this.hasChildren(); - } +/** + * Sets data to this tree node. + * + * @param value Value to set. + */ +export function treeSetValue(tree: Tree, value: T | undefined): void { + tree.node.value = value; + treeUpdateParents(tree); +} - /** - * Calls action for each child of this tree node. - * - * @param action Action to be called for each child. - */ - forEachChild(action: (tree: Tree) => void) { - each(this.node_.children, (child: string, childTree: TreeNode) => { - action(new Tree(child, this, childTree)); - }); - } +/** + * @return Whether the tree has any children. + */ +export function treeHasChildren(tree: Tree): boolean { + return tree.node.childCount > 0; +} - /** - * Does a depth-first traversal of this node's descendants, calling action for each one. - * - * @param action Action to be called for each child. - * @param includeSelf Whether to call action on this node as well. Defaults to - * false. - * @param childrenFirst Whether to call action on children before calling it on - * parent. - */ - forEachDescendant( - action: (tree: Tree) => void, - includeSelf?: boolean, - childrenFirst?: boolean - ) { - if (includeSelf && !childrenFirst) { - action(this); - } +/** + * @return Whethe rthe tree is empty (no value or children). + */ +export function treeIsEmpty(tree: Tree): boolean { + return treeGetValue(tree) === undefined && !treeHasChildren(tree); +} - this.forEachChild(child => { - child.forEachDescendant(action, /*includeSelf=*/ true, childrenFirst); - }); +/** + * Calls action for each child of this tree node. + * + * @param action Action to be called for each child. + */ +export function treeForEachChild( + tree: Tree, + action: (tree: Tree) => void +): void { + each(tree.node.children, (child: string, childTree: TreeNode) => { + action(new Tree(child, tree, childTree)); + }); +} - if (includeSelf && childrenFirst) { - action(this); - } +/** + * Does a depth-first traversal of this node's descendants, calling action for each one. + * + * @param action Action to be called for each child. + * @param includeSelf Whether to call action on this node as well. Defaults to + * false. + * @param childrenFirst Whether to call action on children before calling it on + * parent. + */ +export function treeForEachDescendant( + tree: Tree, + action: (tree: Tree) => void, + includeSelf?: boolean, + childrenFirst?: boolean +): void { + if (includeSelf && !childrenFirst) { + action(tree); } - /** - * Calls action on each ancestor node. - * - * @param action Action to be called on each parent; return - * true to abort. - * @param includeSelf Whether to call action on this node as well. - * @return true if the action callback returned true. - */ - forEachAncestor( - action: (tree: Tree) => unknown, - includeSelf?: boolean - ): boolean { - let node = includeSelf ? this : this.parent(); - while (node !== null) { - if (action(node)) { - return true; - } - node = node.parent(); - } - return false; - } + treeForEachChild(tree, child => { + treeForEachDescendant(child, action, true, childrenFirst); + }); - /** - * Does a depth-first traversal of this node's descendants. When a descendant with a value - * is found, action is called on it and traversal does not continue inside the node. - * Action is *not* called on this node. - * - * @param action Action to be called for each child. - */ - forEachImmediateDescendantWithValue(action: (tree: Tree) => void) { - this.forEachChild(child => { - if (child.getValue() !== null) { - action(child); - } else { - child.forEachImmediateDescendantWithValue(action); - } - }); + if (includeSelf && childrenFirst) { + action(tree); } +} - /** - * @return The path of this tree node, as a Path. - */ - path(): Path { - return new Path( - this.parent_ === null - ? this.name_ - : this.parent_.path() + '/' + this.name_ - ); +/** + * Calls action on each ancestor node. + * + * @param action Action to be called on each parent; return + * true to abort. + * @param includeSelf Whether to call action on this node as well. + * @return true if the action callback returned true. + */ +export function treeForEachAncestor( + tree: Tree, + action: (tree: Tree) => unknown, + includeSelf?: boolean +): boolean { + let node = includeSelf ? tree : tree.parent; + while (node !== null) { + if (action(node)) { + return true; + } + node = node.parent; } + return false; +} - /** - * @return The name of the tree node. - */ - name(): string { - return this.name_; - } +/** + * Does a depth-first traversal of this node's descendants. When a descendant with a value + * is found, action is called on it and traversal does not continue inside the node. + * Action is *not* called on this node. + * + * @param action Action to be called for each child. + */ +export function treeForEachImmediateDescendantWithValue( + tree: Tree, + action: (tree: Tree) => void +): void { + treeForEachChild(tree, child => { + if (treeGetValue(child) !== undefined) { + action(child); + } else { + treeForEachImmediateDescendantWithValue(child, action); + } + }); +} - /** - * @return The parent tree node, or null if this is the root of the tree. - */ - parent(): Tree | null { - return this.parent_; - } +/** + * @return The path of this tree node, as a Path. + */ +export function treeGetPath(tree: Tree) { + return new Path( + tree.parent === null + ? tree.name + : treeGetPath(tree.parent) + '/' + tree.name + ); +} - /** - * Adds or removes this child from its parent based on whether it's empty or not. - */ - private updateParents_() { - if (this.parent_ !== null) { - this.parent_.updateChild_(this.name_, this); - } +/** + * Adds or removes this child from its parent based on whether it's empty or not. + */ +function treeUpdateParents(tree: Tree) { + if (tree.parent !== null) { + treeUpdateChild(tree.parent, tree.name, tree); } +} - /** - * Adds or removes the passed child to this tree node, depending on whether it's empty. - * - * @param childName The name of the child to update. - * @param child The child to update. - */ - private updateChild_(childName: string, child: Tree) { - const childEmpty = child.isEmpty(); - const childExists = contains(this.node_.children, childName); - if (childEmpty && childExists) { - delete this.node_.children[childName]; - this.node_.childCount--; - this.updateParents_(); - } else if (!childEmpty && !childExists) { - this.node_.children[childName] = child.node_; - this.node_.childCount++; - this.updateParents_(); - } +/** + * Adds or removes the passed child to this tree node, depending on whether it's empty. + * + * @param childName The name of the child to update. + * @param child The child to update. + */ +function treeUpdateChild(tree: Tree, childName: string, child: Tree) { + const childEmpty = treeIsEmpty(child); + const childExists = contains(tree.node.children, childName); + if (childEmpty && childExists) { + delete tree.node.children[childName]; + tree.node.childCount--; + treeUpdateParents(tree); + } else if (!childEmpty && !childExists) { + tree.node.children[childName] = child.node; + tree.node.childCount++; + treeUpdateParents(tree); } } diff --git a/packages/database/src/core/view/ViewProcessor.ts b/packages/database/src/core/view/ViewProcessor.ts index 5750789d4c5..a36dd38f2a8 100644 --- a/packages/database/src/core/view/ViewProcessor.ts +++ b/packages/database/src/core/view/ViewProcessor.ts @@ -588,7 +588,7 @@ export class ViewProcessor { viewMergeTree.children.inorderTraversal((childKey, childMergeTree) => { const isUnknownDeepMerge = !viewCache.getServerCache().isCompleteForChild(childKey) && - childMergeTree.value == null; + childMergeTree.value === undefined; if (!serverNode.hasChild(childKey) && !isUnknownDeepMerge) { const serverChild = viewCache .getServerCache()