From 980ff084725c78fe75743d38732b3660853a2880 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 10 Feb 2021 09:50:39 -0700 Subject: [PATCH 1/3] Tree-shake EventGenerator --- .../database/src/core/view/EventGenerator.ts | 240 ++++++++++-------- packages/database/src/core/view/View.ts | 8 +- 2 files changed, 134 insertions(+), 114 deletions(-) diff --git a/packages/database/src/core/view/EventGenerator.ts b/packages/database/src/core/view/EventGenerator.ts index 28f63cc319a..bf3ba9a9733 100644 --- a/packages/database/src/core/view/EventGenerator.ts +++ b/packages/database/src/core/view/EventGenerator.ts @@ -19,7 +19,7 @@ import { NamedNode, Node } from '../snap/Node'; import { Change, ChangeType, changeChildMoved } from './Change'; import { assertionError } from '@firebase/util'; import { Query } from '../../api/Query'; -import { Index } from '../snap/indexes/Index'; +import { Index } from '../snap/indexes'; import { EventRegistration } from './EventRegistration'; import { Event } from './Event'; @@ -30,129 +30,145 @@ import { Event } from './Event'; * */ export class EventGenerator { - private index_: Index; + index_: Index; - constructor(private query_: Query) { - /** - */ + constructor(public query_: Query) { this.index_ = this.query_.getQueryParams().getIndex(); } +} - /** - * Given a set of raw changes (no moved events and prevName not specified yet), and a set of - * EventRegistrations that should be notified of these changes, generate the actual events to be raised. - * - * Notes: - * - child_moved events will be synthesized at this time for any child_changed events that affect - * our index. - * - prevName will be calculated based on the index ordering. - */ - generateEventsForChanges( - changes: Change[], - eventCache: Node, - eventRegistrations: EventRegistration[] - ): Event[] { - const events: Event[] = []; - const moves: Change[] = []; +/** + * Given a set of raw changes (no moved events and prevName not specified yet), and a set of + * EventRegistrations that should be notified of these changes, generate the actual events to be raised. + * + * Notes: + * - child_moved events will be synthesized at this time for any child_changed events that affect + * our index. + * - prevName will be calculated based on the index ordering. + */ +export function eventGeneratorGenerateEventsForChanges( + eventGenerator: EventGenerator, + changes: Change[], + eventCache: Node, + eventRegistrations: EventRegistration[] +): Event[] { + const events: Event[] = []; + const moves: Change[] = []; - changes.forEach(change => { - if ( - change.type === ChangeType.CHILD_CHANGED && - this.index_.indexedValueChanged( - change.oldSnap as Node, - change.snapshotNode - ) - ) { - moves.push(changeChildMoved(change.childName, change.snapshotNode)); - } - }); + changes.forEach(change => { + if ( + change.type === ChangeType.CHILD_CHANGED && + this.index_.indexedValueChanged( + change.oldSnap as Node, + change.snapshotNode + ) + ) { + moves.push(changeChildMoved(change.childName, change.snapshotNode)); + } + }); - this.generateEventsForType_( - events, - ChangeType.CHILD_REMOVED, - changes, - eventRegistrations, - eventCache - ); - this.generateEventsForType_( - events, - ChangeType.CHILD_ADDED, - changes, - eventRegistrations, - eventCache - ); - this.generateEventsForType_( - events, - ChangeType.CHILD_MOVED, - moves, - eventRegistrations, - eventCache - ); - this.generateEventsForType_( - events, - ChangeType.CHILD_CHANGED, - changes, - eventRegistrations, - eventCache - ); - this.generateEventsForType_( - events, - ChangeType.VALUE, - changes, - eventRegistrations, - eventCache - ); + eventGeneratorGenerateEventsForType( + eventGenerator, + events, + ChangeType.CHILD_REMOVED, + changes, + eventRegistrations, + eventCache + ); + eventGeneratorGenerateEventsForType( + eventGenerator, + events, + ChangeType.CHILD_ADDED, + changes, + eventRegistrations, + eventCache + ); + eventGeneratorGenerateEventsForType( + eventGenerator, + events, + ChangeType.CHILD_MOVED, + moves, + eventRegistrations, + eventCache + ); + eventGeneratorGenerateEventsForType( + eventGenerator, + events, + ChangeType.CHILD_CHANGED, + changes, + eventRegistrations, + eventCache + ); + eventGeneratorGenerateEventsForType( + eventGenerator, + events, + ChangeType.VALUE, + changes, + eventRegistrations, + eventCache + ); - return events; - } + return events; +} - /** - * Given changes of a single change type, generate the corresponding events. - */ - private generateEventsForType_( - events: Event[], - eventType: string, - changes: Change[], - registrations: EventRegistration[], - eventCache: Node - ) { - const filteredChanges = changes.filter(change => change.type === eventType); +/** + * Given changes of a single change type, generate the corresponding events. + */ +function eventGeneratorGenerateEventsForType( + eventGenerator: EventGenerator, + events: Event[], + eventType: string, + changes: Change[], + registrations: EventRegistration[], + eventCache: Node +) { + const filteredChanges = changes.filter(change => change.type === eventType); - filteredChanges.sort(this.compareChanges_.bind(this)); - filteredChanges.forEach(change => { - const materializedChange = this.materializeSingleChange_( - change, - eventCache - ); - registrations.forEach(registration => { - if (registration.respondsTo(change.type)) { - events.push( - registration.createEvent(materializedChange, this.query_) - ); - } - }); + filteredChanges.sort((a, b) => + eventGeneratorCompareChanges(eventGenerator, a, b) + ); + filteredChanges.forEach(change => { + const materializedChange = eventGeneratorMaterializeSingleChange( + eventGenerator, + change, + eventCache + ); + registrations.forEach(registration => { + if (registration.respondsTo(change.type)) { + events.push( + registration.createEvent(materializedChange, eventGenerator.query_) + ); + } }); - } + }); +} - private materializeSingleChange_(change: Change, eventCache: Node): Change { - if (change.type === 'value' || change.type === 'child_removed') { - return change; - } else { - change.prevName = eventCache.getPredecessorChildName( - change.childName, - change.snapshotNode, - this.index_ - ); - return change; - } +function eventGeneratorMaterializeSingleChange( + eventGenerator: EventGenerator, + change: Change, + eventCache: Node +): Change { + if (change.type === 'value' || change.type === 'child_removed') { + return change; + } else { + change.prevName = eventCache.getPredecessorChildName( + change.childName, + change.snapshotNode, + eventGenerator.index_ + ); + return change; } +} - private compareChanges_(a: Change, b: Change) { - if (a.childName == null || b.childName == null) { - throw assertionError('Should only compare child_ events.'); - } - const aWrapped = new NamedNode(a.childName, a.snapshotNode); - const bWrapped = new NamedNode(b.childName, b.snapshotNode); - return this.index_.compare(aWrapped, bWrapped); +function eventGeneratorCompareChanges( + eventGenerator: EventGenerator, + a: Change, + b: Change +) { + if (a.childName == null || b.childName == null) { + throw assertionError('Should only compare child_ events.'); } + const aWrapped = new NamedNode(a.childName, a.snapshotNode); + const bWrapped = new NamedNode(b.childName, b.snapshotNode); + return eventGenerator.index_.compare(aWrapped, bWrapped); } diff --git a/packages/database/src/core/view/View.ts b/packages/database/src/core/view/View.ts index 5c2176c3b43..7f66bb3ce99 100644 --- a/packages/database/src/core/view/View.ts +++ b/packages/database/src/core/view/View.ts @@ -20,7 +20,10 @@ import { ViewProcessor } from './ViewProcessor'; import { ChildrenNode } from '../snap/ChildrenNode'; import { CacheNode } from './CacheNode'; import { ViewCache } from './ViewCache'; -import { EventGenerator } from './EventGenerator'; +import { + EventGenerator, + eventGeneratorGenerateEventsForChanges +} from './EventGenerator'; import { assert } from '@firebase/util'; import { Operation, OperationType } from '../operation/Operation'; import { Change, changeChildAdded, changeValue } from './Change'; @@ -231,7 +234,8 @@ export class View { const registrations = eventRegistration ? [eventRegistration] : this.eventRegistrations_; - return this.eventGenerator_.generateEventsForChanges( + return eventGeneratorGenerateEventsForChanges( + this.eventGenerator_, changes, eventCache, registrations From 9252e2d2db376df3c97c5c1d346e159038913fe4 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 10 Feb 2021 10:37:29 -0700 Subject: [PATCH 2/3] Build --- packages/database/src/core/view/EventGenerator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/src/core/view/EventGenerator.ts b/packages/database/src/core/view/EventGenerator.ts index bf3ba9a9733..6a6dbf136ca 100644 --- a/packages/database/src/core/view/EventGenerator.ts +++ b/packages/database/src/core/view/EventGenerator.ts @@ -19,7 +19,7 @@ import { NamedNode, Node } from '../snap/Node'; import { Change, ChangeType, changeChildMoved } from './Change'; import { assertionError } from '@firebase/util'; import { Query } from '../../api/Query'; -import { Index } from '../snap/indexes'; +import { Index } from '../snap/indexes/Index'; import { EventRegistration } from './EventRegistration'; import { Event } from './Event'; From 7bcd6488f5fb0a41fa294cbae3d77edb0b98af52 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 10 Feb 2021 11:14:21 -0700 Subject: [PATCH 3/3] Fix CI --- packages/database/src/core/view/EventGenerator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/database/src/core/view/EventGenerator.ts b/packages/database/src/core/view/EventGenerator.ts index 6a6dbf136ca..4727f2c70c8 100644 --- a/packages/database/src/core/view/EventGenerator.ts +++ b/packages/database/src/core/view/EventGenerator.ts @@ -58,7 +58,7 @@ export function eventGeneratorGenerateEventsForChanges( changes.forEach(change => { if ( change.type === ChangeType.CHILD_CHANGED && - this.index_.indexedValueChanged( + eventGenerator.index_.indexedValueChanged( change.oldSnap as Node, change.snapshotNode )