From fad5cf1588a96ea828810ceb54e62b69b5da67d1 Mon Sep 17 00:00:00 2001 From: Jenn Date: Thu, 31 Aug 2023 23:43:47 +0000 Subject: [PATCH 1/6] Prev changes --- .../dds/tree2/src/core/tree/anchorSet.ts | 180 ++++++++++-------- .../dds/tree2/src/core/tree/visitDelta.ts | 6 + .../chunked-forest/chunkedForest.ts | 117 ++++++------ .../object-forest/objectForest.ts | 52 +++-- 4 files changed, 207 insertions(+), 148 deletions(-) diff --git a/experimental/dds/tree2/src/core/tree/anchorSet.ts b/experimental/dds/tree2/src/core/tree/anchorSet.ts index 862234ea5f8c..c0f117ff3d8d 100644 --- a/experimental/dds/tree2/src/core/tree/anchorSet.ts +++ b/experimental/dds/tree2/src/core/tree/anchorSet.ts @@ -19,7 +19,7 @@ import { FieldKey } from "../schema-stored"; import { UpPath } from "./pathTree"; import { Value, detachedFieldAsKey, DetachedField, EmptyKey } from "./types"; import { PathVisitor } from "./visitPath"; -import { visitDelta, DeltaVisitor } from "./visitDelta"; +import { visitDelta } from "./visitDelta"; import * as Delta from "./delta"; /** @@ -557,127 +557,147 @@ export class AnchorSet implements ISubscribable, AnchorLoca * Updates the anchors according to the changes described in the given delta */ public applyDelta(delta: Delta.Root): void { - let parentField: FieldKey | undefined; - let parent: UpPath | undefined; const moveTable = new Map(); - - // Run `withNode` on anchorNode for parent if there is such an anchorNode. - // If at root, run `withRoot` instead. - const maybeWithNode: ( - withNode: (anchorNode: PathNode) => void, - withRoot?: () => void, - ) => void = (withNode, withRoot) => { - if (parent === undefined && withRoot !== undefined) { - withRoot(); - } else { - assert(parent !== undefined, 0x5b0 /* parent must exist */); - // TODO:Perf: - // When traversing to a depth D when there are not anchors in that subtree, this goes O(D^2). - // Delta traversal should early out in this case because no work is needed (and all move outs are known to not contain anchors). - parent = this.internalizePath(parent); - if (parent instanceof PathNode) { - withNode(parent); + // eslint-disable-next-line @typescript-eslint/no-this-alias + const thisAnchorSet = this; + + const visitor = { + // Run `withNode` on anchorNode for parent if there is such an anchorNode. + // If at root, run `withRoot` instead. + maybeWithNode(withNode: (anchorNode: PathNode) => void, withRoot?: () => void) { + if (this.parent === undefined && withRoot !== undefined) { + withRoot(); + } else { + assert(this.parent !== undefined, 0x5b0 /* parent must exist */); + // TODO:Perf: + // When traversing to a depth D when there are not anchors in that subtree, this goes O(D^2). + // Delta traversal should early out in this case because no work is needed (and all move outs are known to not contain anchors). + this.parent = thisAnchorSet.internalizePath(this.parent); + if (this.parent instanceof PathNode) { + withNode(this.parent); + } } - } - }; - - // Lookup table for path visitors collected from {@link AnchorEvents.visitSubtreeChanging} emitted events. - // The key is the path of the node that the visitor is registered on. The code ensures that the path visitor visits only the appropriate subtrees - // by maintaining the mapping only during time between the {@link DeltaVisitor.enterNode} and {@link DeltaVisitor.exitNode} calls for a given anchorNode. - const pathVisitors: Map> = new Map(); - - const visitor: DeltaVisitor = { - onDelete: (start: number, count: number): void => { - assert(parentField !== undefined, 0x3a7 /* Must be in a field to delete */); - maybeWithNode( + }, + // Lookup table for path visitors collected from {@link AnchorEvents.visitSubtreeChanging} emitted events. + // The key is the path of the node that the visitor is registered on. The code ensures that the path visitor visits only the appropriate subtrees + // by maintaining the mapping only during time between the {@link DeltaVisitor.enterNode} and {@link DeltaVisitor.exitNode} calls for a given anchorNode. + pathVisitors: new Map>(), + parentField: undefined as FieldKey | undefined, + parent: undefined as UpPath | undefined, + fork() { + return { ...this }; + }, + free() {}, + onDelete(start: number, count: number): void { + assert(this.parentField !== undefined, 0x3a7 /* Must be in a field to delete */); + this.maybeWithNode( (p) => { p.events.emit("childrenChanging", p); }, - () => this.events.emit("childrenChanging", this), + () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), ); const upPath: UpPath = { - parent, - parentField, + parent: this.parent, + parentField: this.parentField, parentIndex: start, }; - for (const visitors of pathVisitors.values()) { + for (const visitors of this.pathVisitors.values()) { for (const pathVisitor of visitors) { pathVisitor.onDelete(upPath, count); } } - this.removeChildren( + thisAnchorSet.removeChildren( { - parent, - parentField, + parent: this.parent, + parentField: this.parentField, parentIndex: start, }, count, ); }, - onInsert: (start: number, content: Delta.ProtoNodes): void => { - assert(parentField !== undefined, 0x3a8 /* Must be in a field to insert */); - maybeWithNode( + onInsert(start: number, content: Delta.ProtoNodes): void { + assert(this.parentField !== undefined, 0x3a8 /* Must be in a field to insert */); + this.maybeWithNode( (p) => p.events.emit("childrenChanging", p), - () => this.events.emit("childrenChanging", this), + () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), ); const upPath: UpPath = { - parent, - parentField, + parent: this.parent, + parentField: this.parentField, parentIndex: start, }; - for (const visitors of pathVisitors.values()) { + for (const visitors of this.pathVisitors.values()) { for (const pathVisitor of visitors) { pathVisitor.onInsert(upPath, content); } } - this.offsetChildren( + thisAnchorSet.offsetChildren( { - parent, - parentField, + parent: this.parent, + parentField: this.parentField, parentIndex: start, }, content.length, ); }, - onMoveOut: (start: number, count: number, id: Delta.MoveId): void => { - assert(parentField !== undefined, 0x3a9 /* Must be in a field to move out */); - maybeWithNode( + onMoveOut(start: number, count: number, id: Delta.MoveId): void { + assert(this.parentField !== undefined, 0x3a9 /* Must be in a field to move out */); + this.maybeWithNode( (p) => p.events.emit("childrenChanging", p), - () => this.events.emit("childrenChanging", this), + () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), ); - const fieldKey = this.createEmptyDetachedField(); - const source = { parent, parentField, parentIndex: start }; - const destination = { parent: this.root, parentField: fieldKey, parentIndex: 0 }; - this.moveChildren(source, destination, count); + const fieldKey = thisAnchorSet.createEmptyDetachedField(); + const source = { + parent: this.parent, + parentField: this.parentField, + parentIndex: start, + }; + const destination = { + parent: thisAnchorSet.root, + parentField: fieldKey, + parentIndex: 0, + }; + thisAnchorSet.moveChildren(source, destination, count); moveTable.set(id, destination); }, - onMoveIn: (start: number, count: number, id: Delta.MoveId): void => { - assert(parentField !== undefined, 0x3aa /* Must be in a field to move in */); - maybeWithNode( + onMoveIn(start: number, count: number, id: Delta.MoveId): void { + assert(this.parentField !== undefined, 0x3aa /* Must be in a field to move in */); + this.maybeWithNode( (p) => p.events.emit("childrenChanging", p), - () => this.events.emit("childrenChanging", this), + () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), ); const sourcePath = moveTable.get(id) ?? fail("Must visit a move in after its move out"); moveTable.delete(id); - this.moveChildren(sourcePath, { parent, parentField, parentIndex: start }, count); + thisAnchorSet.moveChildren( + sourcePath, + { parent: this.parent, parentField: this.parentField, parentIndex: start }, + count, + ); }, - enterNode: (index: number): void => { - assert(parentField !== undefined, 0x3ab /* Must be in a field to enter node */); - parent = { parent, parentField, parentIndex: index }; - parentField = undefined; - maybeWithNode((p) => { + enterNode(index: number): void { + assert( + this.parentField !== undefined, + 0x3ab /* Must be in a field to enter node */, + ); + this.parent = { + parent: this.parent, + parentField: this.parentField, + parentIndex: index, + }; + this.parentField = undefined; + this.maybeWithNode((p) => { // avoid multiple pass side-effects - if (!pathVisitors.has(p)) { + if (!this.pathVisitors.has(p)) { const visitors: (PathVisitor | void)[] = p.events.emitAndCollect( "subtreeChanging", p, ); if (visitors.length > 0) { - pathVisitors.set( + this.pathVisitors.set( p, new Set(visitors.filter((v): v is PathVisitor => v !== undefined)), ); @@ -685,20 +705,22 @@ export class AnchorSet implements ISubscribable, AnchorLoca } }); }, - exitNode: (index: number): void => { - assert(parent !== undefined, 0x3ac /* Must have parent node */); - maybeWithNode((p) => { + exitNode(index: number): void { + assert(this.parent !== undefined, 0x3ac /* Must have parent node */); + this.maybeWithNode((p) => { // Remove subtree path visitors added at this node if there are any - pathVisitors.delete(p); + this.pathVisitors.delete(p); }); - parentField = parent.parentField; - parent = parent.parent; + const parent = this.parent; + assert(parent !== undefined, "Unable to exit root node"); + this.parentField = parent.parentField; + this.parent = parent.parent; }, - enterField: (key: FieldKey): void => { - parentField = key; + enterField(key: FieldKey): void { + this.parentField = key; }, - exitField: (key: FieldKey): void => { - parentField = undefined; + exitField(key: FieldKey): void { + this.parentField = undefined; }, }; this.events.emit("treeChanging", this); diff --git a/experimental/dds/tree2/src/core/tree/visitDelta.ts b/experimental/dds/tree2/src/core/tree/visitDelta.ts index 8af39b647c90..4cf644efd578 100644 --- a/experimental/dds/tree2/src/core/tree/visitDelta.ts +++ b/experimental/dds/tree2/src/core/tree/visitDelta.ts @@ -98,6 +98,12 @@ export function visitDelta(delta: Delta.Root, visitor: DeltaVisitor): void { } export interface DeltaVisitor { + /** + * Forks the current visitor. + * Any fork produced this way is freed before the visit terminates. + */ + fork(): DeltaVisitor; + free(): void; onDelete(index: number, count: number): void; onInsert(index: number, content: Delta.ProtoNodes): void; onMoveOut(index: number, count: number, id: Delta.MoveId): void; diff --git a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts index 16f8fe0b88ef..d9f973ce99c7 100644 --- a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts @@ -95,51 +95,62 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { this.roots = this.roots.clone(); } - // Current location in the tree, as a non-shared BasicChunk (TODO: support in-place modification of other chunk formats when possible). - // Start above root detached sequences. - const mutableChunkStack: StackNode[] = []; - let mutableChunk: BasicChunk | undefined = this.roots; - - const getParent = () => { - assert(mutableChunkStack.length > 0, 0x532 /* invalid access to root's parent */); - return mutableChunkStack[mutableChunkStack.length - 1]; - }; + // eslint-disable-next-line @typescript-eslint/no-this-alias + const thisForest = this; - const moveIn = (index: number, toAttach: DetachedField): number => { - const detachedKey = detachedFieldAsKey(toAttach); - const children = this.roots.fields.get(detachedKey) ?? []; - this.roots.fields.delete(detachedKey); - if (children.length === 0) { - return 0; // Prevent creating 0 sized fields when inserting empty into empty. - } + const visitor = { + // Current location in the tree, as a non-shared BasicChunk (TODO: support in-place modification of other chunk formats when possible). + // Start above root detached sequences. + mutableChunkStack: [] as StackNode[], + mutableChunk: this.roots as BasicChunk | undefined, + getParent() { + assert( + this.mutableChunkStack.length > 0, + 0x532 /* invalid access to root's parent */, + ); + return this.mutableChunkStack[this.mutableChunkStack.length - 1]; + }, + moveIn(index: number, toAttach: DetachedField): number { + const detachedKey = detachedFieldAsKey(toAttach); + const children = thisForest.roots.fields.get(detachedKey) ?? []; + thisForest.roots.fields.delete(detachedKey); + if (children.length === 0) { + return 0; // Prevent creating 0 sized fields when inserting empty into empty. + } - const parent = getParent(); - const destinationField = getOrAddEmptyToMap(parent.mutableChunk.fields, parent.key); - // TODO: this will fail for very large moves due to argument limits. - destinationField.splice(index, 0, ...children); + const parent = this.getParent(); + const destinationField = getOrAddEmptyToMap(parent.mutableChunk.fields, parent.key); + // TODO: this will fail for very large moves due to argument limits. + destinationField.splice(index, 0, ...children); - return children.length; - }; - const visitor = { - onDelete: (index: number, count: number): void => { - visitor.onMoveOut(index, count); + return children.length; + }, + fork() { + return { ...this, mutableChunkStack: [...this.mutableChunkStack] }; + }, + free(): void { + this.mutableChunk = undefined; + this.mutableChunkStack.length = 0; + }, + onDelete(index: number, count: number): void { + this.onMoveOut(index, count); }, - onInsert: (index: number, content: Delta.ProtoNodes): void => { - const chunks: TreeChunk[] = content.map((c) => chunkTree(c, this.chunker)); - const field = this.newDetachedField(); - this.roots.fields.set(detachedFieldAsKey(field), chunks); - moveIn(index, field); + onInsert(index: number, content: Delta.ProtoNodes): void { + const chunks: TreeChunk[] = content.map((c) => chunkTree(c, thisForest.chunker)); + const field = thisForest.newDetachedField(); + thisForest.roots.fields.set(detachedFieldAsKey(field), chunks); + this.moveIn(index, field); }, - onMoveOut: (index: number, count: number, id?: Delta.MoveId): void => { - const parent = getParent(); + onMoveOut(index: number, count: number, id?: Delta.MoveId): void { + const parent = this.getParent(); const sourceField = parent.mutableChunk.fields.get(parent.key) ?? []; const newField = sourceField.splice(index, count); if (id !== undefined) { - const detached = this.newDetachedField(); + const detached = thisForest.newDetachedField(); const key = detachedFieldAsKey(detached); if (newField.length > 0) { - this.roots.fields.set(key, newField); + thisForest.roots.fields.set(key, newField); } moves.set(id, detached); } else { @@ -151,15 +162,15 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { parent.mutableChunk.fields.delete(parent.key); } }, - onMoveIn: (index: number, count: number, id: Delta.MoveId): void => { + onMoveIn(index: number, count: number, id: Delta.MoveId): void { const toAttach = moves.get(id) ?? fail("move in without move out"); moves.delete(id); - const countMoved = moveIn(index, toAttach); + const countMoved = this.moveIn(index, toAttach); assert(countMoved === count, 0x533 /* counts must match */); }, - enterNode: (index: number): void => { - assert(mutableChunk === undefined, 0x535 /* should be in field */); - const parent = getParent(); + enterNode(index: number): void { + assert(this.mutableChunk === undefined, 0x535 /* should be in field */); + const parent = this.getParent(); const chunks = parent.mutableChunk.fields.get(parent.key) ?? fail("missing edited field"); let indexWithinChunk = index; @@ -179,7 +190,7 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { // // Maybe build path when visitor navigates then lazily sync to chunk tree when editing? const newChunks = mapCursorField(found.cursor(), (cursor) => - basicChunkTree(cursor, this.chunker), + basicChunkTree(cursor, thisForest.chunker), ); // TODO: this could fail for really long chunks being split (due to argument count limits). // Current implementations of chunks shouldn't ever be that long, but it could be an issue if they get bigger. @@ -190,25 +201,25 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { } assert(found instanceof BasicChunk, 0x536 /* chunk should have been normalized */); if (found.isShared()) { - mutableChunk = chunks[indexOfChunk] = found.clone(); + this.mutableChunk = chunks[indexOfChunk] = found.clone(); found.referenceRemoved(); } else { - mutableChunk = found; + this.mutableChunk = found; } }, - exitNode: (index: number): void => { - assert(mutableChunk !== undefined, 0x537 /* should be in node */); - mutableChunk = undefined; + exitNode(index: number): void { + assert(this.mutableChunk !== undefined, 0x537 /* should be in node */); + this.mutableChunk = undefined; }, - enterField: (key: FieldKey): void => { - assert(mutableChunk !== undefined, 0x538 /* should be in node */); - mutableChunkStack.push({ key, mutableChunk }); - mutableChunk = undefined; + enterField(key: FieldKey): void { + assert(this.mutableChunk !== undefined, 0x538 /* should be in node */); + this.mutableChunkStack.push({ key, mutableChunk: this.mutableChunk }); + this.mutableChunk = undefined; }, - exitField: (key: FieldKey): void => { - const top = mutableChunkStack.pop() ?? fail("should not be at root"); - assert(mutableChunk === undefined, 0x539 /* should be in field */); - mutableChunk = top.mutableChunk; + exitField(key: FieldKey): void { + const top = this.mutableChunkStack.pop() ?? fail("should not be at root"); + assert(this.mutableChunk === undefined, 0x539 /* should be in field */); + this.mutableChunk = top.mutableChunk; }, }; visitDelta(delta, visitor); diff --git a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts index 960ddb1d07aa..8f5e7b91513d 100644 --- a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts @@ -112,7 +112,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { const moves: Map = new Map(); const cursor: Cursor = this.allocateCursor(); cursor.setToAboveDetachedSequences(); - const moveIn = (index: number, toAttach: DetachedField): number => { + const moveIn = (index: number, toAttach: DetachedField, moveInCursor: Cursor): number => { const detachedKey = detachedFieldAsKey(toAttach); const children = getMapTreeField(this.roots, detachedKey, false); this.roots.fields.delete(detachedKey); @@ -120,7 +120,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { return 0; // Prevent creating 0 sized fields when inserting empty into empty. } - const [parent, key] = cursor.getParent(); + const [parent, key] = moveInCursor.getParent(); const destinationField = getMapTreeField(parent, key, true); assertValidIndex(index, destinationField, true); // TODO: this will fail for very large moves due to argument limits. @@ -128,15 +128,27 @@ class ObjectForest extends SimpleDependee implements IEditableForest { return children.length; }; + // eslint-disable-next-line @typescript-eslint/no-this-alias + const thisForest = this; + const cursors: ITreeSubscriptionCursor[] = [cursor]; const visitor = { - onDelete: (index: number, count: number): void => { - visitor.onMoveOut(index, count); + cursor, + fork() { + const newCursor = this.cursor.fork(); + cursors.push(newCursor); + return { ...visitor, cursor: newCursor }; }, - onInsert: (index: number, content: Delta.ProtoNode[]): void => { - const range = this.add(content); - moveIn(index, range); + free() { + this.cursor.free(); }, - onMoveOut: (index: number, count: number, id?: Delta.MoveId): void => { + onDelete(index: number, count: number): void { + this.onMoveOut(index, count); + }, + onInsert(index: number, content: Delta.ProtoNode[]): void { + const range = thisForest.add(content); + moveIn(index, range, this.cursor); + }, + onMoveOut(index: number, count: number, id?: Delta.MoveId): void { const [parent, key] = cursor.getParent(); const sourceField = getMapTreeField(parent, key, false); const startIndex = index; @@ -148,26 +160,34 @@ class ObjectForest extends SimpleDependee implements IEditableForest { 0x371 /* detached range's end must be after its start */, ); const newField = sourceField.splice(startIndex, endIndex - startIndex); - const field = this.addFieldAsDetached(newField); + const field = thisForest.addFieldAsDetached(newField); if (id !== undefined) { moves.set(id, field); } else { - this.delete(field); + thisForest.delete(field); } if (sourceField.length === 0) { parent.fields.delete(key); } }, - onMoveIn: (index: number, count: number, id: Delta.MoveId): void => { + onMoveIn(index: number, count: number, id: Delta.MoveId): void { const toAttach = moves.get(id) ?? fail("move in without move out"); moves.delete(id); - const countMoved = moveIn(index, toAttach); + const countMoved = moveIn(index, toAttach, this.cursor); assert(countMoved === count, 0x369 /* counts must match */); }, - enterNode: (index: number): void => cursor.enterNode(index), - exitNode: (index: number): void => cursor.exitNode(), - enterField: (key: FieldKey): void => cursor.enterField(key), - exitField: (key: FieldKey): void => cursor.exitField(), + enterNode(index: number): void { + this.cursor.enterNode(index); + }, + exitNode(index: number): void { + this.cursor.exitNode(); + }, + enterField(key: FieldKey): void { + this.cursor.enterField(key); + }, + exitField(key: FieldKey): void { + this.cursor.exitField(); + }, }; visitDelta(delta, visitor); cursor.free(); From 7154f03ffa308f09fd8a99e91184873cbc6fb882 Mon Sep 17 00:00:00 2001 From: Jenn Date: Fri, 1 Sep 2023 21:05:11 +0000 Subject: [PATCH 2/6] Changes --- api-report/tree2.api.md | 4 +- .../tree2/src/core/forest/editableForest.ts | 7 +++- .../dds/tree2/src/core/tree/anchorSet.ts | 11 ++++-- .../dds/tree2/src/core/tree/visitDelta.ts | 10 +++++ .../chunked-forest/chunkedForest.ts | 25 +++++++------ .../forestRepairDataStore.ts | 4 +- .../object-forest/objectForest.ts | 26 +++++++------ .../tree2/src/shared-tree/sharedTreeView.ts | 7 +++- .../chunked-forest/chunkedForest.spec.ts | 3 +- .../defaultChangeFamily.spec.ts | 3 +- .../forestRepairDataStore.spec.ts | 3 +- .../dds/tree2/src/test/forestTestSuite.ts | 37 ++++++++++--------- .../dds/tree2/src/test/tree/anchorSet.spec.ts | 31 ++++++++-------- 13 files changed, 101 insertions(+), 70 deletions(-) diff --git a/api-report/tree2.api.md b/api-report/tree2.api.md index 23393c6094f2..2d4a85117856 100644 --- a/api-report/tree2.api.md +++ b/api-report/tree2.api.md @@ -71,9 +71,9 @@ export type AnchorsCompare = CompareFunction; // @alpha @sealed export class AnchorSet implements ISubscribable, AnchorLocator { - applyDelta(delta: Delta.Root): void; // (undocumented) forget(anchor: Anchor): void; + getVisitor(): DeltaVisitor; internalizePath(originalPath: UpPath): UpPath; isEmpty(): boolean; // (undocumented) @@ -850,7 +850,7 @@ export interface IdRange { // @alpha export interface IEditableForest extends IForestSubscription { readonly anchors: AnchorSet; - applyDelta(delta: Delta.Root): void; + getVisitor(): DeltaVisitor; } // @alpha diff --git a/experimental/dds/tree2/src/core/forest/editableForest.ts b/experimental/dds/tree2/src/core/forest/editableForest.ts index de5c9e97775e..51e6bded76eb 100644 --- a/experimental/dds/tree2/src/core/forest/editableForest.ts +++ b/experimental/dds/tree2/src/core/forest/editableForest.ts @@ -12,6 +12,8 @@ import { Anchor, ITreeCursorSynchronous, rootFieldKey, + DeltaVisitor, + visitDelta, } from "../tree"; import { IForestSubscription, ITreeSubscriptionCursor } from "./forest"; @@ -34,7 +36,7 @@ export interface IEditableForest extends IForestSubscription { * Applies the supplied Delta to the forest. * Does NOT update anchors. */ - applyDelta(delta: Delta.Root): void; + getVisitor(): DeltaVisitor; } /** @@ -50,7 +52,8 @@ export function initializeForest( ): void { assert(forest.isEmpty, 0x747 /* forest must be empty */); const insert: Delta.Insert = { type: Delta.MarkType.Insert, content }; - forest.applyDelta(new Map([[rootFieldKey, [insert]]])); + const deltaVisitor = forest.getVisitor(); + visitDelta(new Map([[rootFieldKey, [insert]]]), deltaVisitor); } // TODO: Types below here may be useful for input into edit building APIs, but are no longer used here directly. diff --git a/experimental/dds/tree2/src/core/tree/anchorSet.ts b/experimental/dds/tree2/src/core/tree/anchorSet.ts index c0f117ff3d8d..b584443b580c 100644 --- a/experimental/dds/tree2/src/core/tree/anchorSet.ts +++ b/experimental/dds/tree2/src/core/tree/anchorSet.ts @@ -19,7 +19,7 @@ import { FieldKey } from "../schema-stored"; import { UpPath } from "./pathTree"; import { Value, detachedFieldAsKey, DetachedField, EmptyKey } from "./types"; import { PathVisitor } from "./visitPath"; -import { visitDelta } from "./visitDelta"; +import { DeltaVisitor, visitDelta } from "./visitDelta"; import * as Delta from "./delta"; /** @@ -556,7 +556,7 @@ export class AnchorSet implements ISubscribable, AnchorLoca /** * Updates the anchors according to the changes described in the given delta */ - public applyDelta(delta: Delta.Root): void { + public getVisitor(): DeltaVisitor { const moveTable = new Map(); // eslint-disable-next-line @typescript-eslint/no-this-alias const thisAnchorSet = this; @@ -584,6 +584,10 @@ export class AnchorSet implements ISubscribable, AnchorLoca pathVisitors: new Map>(), parentField: undefined as FieldKey | undefined, parent: undefined as UpPath | undefined, + beforeDelta(delta: Delta.Root): void {}, + afterDelta(delta: Delta.Root): void { + thisAnchorSet.events.emit("treeChanging", thisAnchorSet); + }, fork() { return { ...this }; }, @@ -723,8 +727,7 @@ export class AnchorSet implements ISubscribable, AnchorLoca this.parentField = undefined; }, }; - this.events.emit("treeChanging", this); - visitDelta(delta, visitor); + return visitor; } } diff --git a/experimental/dds/tree2/src/core/tree/visitDelta.ts b/experimental/dds/tree2/src/core/tree/visitDelta.ts index 4cf644efd578..c9f15e9ee3bd 100644 --- a/experimental/dds/tree2/src/core/tree/visitDelta.ts +++ b/experimental/dds/tree2/src/core/tree/visitDelta.ts @@ -79,6 +79,7 @@ import * as Delta from "./delta"; * @param visitor - The object to notify of the changes encountered. */ export function visitDelta(delta: Delta.Root, visitor: DeltaVisitor): void { + visitor.beforeDelta(delta); const modsToMovedTrees = new Map(); const movedOutNodes: RangeMap = []; const containsMovesOrDeletes = visitFieldMarks(delta, visitor, { @@ -95,9 +96,18 @@ export function visitDelta(delta: Delta.Root, visitor: DeltaVisitor): void { movedOutRanges: movedOutNodes, }); } + visitor.afterDelta(delta); } export interface DeltaVisitor { + /** + * Called before the delta is visited. + */ + beforeDelta(delta: Delta.Root): void; + /** + * Called after the delta is visited. + */ + afterDelta(delta: Delta.Root): void; /** * Forks the current visitor. * Any fork produced this way is freed before the visit terminates. diff --git a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts index d9f973ce99c7..05778ca23f29 100644 --- a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts @@ -25,6 +25,7 @@ import { ITreeSubscriptionCursorState, rootFieldKey, mapCursorField, + DeltaVisitor, } from "../../core"; import { brand, fail, getOrAddEmptyToMap } from "../../util"; import { createEmitter } from "../../events"; @@ -85,16 +86,9 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { this.anchors.forget(anchor); } - public applyDelta(delta: Delta.Root): void { - this.events.emit("beforeDelta", delta); - this.invalidateDependents(); - + public getVisitor(): DeltaVisitor { const moves: Map = new Map(); - if (this.roots.isShared()) { - this.roots = this.roots.clone(); - } - // eslint-disable-next-line @typescript-eslint/no-this-alias const thisForest = this; @@ -125,6 +119,17 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { return children.length; }, + beforeDelta(delta: Delta.Root): void { + thisForest.events.emit("beforeDelta", delta); + thisForest.invalidateDependents(); + + if (thisForest.roots.isShared()) { + thisForest.roots = thisForest.roots.clone(); + } + }, + afterDelta(delta: Delta.Root): void { + thisForest.events.emit("afterDelta", delta); + }, fork() { return { ...this, mutableChunkStack: [...this.mutableChunkStack] }; }, @@ -222,9 +227,7 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { this.mutableChunk = top.mutableChunk; }, }; - visitDelta(delta, visitor); - - this.events.emit("afterDelta", delta); + return visitor; } private nextDetachedFieldIdentifier = 0; diff --git a/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts b/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts index 6aa142288799..aebb633490c5 100644 --- a/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts +++ b/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts @@ -22,6 +22,7 @@ import { RevisionTag, SparseNode, UpPath, + visitDelta, } from "../core"; import { chunkTree, TreeChunk, defaultChunkPolicy } from "./chunked-forest"; @@ -200,7 +201,8 @@ export class ForestRepairDataStoreProvider implements IRepairDataStoreP public applyChange(change: TChange): void { if (this.frozenForest === undefined) { - this.forest.applyDelta(this.intoDelta(change)); + const visitor = this.forest.getVisitor(); + visitDelta(this.intoDelta(change), visitor); } } diff --git a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts index 8f5e7b91513d..527cbb4a16e2 100644 --- a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts @@ -31,6 +31,7 @@ import { FieldUpPath, ForestEvents, PathRootPrefix, + DeltaVisitor, } from "../../core"; import { brand, fail, assertValidIndex } from "../../util"; import { CursorWithNode, SynchronousCursor } from "../treeCursorUtils"; @@ -95,14 +96,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { this.anchors.forget(anchor); } - public applyDelta(delta: Delta.Root): void { - this.events.emit("beforeDelta", delta); - this.invalidateDependents(); - assert( - this.currentCursors.size === 0, - 0x374 /* No cursors can be current when modifying forest */, - ); - + public getVisitor(): DeltaVisitor { // Note: This code uses cursors, however it also modifies the tree. // In general this is not safe, but this code happens to only modify the tree below the current cursor location, // which happens to work. @@ -133,6 +127,17 @@ class ObjectForest extends SimpleDependee implements IEditableForest { const cursors: ITreeSubscriptionCursor[] = [cursor]; const visitor = { cursor, + beforeDelta(delta: Delta.Root) { + thisForest.events.emit("beforeDelta", delta); + thisForest.invalidateDependents(); + assert( + thisForest.currentCursors.size === 0, + 0x374 /* No cursors can be current when modifying forest */, + ); + }, + afterDelta(delta: Delta.Root) { + thisForest.events.emit("afterDelta", delta); + }, fork() { const newCursor = this.cursor.fork(); cursors.push(newCursor); @@ -189,10 +194,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { this.cursor.exitField(); }, }; - visitDelta(delta, visitor); - cursor.free(); - - this.events.emit("afterDelta", delta); + return visitor; } private nextRange = 0; diff --git a/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts b/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts index ec7966d73ef0..7ba8eeb2fdc5 100644 --- a/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts +++ b/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts @@ -17,6 +17,7 @@ import { UndoRedoManager, LocalCommitSource, schemaDataIsEmpty, + visitDelta, } from "../core"; import { HasListeners, IEmitter, ISubscribable, createEmitter } from "../events"; import { @@ -432,8 +433,10 @@ export class SharedTreeView implements ISharedTreeBranchView { branch.on("change", ({ change }) => { if (change !== undefined) { const delta = this.changeFamily.intoDelta(change); - this.forest.anchors.applyDelta(delta); - this.forest.applyDelta(delta); + const anchorVisitor = this.forest.anchors.getVisitor(); + const visitor = this.forest.getVisitor(); + visitDelta(delta, anchorVisitor); + visitDelta(delta, visitor); this.nodeKeyIndex.scanKeys(this.context); this.events.emit("afterBatch"); } diff --git a/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts index 5589d9cffb3a..b3b14553041b 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts @@ -34,6 +34,7 @@ import { Delta, IForestSubscription, StoredSchemaRepository, + visitDelta, } from "../../../core"; import { jsonObject } from "../../../domains"; import { @@ -143,7 +144,7 @@ describe("ChunkedForest", () => { // Captured reference owns a ref count making it shared. assert(chunk.isShared()); // Delete from forest, removing the forest's ref, making chunk not shared again. - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); assert(!chunk.isShared()); compareForest(forest, []); diff --git a/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts index 9e114dbc3728..e121844e59b7 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts @@ -18,6 +18,7 @@ import { rootFieldKey, TaggedChange, UpPath, + visitDelta, } from "../../../core"; import { jsonNumber, jsonObject, jsonString } from "../../../domains"; import { @@ -121,7 +122,7 @@ function initializeEditableForest(data?: JsonableTree): { changes.push({ revision: currentRevision, change }); const delta = defaultChangeFamily.intoDelta(change); deltas.push(delta); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); currentRevision = mintRevisionTag(); }); return { diff --git a/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts index 9967d62ebab1..c61d7012d781 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts @@ -13,6 +13,7 @@ import { RevisionTag, rootFieldKey, UpPath, + visitDelta, } from "../../core"; import { jsonNumber, jsonObject } from "../../domains"; import { @@ -83,7 +84,7 @@ describe("ForestRepairDataStore", () => { ], ]); store.capture(delta1, revision1); - forest.applyDelta(delta1); + visitDelta(delta1, forest.getVisitor()); const delta2 = new Map([ [ rootFieldKey, diff --git a/experimental/dds/tree2/src/test/forestTestSuite.ts b/experimental/dds/tree2/src/test/forestTestSuite.ts index cc60e7fe9f19..677d72bc386e 100644 --- a/experimental/dds/tree2/src/test/forestTestSuite.ts +++ b/experimental/dds/tree2/src/test/forestTestSuite.ts @@ -24,6 +24,7 @@ import { EmptyKey, ValueSchema, FieldUpPath, + visitDelta, } from "../core"; import { cursorToJsonObject, @@ -173,7 +174,7 @@ export function testForest(config: ForestTestConfiguration): void { type: Delta.MarkType.Insert, content: [singleJsonCursor([])], }; - forest.applyDelta(new Map([[brand("different root"), [insert]]])); + visitDelta(new Map([[brand("different root"), [insert]]]), forest.getVisitor()); assert(!forest.isEmpty); }); @@ -343,8 +344,8 @@ export function testForest(config: ForestTestConfiguration): void { const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - forest.applyDelta(delta); - forest.anchors.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); + visitDelta(delta, forest.anchors.getVisitor()); assert.equal( forest.tryMoveCursorToNode(firstNodeAnchor, cursor), @@ -429,7 +430,7 @@ export function testForest(config: ForestTestConfiguration): void { const clone = forest.clone(forest.schema, forest.anchors); const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - clone.applyDelta(delta); + visitDelta(delta, clone.getVisitor()); // Check the clone has the new value const cloneReader = clone.allocateCursor(); @@ -456,7 +457,7 @@ export function testForest(config: ForestTestConfiguration): void { const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - assert.throws(() => forest.applyDelta(delta)); + assert.throws(() => visitDelta(delta, forest.getVisitor())); }); } @@ -487,7 +488,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [setField]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -506,7 +507,7 @@ export function testForest(config: ForestTestConfiguration): void { const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [0, mark]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); // Inspect resulting tree: should just have `2`. const reader = forest.allocateCursor(); @@ -531,7 +532,7 @@ export function testForest(config: ForestTestConfiguration): void { const skip: Delta.Skip = 1; const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [skip, mark]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); // Inspect resulting tree: should just have `1`. const reader = forest.allocateCursor(); @@ -552,7 +553,7 @@ export function testForest(config: ForestTestConfiguration): void { content: [singleJsonCursor(3)], }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -589,7 +590,7 @@ export function testForest(config: ForestTestConfiguration): void { fields: new Map([[xField, [moveOut]]]), }; const delta: Delta.Root = new Map([[rootFieldKey, [mark, moveIn]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -623,7 +624,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [modify]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); assert(reader.firstNode()); @@ -659,7 +660,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -740,7 +741,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -774,15 +775,15 @@ export function testForest(config: ForestTestConfiguration): void { const delta: Delta.Root = new Map([[rootFieldKey, [insert]]]); assert.deepEqual(dependent.tokens, []); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); assert.deepEqual(dependent.tokens.length, 1); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); assert.deepEqual(dependent.tokens.length, 2); // Remove the dependency so the dependent stops getting invalidation messages forest.removeDependent(dependent); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); assert.deepEqual(dependent.tokens.length, 2); }); @@ -826,7 +827,7 @@ export function testForest(config: ForestTestConfiguration): void { ]); const expected: JsonCompatible[] = [{ y: 1 }]; initializeForest(forest, [singleJsonCursor(nestedContent)]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const readCursor = forest.allocateCursor(); moveToDetachedField(forest, readCursor); const actual = mapCursorField(readCursor, cursorToJsonObject); @@ -870,7 +871,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [modify]]]); - forest.applyDelta(delta); + visitDelta(delta, forest.getVisitor()); const expectedCursor = cursorForTypedTreeData({ schema }, root, { x: [], y: [1, 2], diff --git a/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts b/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts index 5e3e1ad100a9..0eb42e7b60fe 100644 --- a/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts +++ b/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts @@ -16,6 +16,7 @@ import { UpPath, clonePath, rootFieldKey, + visitDelta, } from "../../core"; import { brand } from "../../util"; import { expectEqualPaths } from "../utils"; @@ -61,7 +62,7 @@ describe("AnchorSet", () => { }; const delta = new Map([[rootFieldKey, [1, moveOut, 1, moveIn]]]); - anchors.applyDelta(delta); + visitDelta(delta, anchors.getVisitor()); checkEquality(anchors.locate(anchor0), makePath([rootFieldKey, 0])); checkEquality(anchors.locate(anchor1), makePath([rootFieldKey, 2])); checkEquality(anchors.locate(anchor2), makePath([rootFieldKey, 1])); @@ -76,7 +77,7 @@ describe("AnchorSet", () => { content: [node, node].map(singleTextCursor), }; - anchors.applyDelta(makeDelta(insert, makePath([fieldFoo, 4]))); + visitDelta(makeDelta(insert, makePath([fieldFoo, 4])), anchors.getVisitor()); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 7], [fieldBar, 4])); checkEquality(anchors.locate(anchor2), makePath([fieldFoo, 3], [fieldBaz, 2])); @@ -90,7 +91,7 @@ describe("AnchorSet", () => { count: 1, }; - anchors.applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 4]))); + visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 4])), anchors.getVisitor()); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 4], [fieldBar, 4])); checkEquality(anchors.locate(anchor2), path2); assert.equal(anchors.locate(anchor3), undefined); @@ -105,7 +106,7 @@ describe("AnchorSet", () => { count: 1, }; - anchors.applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 5]))); + visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 5])), anchors.getVisitor()); assert.equal(anchors.locate(anchor4), undefined); assert.equal(anchors.locate(anchor1), undefined); assert.doesNotThrow(() => anchors.forget(anchor4)); @@ -116,14 +117,14 @@ describe("AnchorSet", () => { assert.throws(() => anchors.locate(anchor1)); checkEquality(anchors.locate(anchor2), path2); - anchors.applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 3]))); + visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 3])), anchors.getVisitor()); checkEquality(anchors.locate(anchor2), undefined); assert.doesNotThrow(() => anchors.forget(anchor2)); assert.throws(() => anchors.locate(anchor2)); // The index of anchor3 has changed from 4 to 3 because of the deletion of the node at index 3. checkEquality(anchors.locate(anchor3), makePath([fieldFoo, 3])); - anchors.applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 3]))); + visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 3])), anchors.getVisitor()); checkEquality(anchors.locate(anchor3), undefined); assert.doesNotThrow(() => anchors.forget(anchor3)); assert.throws(() => anchors.locate(anchor3)); @@ -149,7 +150,7 @@ describe("AnchorSet", () => { }; const delta = new Map([[fieldFoo, [3, moveOut, 1, modify]]]); - anchors.applyDelta(delta); + visitDelta(delta, anchors.getVisitor()); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 4], [fieldBar, 5])); checkEquality( anchors.locate(anchor2), @@ -232,7 +233,7 @@ describe("AnchorSet", () => { }; log.expect([]); - anchors.applyDelta(new Map([[rootFieldKey, [0, deleteMark]]])); + visitDelta(new Map([[rootFieldKey, [0, deleteMark]]]), anchors.getVisitor()); log.expect([ ["root childrenChange", 1], @@ -253,7 +254,7 @@ describe("AnchorSet", () => { type: Delta.MarkType.Insert, content: [singleTextCursor({ type: jsonString.name, value: "x" })], }; - anchors.applyDelta(new Map([[rootFieldKey, [deleteMark, insertMark]]])); + visitDelta(new Map([[rootFieldKey, [deleteMark, insertMark]]]), anchors.getVisitor()); log.expect([ ["afterDelete", 1], @@ -262,12 +263,12 @@ describe("AnchorSet", () => { ]); log.clear(); - anchors.applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 5]))); + visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 5])), anchors.getVisitor()); log.expect([["root treeChange", 1]]); log.clear(); - anchors.applyDelta(new Map([[rootFieldKey, [0, deleteMark]]])); + visitDelta(new Map([[rootFieldKey, [0, deleteMark]]]), anchors.getVisitor()); log.expect([ ["root childrenChange", 1], ["root treeChange", 1], @@ -285,7 +286,7 @@ describe("AnchorSet", () => { }; const log = new UnorderedTestLogger(); const anchors = new AnchorSet(); - anchors.applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 3]))); + visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 3])), anchors.getVisitor()); const anchor0 = anchors.track(makePath([rootFieldKey, 0])); const node0 = anchors.locate(anchor0) ?? assert.fail(); const pathVisitor: PathVisitor = { @@ -303,14 +304,14 @@ describe("AnchorSet", () => { }, }; const unsubscribePathVisitor = node0.on("subtreeChanging", (n: AnchorNode) => pathVisitor); - anchors.applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4]))); + visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4])), anchors.getVisitor()); log.expect([["visitSubtreeChange.onInsert-foo-4", 1]]); log.clear(); - anchors.applyDelta(makeDelta(deleteMark, makePath([rootFieldKey, 0], [fieldFoo, 5]))); + visitDelta(makeDelta(deleteMark, makePath([rootFieldKey, 0], [fieldFoo, 5])), anchors.getVisitor()); log.expect([["visitSubtreeChange.onDelete-foo-5-1", 1]]); log.clear(); unsubscribePathVisitor(); - anchors.applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4]))); + visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4])), anchors.getVisitor()); log.expect([]); }); }); From 5019f73c6cdd0abbccf1c359a08b13a040d073b9 Mon Sep 17 00:00:00 2001 From: Yann Achard Date: Fri, 1 Sep 2023 15:14:10 -0700 Subject: [PATCH 3/6] Tweaks --- api-report/tree2.api.md | 31 +++++++++-- .../tree2/src/core/forest/editableForest.ts | 13 +++-- .../dds/tree2/src/core/forest/forest.ts | 9 ++- experimental/dds/tree2/src/core/index.ts | 1 + .../dds/tree2/src/core/tree/anchorSet.ts | 50 +++++++++-------- experimental/dds/tree2/src/core/tree/index.ts | 2 +- .../dds/tree2/src/core/tree/visitDelta.ts | 32 +++++------ .../chunked-forest/chunkedForest.ts | 55 ++++++++++--------- .../editable-tree/editableTreeContext.ts | 2 +- .../forestRepairDataStore.ts | 5 +- .../object-forest/objectForest.ts | 50 +++++++++-------- experimental/dds/tree2/src/index.ts | 1 + .../tree2/src/shared-tree/sharedTreeView.ts | 8 +-- .../chunked-forest/chunkedForest.spec.ts | 4 +- .../defaultChangeFamily.spec.ts | 4 +- .../editable-tree/editableTreeContext.spec.ts | 2 +- .../forestRepairDataStore.spec.ts | 4 +- .../dds/tree2/src/test/forestTestSuite.ts | 40 +++++++------- .../dds/tree2/src/test/tree/anchorSet.spec.ts | 32 +++++------ 19 files changed, 186 insertions(+), 159 deletions(-) diff --git a/api-report/tree2.api.md b/api-report/tree2.api.md index 2d4a85117856..3e1510f38cfd 100644 --- a/api-report/tree2.api.md +++ b/api-report/tree2.api.md @@ -71,9 +71,9 @@ export type AnchorsCompare = CompareFunction; // @alpha @sealed export class AnchorSet implements ISubscribable, AnchorLocator { + acquireVisitor(): DeltaVisitor; // (undocumented) forget(anchor: Anchor): void; - getVisitor(): DeltaVisitor; internalizePath(originalPath: UpPath): UpPath; isEmpty(): boolean; // (undocumented) @@ -428,6 +428,28 @@ declare namespace Delta { } export { Delta } +// @alpha +export interface DeltaVisitor { + // (undocumented) + enterField(key: FieldKey): void; + // (undocumented) + enterNode(index: number): void; + // (undocumented) + exitField(key: FieldKey): void; + // (undocumented) + exitNode(index: number): void; + // (undocumented) + free(): void; + // (undocumented) + onDelete(index: number, count: number): void; + // (undocumented) + onInsert(index: number, content: Delta.ProtoNodes): void; + // (undocumented) + onMoveIn(index: number, count: number, id: Delta.MoveId): void; + // (undocumented) + onMoveOut(index: number, count: number, id: Delta.MoveId): void; +} + // @alpha export interface Dependee extends NamedComputation { registerDependent(dependent: Dependent): boolean; @@ -747,8 +769,8 @@ export const forbiddenFieldKindIdentifier = "Forbidden"; // @alpha export interface ForestEvents { - afterDelta(delta: Delta.Root): void; - beforeDelta(delta: Delta.Root): void; + afterChange(): void; + beforeChange(): void; } // @alpha @@ -849,8 +871,9 @@ export interface IdRange { // @alpha export interface IEditableForest extends IForestSubscription { + // (undocumented) + acquireVisitor(): DeltaVisitor; readonly anchors: AnchorSet; - getVisitor(): DeltaVisitor; } // @alpha diff --git a/experimental/dds/tree2/src/core/forest/editableForest.ts b/experimental/dds/tree2/src/core/forest/editableForest.ts index 51e6bded76eb..ce5587e81a32 100644 --- a/experimental/dds/tree2/src/core/forest/editableForest.ts +++ b/experimental/dds/tree2/src/core/forest/editableForest.ts @@ -13,7 +13,7 @@ import { ITreeCursorSynchronous, rootFieldKey, DeltaVisitor, - visitDelta, + applyDelta, } from "../tree"; import { IForestSubscription, ITreeSubscriptionCursor } from "./forest"; @@ -33,10 +33,12 @@ export interface IEditableForest extends IForestSubscription { readonly anchors: AnchorSet; /** - * Applies the supplied Delta to the forest. - * Does NOT update anchors. + * @returns a visitor that can be used to mutate the forest. + * Mutating the forest does NOT update anchors. + * The visitor must be released after use. + * It is invalid to acquire a visitor without releasing the previous one. */ - getVisitor(): DeltaVisitor; + acquireVisitor(): DeltaVisitor; } /** @@ -52,8 +54,7 @@ export function initializeForest( ): void { assert(forest.isEmpty, 0x747 /* forest must be empty */); const insert: Delta.Insert = { type: Delta.MarkType.Insert, content }; - const deltaVisitor = forest.getVisitor(); - visitDelta(new Map([[rootFieldKey, [insert]]]), deltaVisitor); + applyDelta(new Map([[rootFieldKey, [insert]]]), forest); } // TODO: Types below here may be useful for input into edit building APIs, but are no longer used here directly. diff --git a/experimental/dds/tree2/src/core/forest/forest.ts b/experimental/dds/tree2/src/core/forest/forest.ts index cfd9cfc88d67..16bff20c1582 100644 --- a/experimental/dds/tree2/src/core/forest/forest.ts +++ b/experimental/dds/tree2/src/core/forest/forest.ts @@ -10,7 +10,6 @@ import { StoredSchemaRepository, FieldKey } from "../schema-stored"; import { Anchor, AnchorSet, - Delta, DetachedField, detachedFieldAsKey, ITreeCursor, @@ -35,14 +34,14 @@ import type { IEditableForest } from "./editableForest"; */ export interface ForestEvents { /** - * Delta is about to be applied to forest. + * The forest is about to be changed. */ - beforeDelta(delta: Delta.Root): void; + beforeChange(): void; /** - * Delta was just applied to forest. + * The forest was just changed. */ - afterDelta(delta: Delta.Root): void; + afterChange(): void; } /** diff --git a/experimental/dds/tree2/src/core/index.ts b/experimental/dds/tree2/src/core/index.ts index 06b60559addf..191c7706c675 100644 --- a/experimental/dds/tree2/src/core/index.ts +++ b/experimental/dds/tree2/src/core/index.ts @@ -55,6 +55,7 @@ export { detachedFieldAsKey, keyAsDetachedField, visitDelta, + applyDelta, setGenericTreeField, DeltaVisitor, PathVisitor, diff --git a/experimental/dds/tree2/src/core/tree/anchorSet.ts b/experimental/dds/tree2/src/core/tree/anchorSet.ts index b584443b580c..88a72425ca38 100644 --- a/experimental/dds/tree2/src/core/tree/anchorSet.ts +++ b/experimental/dds/tree2/src/core/tree/anchorSet.ts @@ -19,7 +19,7 @@ import { FieldKey } from "../schema-stored"; import { UpPath } from "./pathTree"; import { Value, detachedFieldAsKey, DetachedField, EmptyKey } from "./types"; import { PathVisitor } from "./visitPath"; -import { DeltaVisitor, visitDelta } from "./visitDelta"; +import { DeltaVisitor } from "./visitDelta"; import * as Delta from "./delta"; /** @@ -214,6 +214,8 @@ export class AnchorSet implements ISubscribable, AnchorLoca // For now use this more encapsulated approach with maps. private readonly anchorToPath: Map = new Map(); + private activeVisitor?: DeltaVisitor; + public on( eventName: K, listener: AnchorSetRootEvents[K], @@ -556,12 +558,15 @@ export class AnchorSet implements ISubscribable, AnchorLoca /** * Updates the anchors according to the changes described in the given delta */ - public getVisitor(): DeltaVisitor { + public acquireVisitor(): DeltaVisitor { + assert( + this.activeVisitor === undefined, + "Must release existing visitor before acquiring another", + ); const moveTable = new Map(); - // eslint-disable-next-line @typescript-eslint/no-this-alias - const thisAnchorSet = this; const visitor = { + anchorSet: this, // Run `withNode` on anchorNode for parent if there is such an anchorNode. // If at root, run `withRoot` instead. maybeWithNode(withNode: (anchorNode: PathNode) => void, withRoot?: () => void) { @@ -572,7 +577,7 @@ export class AnchorSet implements ISubscribable, AnchorLoca // TODO:Perf: // When traversing to a depth D when there are not anchors in that subtree, this goes O(D^2). // Delta traversal should early out in this case because no work is needed (and all move outs are known to not contain anchors). - this.parent = thisAnchorSet.internalizePath(this.parent); + this.parent = this.anchorSet.internalizePath(this.parent); if (this.parent instanceof PathNode) { withNode(this.parent); } @@ -584,21 +589,20 @@ export class AnchorSet implements ISubscribable, AnchorLoca pathVisitors: new Map>(), parentField: undefined as FieldKey | undefined, parent: undefined as UpPath | undefined, - beforeDelta(delta: Delta.Root): void {}, - afterDelta(delta: Delta.Root): void { - thisAnchorSet.events.emit("treeChanging", thisAnchorSet); - }, - fork() { - return { ...this }; + free() { + assert( + this.anchorSet.activeVisitor !== undefined, + "Multiple free calls for same visitor", + ); + this.anchorSet.activeVisitor = undefined; }, - free() {}, onDelete(start: number, count: number): void { assert(this.parentField !== undefined, 0x3a7 /* Must be in a field to delete */); this.maybeWithNode( (p) => { p.events.emit("childrenChanging", p); }, - () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), + () => this.anchorSet.events.emit("childrenChanging", this.anchorSet), ); const upPath: UpPath = { parent: this.parent, @@ -611,7 +615,7 @@ export class AnchorSet implements ISubscribable, AnchorLoca } } - thisAnchorSet.removeChildren( + this.anchorSet.removeChildren( { parent: this.parent, parentField: this.parentField, @@ -624,7 +628,7 @@ export class AnchorSet implements ISubscribable, AnchorLoca assert(this.parentField !== undefined, 0x3a8 /* Must be in a field to insert */); this.maybeWithNode( (p) => p.events.emit("childrenChanging", p), - () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), + () => this.anchorSet.events.emit("childrenChanging", this.anchorSet), ); const upPath: UpPath = { parent: this.parent, @@ -637,7 +641,7 @@ export class AnchorSet implements ISubscribable, AnchorLoca } } - thisAnchorSet.offsetChildren( + this.anchorSet.offsetChildren( { parent: this.parent, parentField: this.parentField, @@ -650,33 +654,33 @@ export class AnchorSet implements ISubscribable, AnchorLoca assert(this.parentField !== undefined, 0x3a9 /* Must be in a field to move out */); this.maybeWithNode( (p) => p.events.emit("childrenChanging", p), - () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), + () => this.anchorSet.events.emit("childrenChanging", this.anchorSet), ); - const fieldKey = thisAnchorSet.createEmptyDetachedField(); + const fieldKey = this.anchorSet.createEmptyDetachedField(); const source = { parent: this.parent, parentField: this.parentField, parentIndex: start, }; const destination = { - parent: thisAnchorSet.root, + parent: this.anchorSet.root, parentField: fieldKey, parentIndex: 0, }; - thisAnchorSet.moveChildren(source, destination, count); + this.anchorSet.moveChildren(source, destination, count); moveTable.set(id, destination); }, onMoveIn(start: number, count: number, id: Delta.MoveId): void { assert(this.parentField !== undefined, 0x3aa /* Must be in a field to move in */); this.maybeWithNode( (p) => p.events.emit("childrenChanging", p), - () => thisAnchorSet.events.emit("childrenChanging", thisAnchorSet), + () => this.anchorSet.events.emit("childrenChanging", this.anchorSet), ); const sourcePath = moveTable.get(id) ?? fail("Must visit a move in after its move out"); moveTable.delete(id); - thisAnchorSet.moveChildren( + this.anchorSet.moveChildren( sourcePath, { parent: this.parent, parentField: this.parentField, parentIndex: start }, count, @@ -727,6 +731,8 @@ export class AnchorSet implements ISubscribable, AnchorLoca this.parentField = undefined; }, }; + this.events.emit("treeChanging", this); + this.activeVisitor = visitor; return visitor; } } diff --git a/experimental/dds/tree2/src/core/tree/index.ts b/experimental/dds/tree2/src/core/tree/index.ts index 0011542ffba5..7fc9db6e00f2 100644 --- a/experimental/dds/tree2/src/core/tree/index.ts +++ b/experimental/dds/tree2/src/core/tree/index.ts @@ -67,7 +67,7 @@ export { NodeData, rootField, } from "./types"; -export { DeltaVisitor, visitDelta } from "./visitDelta"; +export { DeltaVisitor, visitDelta, applyDelta } from "./visitDelta"; export { PathVisitor } from "./visitPath"; // Split this up into separate import and export for compatibility with API-Extractor. diff --git a/experimental/dds/tree2/src/core/tree/visitDelta.ts b/experimental/dds/tree2/src/core/tree/visitDelta.ts index c9f15e9ee3bd..f6dd001069fe 100644 --- a/experimental/dds/tree2/src/core/tree/visitDelta.ts +++ b/experimental/dds/tree2/src/core/tree/visitDelta.ts @@ -79,7 +79,6 @@ import * as Delta from "./delta"; * @param visitor - The object to notify of the changes encountered. */ export function visitDelta(delta: Delta.Root, visitor: DeltaVisitor): void { - visitor.beforeDelta(delta); const modsToMovedTrees = new Map(); const movedOutNodes: RangeMap = []; const containsMovesOrDeletes = visitFieldMarks(delta, visitor, { @@ -96,32 +95,27 @@ export function visitDelta(delta: Delta.Root, visitor: DeltaVisitor): void { movedOutRanges: movedOutNodes, }); } - visitor.afterDelta(delta); } +export function applyDelta( + delta: Delta.Root, + deltaProcessor: { acquireVisitor: () => DeltaVisitor }, +): void { + const visitor = deltaProcessor.acquireVisitor(); + visitDelta(delta, visitor); + visitor.free(); +} +/** + * Visitor for changes in a delta. + * Must be freed after use. + * @alpha + */ export interface DeltaVisitor { - /** - * Called before the delta is visited. - */ - beforeDelta(delta: Delta.Root): void; - /** - * Called after the delta is visited. - */ - afterDelta(delta: Delta.Root): void; - /** - * Forks the current visitor. - * Any fork produced this way is freed before the visit terminates. - */ - fork(): DeltaVisitor; free(): void; onDelete(index: number, count: number): void; onInsert(index: number, content: Delta.ProtoNodes): void; onMoveOut(index: number, count: number, id: Delta.MoveId): void; onMoveIn(index: number, count: number, id: Delta.MoveId): void; - // TODO: better align this with ITreeCursor: - // maybe rename its up and down to enter / exit? Maybe Also)? - // Maybe also have cursor have "current field key" state to allow better handling of empty fields and better match - // this visitor? enterNode(index: number): void; exitNode(index: number): void; enterField(key: FieldKey): void; diff --git a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts index 05778ca23f29..bf25e86cccbf 100644 --- a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts @@ -19,7 +19,6 @@ import { Delta, UpPath, Anchor, - visitDelta, FieldAnchor, ForestEvents, ITreeSubscriptionCursorState, @@ -50,6 +49,8 @@ interface StackNode { class ChunkedForest extends SimpleDependee implements IEditableForest { private readonly dependent = new SimpleObservingDependent(() => this.invalidateDependents()); + private activeVisitor?: DeltaVisitor; + private readonly events = createEmitter(); /** @@ -86,13 +87,22 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { this.anchors.forget(anchor); } - public getVisitor(): DeltaVisitor { + public acquireVisitor(): DeltaVisitor { + assert( + this.activeVisitor === undefined, + "Must release existing visitor before acquiring another", + ); + this.events.emit("beforeChange"); + this.invalidateDependents(); + const moves: Map = new Map(); - // eslint-disable-next-line @typescript-eslint/no-this-alias - const thisForest = this; + if (this.roots.isShared()) { + this.roots = this.roots.clone(); + } const visitor = { + forest: this, // Current location in the tree, as a non-shared BasicChunk (TODO: support in-place modification of other chunk formats when possible). // Start above root detached sequences. mutableChunkStack: [] as StackNode[], @@ -106,8 +116,8 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { }, moveIn(index: number, toAttach: DetachedField): number { const detachedKey = detachedFieldAsKey(toAttach); - const children = thisForest.roots.fields.get(detachedKey) ?? []; - thisForest.roots.fields.delete(detachedKey); + const children = this.forest.roots.fields.get(detachedKey) ?? []; + this.forest.roots.fields.delete(detachedKey); if (children.length === 0) { return 0; // Prevent creating 0 sized fields when inserting empty into empty. } @@ -119,31 +129,23 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { return children.length; }, - beforeDelta(delta: Delta.Root): void { - thisForest.events.emit("beforeDelta", delta); - thisForest.invalidateDependents(); - - if (thisForest.roots.isShared()) { - thisForest.roots = thisForest.roots.clone(); - } - }, - afterDelta(delta: Delta.Root): void { - thisForest.events.emit("afterDelta", delta); - }, - fork() { - return { ...this, mutableChunkStack: [...this.mutableChunkStack] }; - }, free(): void { this.mutableChunk = undefined; this.mutableChunkStack.length = 0; + assert( + this.forest.activeVisitor !== undefined, + "Multiple free calls for same visitor", + ); + this.forest.activeVisitor = undefined; + this.forest.events.emit("afterChange"); }, onDelete(index: number, count: number): void { this.onMoveOut(index, count); }, onInsert(index: number, content: Delta.ProtoNodes): void { - const chunks: TreeChunk[] = content.map((c) => chunkTree(c, thisForest.chunker)); - const field = thisForest.newDetachedField(); - thisForest.roots.fields.set(detachedFieldAsKey(field), chunks); + const chunks: TreeChunk[] = content.map((c) => chunkTree(c, this.forest.chunker)); + const field = this.forest.newDetachedField(); + this.forest.roots.fields.set(detachedFieldAsKey(field), chunks); this.moveIn(index, field); }, onMoveOut(index: number, count: number, id?: Delta.MoveId): void { @@ -152,10 +154,10 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { const newField = sourceField.splice(index, count); if (id !== undefined) { - const detached = thisForest.newDetachedField(); + const detached = this.forest.newDetachedField(); const key = detachedFieldAsKey(detached); if (newField.length > 0) { - thisForest.roots.fields.set(key, newField); + this.forest.roots.fields.set(key, newField); } moves.set(id, detached); } else { @@ -195,7 +197,7 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { // // Maybe build path when visitor navigates then lazily sync to chunk tree when editing? const newChunks = mapCursorField(found.cursor(), (cursor) => - basicChunkTree(cursor, thisForest.chunker), + basicChunkTree(cursor, this.forest.chunker), ); // TODO: this could fail for really long chunks being split (due to argument count limits). // Current implementations of chunks shouldn't ever be that long, but it could be an issue if they get bigger. @@ -227,6 +229,7 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { this.mutableChunk = top.mutableChunk; }, }; + this.activeVisitor = visitor; return visitor; } diff --git a/experimental/dds/tree2/src/feature-libraries/editable-tree/editableTreeContext.ts b/experimental/dds/tree2/src/feature-libraries/editable-tree/editableTreeContext.ts index 48931506cecf..af8521e0c184 100644 --- a/experimental/dds/tree2/src/feature-libraries/editable-tree/editableTreeContext.ts +++ b/experimental/dds/tree2/src/feature-libraries/editable-tree/editableTreeContext.ts @@ -130,7 +130,7 @@ export class ProxyContext implements EditableTreeContext { public readonly nodeKeyFieldKey?: FieldKey, ) { this.eventUnregister = [ - this.forest.on("beforeDelta", () => { + this.forest.on("beforeChange", () => { this.prepareForEdit(); }), ]; diff --git a/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts b/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts index aebb633490c5..a646835462ca 100644 --- a/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts +++ b/experimental/dds/tree2/src/feature-libraries/forestRepairDataStore.ts @@ -6,6 +6,7 @@ import { assert, unreachableCase } from "@fluidframework/common-utils"; import { AnchorSet, + applyDelta, castCursorToSynchronous, Delta, EmptyKey, @@ -22,7 +23,6 @@ import { RevisionTag, SparseNode, UpPath, - visitDelta, } from "../core"; import { chunkTree, TreeChunk, defaultChunkPolicy } from "./chunked-forest"; @@ -201,8 +201,7 @@ export class ForestRepairDataStoreProvider implements IRepairDataStoreP public applyChange(change: TChange): void { if (this.frozenForest === undefined) { - const visitor = this.forest.getVisitor(); - visitDelta(this.intoDelta(change), visitor); + applyDelta(this.intoDelta(change), this.forest); } } diff --git a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts index 527cbb4a16e2..bb7b2d3199d8 100644 --- a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts @@ -20,7 +20,6 @@ import { Delta, UpPath, Anchor, - visitDelta, ITreeCursor, CursorLocationType, TreeSchemaIdentifier, @@ -54,6 +53,8 @@ function makeRoot(): MapTree { class ObjectForest extends SimpleDependee implements IEditableForest { private readonly dependent = new SimpleObservingDependent(() => this.invalidateDependents()); + private activeVisitor?: DeltaVisitor; + public readonly roots: MapTree = makeRoot(); // All cursors that are in the "Current" state. Must be empty when editing. @@ -96,7 +97,19 @@ class ObjectForest extends SimpleDependee implements IEditableForest { this.anchors.forget(anchor); } - public getVisitor(): DeltaVisitor { + public acquireVisitor(): DeltaVisitor { + assert( + this.activeVisitor === undefined, + "Must release existing visitor before acquiring another", + ); + this.events.emit("beforeChange"); + + this.invalidateDependents(); + assert( + this.currentCursors.size === 0, + 0x374 /* No cursors can be current when modifying forest */, + ); + // Note: This code uses cursors, however it also modifies the tree. // In general this is not safe, but this code happens to only modify the tree below the current cursor location, // which happens to work. @@ -122,35 +135,23 @@ class ObjectForest extends SimpleDependee implements IEditableForest { return children.length; }; - // eslint-disable-next-line @typescript-eslint/no-this-alias - const thisForest = this; - const cursors: ITreeSubscriptionCursor[] = [cursor]; const visitor = { + forest: this, cursor, - beforeDelta(delta: Delta.Root) { - thisForest.events.emit("beforeDelta", delta); - thisForest.invalidateDependents(); - assert( - thisForest.currentCursors.size === 0, - 0x374 /* No cursors can be current when modifying forest */, - ); - }, - afterDelta(delta: Delta.Root) { - thisForest.events.emit("afterDelta", delta); - }, - fork() { - const newCursor = this.cursor.fork(); - cursors.push(newCursor); - return { ...visitor, cursor: newCursor }; - }, free() { this.cursor.free(); + assert( + this.forest.activeVisitor !== undefined, + "Multiple free calls for same visitor", + ); + this.forest.activeVisitor = undefined; + this.forest.events.emit("afterChange"); }, onDelete(index: number, count: number): void { this.onMoveOut(index, count); }, onInsert(index: number, content: Delta.ProtoNode[]): void { - const range = thisForest.add(content); + const range = this.forest.add(content); moveIn(index, range, this.cursor); }, onMoveOut(index: number, count: number, id?: Delta.MoveId): void { @@ -165,11 +166,11 @@ class ObjectForest extends SimpleDependee implements IEditableForest { 0x371 /* detached range's end must be after its start */, ); const newField = sourceField.splice(startIndex, endIndex - startIndex); - const field = thisForest.addFieldAsDetached(newField); + const field = this.forest.addFieldAsDetached(newField); if (id !== undefined) { moves.set(id, field); } else { - thisForest.delete(field); + this.forest.delete(field); } if (sourceField.length === 0) { parent.fields.delete(key); @@ -194,6 +195,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { this.cursor.exitField(); }, }; + this.activeVisitor = visitor; return visitor; } diff --git a/experimental/dds/tree2/src/index.ts b/experimental/dds/tree2/src/index.ts index 46d5ef798749..ea80962beabb 100644 --- a/experimental/dds/tree2/src/index.ts +++ b/experimental/dds/tree2/src/index.ts @@ -24,6 +24,7 @@ export { RootField, ChildCollection, ChildLocation, + DeltaVisitor, FieldMapObject, NodeData, GenericTreeNode, diff --git a/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts b/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts index 7ba8eeb2fdc5..58fb470adc6c 100644 --- a/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts +++ b/experimental/dds/tree2/src/shared-tree/sharedTreeView.ts @@ -17,7 +17,7 @@ import { UndoRedoManager, LocalCommitSource, schemaDataIsEmpty, - visitDelta, + applyDelta, } from "../core"; import { HasListeners, IEmitter, ISubscribable, createEmitter } from "../events"; import { @@ -433,10 +433,8 @@ export class SharedTreeView implements ISharedTreeBranchView { branch.on("change", ({ change }) => { if (change !== undefined) { const delta = this.changeFamily.intoDelta(change); - const anchorVisitor = this.forest.anchors.getVisitor(); - const visitor = this.forest.getVisitor(); - visitDelta(delta, anchorVisitor); - visitDelta(delta, visitor); + applyDelta(delta, this.forest.anchors); + applyDelta(delta, this.forest); this.nodeKeyIndex.scanKeys(this.context); this.events.emit("afterBatch"); } diff --git a/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts index b3b14553041b..186075ff4525 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/chunked-forest/chunkedForest.spec.ts @@ -34,7 +34,7 @@ import { Delta, IForestSubscription, StoredSchemaRepository, - visitDelta, + applyDelta, } from "../../../core"; import { jsonObject } from "../../../domains"; import { @@ -144,7 +144,7 @@ describe("ChunkedForest", () => { // Captured reference owns a ref count making it shared. assert(chunk.isShared()); // Delete from forest, removing the forest's ref, making chunk not shared again. - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); assert(!chunk.isShared()); compareForest(forest, []); diff --git a/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts index e121844e59b7..515d7b15c4fc 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/default-field-kinds/defaultChangeFamily.spec.ts @@ -18,7 +18,7 @@ import { rootFieldKey, TaggedChange, UpPath, - visitDelta, + applyDelta, } from "../../../core"; import { jsonNumber, jsonObject, jsonString } from "../../../domains"; import { @@ -122,7 +122,7 @@ function initializeEditableForest(data?: JsonableTree): { changes.push({ revision: currentRevision, change }); const delta = defaultChangeFamily.intoDelta(change); deltas.push(delta); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); currentRevision = mintRevisionTag(); }); return { diff --git a/experimental/dds/tree2/src/test/feature-libraries/editable-tree/editableTreeContext.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/editable-tree/editableTreeContext.spec.ts index 04254ede121d..8a3a2912dda9 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/editable-tree/editableTreeContext.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/editable-tree/editableTreeContext.spec.ts @@ -40,7 +40,7 @@ describe("editable-tree context", () => { allowedSchemaModifications: AllowedUpdateType.None, }); - view.context.on("afterDelta", () => { + view.context.on("afterChange", () => { view.context.clear(); }); diff --git a/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts index c61d7012d781..9c31f840c6bf 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/forestRepairDataStore.spec.ts @@ -13,7 +13,7 @@ import { RevisionTag, rootFieldKey, UpPath, - visitDelta, + applyDelta, } from "../../core"; import { jsonNumber, jsonObject } from "../../domains"; import { @@ -84,7 +84,7 @@ describe("ForestRepairDataStore", () => { ], ]); store.capture(delta1, revision1); - visitDelta(delta1, forest.getVisitor()); + applyDelta(delta1, forest); const delta2 = new Map([ [ rootFieldKey, diff --git a/experimental/dds/tree2/src/test/forestTestSuite.ts b/experimental/dds/tree2/src/test/forestTestSuite.ts index 677d72bc386e..3ec960fc9ce3 100644 --- a/experimental/dds/tree2/src/test/forestTestSuite.ts +++ b/experimental/dds/tree2/src/test/forestTestSuite.ts @@ -24,7 +24,7 @@ import { EmptyKey, ValueSchema, FieldUpPath, - visitDelta, + applyDelta, } from "../core"; import { cursorToJsonObject, @@ -174,7 +174,7 @@ export function testForest(config: ForestTestConfiguration): void { type: Delta.MarkType.Insert, content: [singleJsonCursor([])], }; - visitDelta(new Map([[brand("different root"), [insert]]]), forest.getVisitor()); + applyDelta(new Map([[brand("different root"), [insert]]]), forest); assert(!forest.isEmpty); }); @@ -344,8 +344,8 @@ export function testForest(config: ForestTestConfiguration): void { const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - visitDelta(delta, forest.getVisitor()); - visitDelta(delta, forest.anchors.getVisitor()); + applyDelta(delta, forest); + applyDelta(delta, forest.anchors); assert.equal( forest.tryMoveCursorToNode(firstNodeAnchor, cursor), @@ -430,7 +430,7 @@ export function testForest(config: ForestTestConfiguration): void { const clone = forest.clone(forest.schema, forest.anchors); const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - visitDelta(delta, clone.getVisitor()); + applyDelta(delta, clone); // Check the clone has the new value const cloneReader = clone.allocateCursor(); @@ -457,7 +457,7 @@ export function testForest(config: ForestTestConfiguration): void { const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - assert.throws(() => visitDelta(delta, forest.getVisitor())); + assert.throws(() => applyDelta(delta, forest)); }); } @@ -488,7 +488,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [setField]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -507,7 +507,7 @@ export function testForest(config: ForestTestConfiguration): void { const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [0, mark]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); // Inspect resulting tree: should just have `2`. const reader = forest.allocateCursor(); @@ -532,7 +532,7 @@ export function testForest(config: ForestTestConfiguration): void { const skip: Delta.Skip = 1; const mark: Delta.Delete = { type: Delta.MarkType.Delete, count: 1 }; const delta: Delta.Root = new Map([[rootFieldKey, [skip, mark]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); // Inspect resulting tree: should just have `1`. const reader = forest.allocateCursor(); @@ -553,7 +553,7 @@ export function testForest(config: ForestTestConfiguration): void { content: [singleJsonCursor(3)], }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -590,7 +590,7 @@ export function testForest(config: ForestTestConfiguration): void { fields: new Map([[xField, [moveOut]]]), }; const delta: Delta.Root = new Map([[rootFieldKey, [mark, moveIn]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -624,7 +624,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [modify]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); assert(reader.firstNode()); @@ -660,7 +660,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -695,7 +695,7 @@ export function testForest(config: ForestTestConfiguration): void { const delta: Delta.Root = new Map([ [rootFieldKey, [mark, { type: Delta.MarkType.MoveIn, count: 1, moveId }]], ]); - forest.applyDelta(delta); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -741,7 +741,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [mark]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const reader = forest.allocateCursor(); moveToDetachedField(forest, reader); @@ -775,15 +775,15 @@ export function testForest(config: ForestTestConfiguration): void { const delta: Delta.Root = new Map([[rootFieldKey, [insert]]]); assert.deepEqual(dependent.tokens, []); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); assert.deepEqual(dependent.tokens.length, 1); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); assert.deepEqual(dependent.tokens.length, 2); // Remove the dependency so the dependent stops getting invalidation messages forest.removeDependent(dependent); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); assert.deepEqual(dependent.tokens.length, 2); }); @@ -827,7 +827,7 @@ export function testForest(config: ForestTestConfiguration): void { ]); const expected: JsonCompatible[] = [{ y: 1 }]; initializeForest(forest, [singleJsonCursor(nestedContent)]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const readCursor = forest.allocateCursor(); moveToDetachedField(forest, readCursor); const actual = mapCursorField(readCursor, cursorToJsonObject); @@ -871,7 +871,7 @@ export function testForest(config: ForestTestConfiguration): void { ]), }; const delta: Delta.Root = new Map([[rootFieldKey, [modify]]]); - visitDelta(delta, forest.getVisitor()); + applyDelta(delta, forest); const expectedCursor = cursorForTypedTreeData({ schema }, root, { x: [], y: [1, 2], diff --git a/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts b/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts index 0eb42e7b60fe..39725d338b67 100644 --- a/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts +++ b/experimental/dds/tree2/src/test/tree/anchorSet.spec.ts @@ -16,7 +16,7 @@ import { UpPath, clonePath, rootFieldKey, - visitDelta, + applyDelta, } from "../../core"; import { brand } from "../../util"; import { expectEqualPaths } from "../utils"; @@ -62,7 +62,7 @@ describe("AnchorSet", () => { }; const delta = new Map([[rootFieldKey, [1, moveOut, 1, moveIn]]]); - visitDelta(delta, anchors.getVisitor()); + applyDelta(delta, anchors); checkEquality(anchors.locate(anchor0), makePath([rootFieldKey, 0])); checkEquality(anchors.locate(anchor1), makePath([rootFieldKey, 2])); checkEquality(anchors.locate(anchor2), makePath([rootFieldKey, 1])); @@ -77,7 +77,7 @@ describe("AnchorSet", () => { content: [node, node].map(singleTextCursor), }; - visitDelta(makeDelta(insert, makePath([fieldFoo, 4])), anchors.getVisitor()); + applyDelta(makeDelta(insert, makePath([fieldFoo, 4])), anchors); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 7], [fieldBar, 4])); checkEquality(anchors.locate(anchor2), makePath([fieldFoo, 3], [fieldBaz, 2])); @@ -91,7 +91,7 @@ describe("AnchorSet", () => { count: 1, }; - visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 4])), anchors.getVisitor()); + applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 4])), anchors); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 4], [fieldBar, 4])); checkEquality(anchors.locate(anchor2), path2); assert.equal(anchors.locate(anchor3), undefined); @@ -106,7 +106,7 @@ describe("AnchorSet", () => { count: 1, }; - visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 5])), anchors.getVisitor()); + applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 5])), anchors); assert.equal(anchors.locate(anchor4), undefined); assert.equal(anchors.locate(anchor1), undefined); assert.doesNotThrow(() => anchors.forget(anchor4)); @@ -117,14 +117,14 @@ describe("AnchorSet", () => { assert.throws(() => anchors.locate(anchor1)); checkEquality(anchors.locate(anchor2), path2); - visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 3])), anchors.getVisitor()); + applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 3])), anchors); checkEquality(anchors.locate(anchor2), undefined); assert.doesNotThrow(() => anchors.forget(anchor2)); assert.throws(() => anchors.locate(anchor2)); // The index of anchor3 has changed from 4 to 3 because of the deletion of the node at index 3. checkEquality(anchors.locate(anchor3), makePath([fieldFoo, 3])); - visitDelta(makeDelta(deleteMark, makePath([fieldFoo, 3])), anchors.getVisitor()); + applyDelta(makeDelta(deleteMark, makePath([fieldFoo, 3])), anchors); checkEquality(anchors.locate(anchor3), undefined); assert.doesNotThrow(() => anchors.forget(anchor3)); assert.throws(() => anchors.locate(anchor3)); @@ -150,7 +150,7 @@ describe("AnchorSet", () => { }; const delta = new Map([[fieldFoo, [3, moveOut, 1, modify]]]); - visitDelta(delta, anchors.getVisitor()); + applyDelta(delta, anchors); checkEquality(anchors.locate(anchor1), makePath([fieldFoo, 4], [fieldBar, 5])); checkEquality( anchors.locate(anchor2), @@ -233,7 +233,7 @@ describe("AnchorSet", () => { }; log.expect([]); - visitDelta(new Map([[rootFieldKey, [0, deleteMark]]]), anchors.getVisitor()); + applyDelta(new Map([[rootFieldKey, [0, deleteMark]]]), anchors); log.expect([ ["root childrenChange", 1], @@ -254,7 +254,7 @@ describe("AnchorSet", () => { type: Delta.MarkType.Insert, content: [singleTextCursor({ type: jsonString.name, value: "x" })], }; - visitDelta(new Map([[rootFieldKey, [deleteMark, insertMark]]]), anchors.getVisitor()); + applyDelta(new Map([[rootFieldKey, [deleteMark, insertMark]]]), anchors); log.expect([ ["afterDelete", 1], @@ -263,12 +263,12 @@ describe("AnchorSet", () => { ]); log.clear(); - visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 5])), anchors.getVisitor()); + applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 5])), anchors); log.expect([["root treeChange", 1]]); log.clear(); - visitDelta(new Map([[rootFieldKey, [0, deleteMark]]]), anchors.getVisitor()); + applyDelta(new Map([[rootFieldKey, [0, deleteMark]]]), anchors); log.expect([ ["root childrenChange", 1], ["root treeChange", 1], @@ -286,7 +286,7 @@ describe("AnchorSet", () => { }; const log = new UnorderedTestLogger(); const anchors = new AnchorSet(); - visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 3])), anchors.getVisitor()); + applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 3])), anchors); const anchor0 = anchors.track(makePath([rootFieldKey, 0])); const node0 = anchors.locate(anchor0) ?? assert.fail(); const pathVisitor: PathVisitor = { @@ -304,14 +304,14 @@ describe("AnchorSet", () => { }, }; const unsubscribePathVisitor = node0.on("subtreeChanging", (n: AnchorNode) => pathVisitor); - visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4])), anchors.getVisitor()); + applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4])), anchors); log.expect([["visitSubtreeChange.onInsert-foo-4", 1]]); log.clear(); - visitDelta(makeDelta(deleteMark, makePath([rootFieldKey, 0], [fieldFoo, 5])), anchors.getVisitor()); + applyDelta(makeDelta(deleteMark, makePath([rootFieldKey, 0], [fieldFoo, 5])), anchors); log.expect([["visitSubtreeChange.onDelete-foo-5-1", 1]]); log.clear(); unsubscribePathVisitor(); - visitDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4])), anchors.getVisitor()); + applyDelta(makeDelta(insertMark, makePath([rootFieldKey, 0], [fieldFoo, 4])), anchors); log.expect([]); }); }); From 429cd2636fba6b0cca44cb971d4b9300d9e6f644 Mon Sep 17 00:00:00 2001 From: Jenn Date: Fri, 1 Sep 2023 16:29:50 -0700 Subject: [PATCH 4/6] Update experimental/dds/tree2/src/core/forest/editableForest.ts Co-authored-by: Craig Macomber (Microsoft) <42876482+CraigMacomber@users.noreply.github.com> --- experimental/dds/tree2/src/core/forest/editableForest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/dds/tree2/src/core/forest/editableForest.ts b/experimental/dds/tree2/src/core/forest/editableForest.ts index ce5587e81a32..2678d54dcd69 100644 --- a/experimental/dds/tree2/src/core/forest/editableForest.ts +++ b/experimental/dds/tree2/src/core/forest/editableForest.ts @@ -34,6 +34,7 @@ export interface IEditableForest extends IForestSubscription { /** * @returns a visitor that can be used to mutate the forest. + * * Mutating the forest does NOT update anchors. * The visitor must be released after use. * It is invalid to acquire a visitor without releasing the previous one. From 338bfcfc0a6fb8c1432359a52e592bc5e817a249 Mon Sep 17 00:00:00 2001 From: Yann Achard Date: Fri, 1 Sep 2023 16:56:25 -0700 Subject: [PATCH 5/6] Invalidate dependents and clarify event policy --- experimental/dds/tree2/src/core/forest/forest.ts | 2 ++ .../src/feature-libraries/chunked-forest/chunkedForest.ts | 4 +++- .../tree2/src/feature-libraries/object-forest/objectForest.ts | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/experimental/dds/tree2/src/core/forest/forest.ts b/experimental/dds/tree2/src/core/forest/forest.ts index 65c1326b84ce..2b51b8b9805b 100644 --- a/experimental/dds/tree2/src/core/forest/forest.ts +++ b/experimental/dds/tree2/src/core/forest/forest.ts @@ -35,11 +35,13 @@ import type { IEditableForest } from "./editableForest"; export interface ForestEvents { /** * The forest is about to be changed. + * Emitted before the first change in a batch of changes. */ beforeChange(): void; /** * The forest was just changed. + * Emitted after the last change in a batch of changes. */ afterChange(): void; } diff --git a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts index 6c6474fc4896..afe5ac78d19c 100644 --- a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts @@ -90,7 +90,6 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { "Must release existing visitor before acquiring another", ); this.events.emit("beforeChange"); - this.invalidateDependents(); const moves: Map = new Map(); @@ -112,6 +111,7 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { return this.mutableChunkStack[this.mutableChunkStack.length - 1]; }, moveIn(index: number, toAttach: DetachedField): number { + this.forest.invalidateDependents(); const detachedKey = detachedFieldAsKey(toAttach); const children = this.forest.roots.fields.get(detachedKey) ?? []; this.forest.roots.fields.delete(detachedKey); @@ -140,12 +140,14 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { this.onMoveOut(index, count); }, onInsert(index: number, content: Delta.ProtoNodes): void { + this.forest.invalidateDependents(); const chunks: TreeChunk[] = content.map((c) => chunkTree(c, this.forest.chunker)); const field = this.forest.newDetachedField(); this.forest.roots.fields.set(detachedFieldAsKey(field), chunks); this.moveIn(index, field); }, onMoveOut(index: number, count: number, id?: Delta.MoveId): void { + this.forest.invalidateDependents(); const parent = this.getParent(); const sourceField = parent.mutableChunk.fields.get(parent.key) ?? []; const newField = sourceField.splice(index, count); diff --git a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts index f03e60465dc5..5fc0b1b338e6 100644 --- a/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/object-forest/objectForest.ts @@ -98,7 +98,6 @@ class ObjectForest extends SimpleDependee implements IEditableForest { ); this.events.emit("beforeChange"); - this.invalidateDependents(); assert( this.currentCursors.size === 0, 0x374 /* No cursors can be current when modifying forest */, @@ -114,6 +113,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { const cursor: Cursor = this.allocateCursor(); cursor.setToAboveDetachedSequences(); const moveIn = (index: number, toAttach: DetachedField, moveInCursor: Cursor): number => { + this.invalidateDependents(); const detachedKey = detachedFieldAsKey(toAttach); const children = getMapTreeField(this.roots, detachedKey, false); this.roots.fields.delete(detachedKey); @@ -149,6 +149,7 @@ class ObjectForest extends SimpleDependee implements IEditableForest { moveIn(index, range, this.cursor); }, onMoveOut(index: number, count: number, id?: Delta.MoveId): void { + this.forest.invalidateDependents(); const [parent, key] = cursor.getParent(); const sourceField = getMapTreeField(parent, key, false); const startIndex = index; From e41152b169a941238bfc257a4ec6324a8fb088f9 Mon Sep 17 00:00:00 2001 From: Yann Achard Date: Tue, 5 Sep 2023 16:47:55 -0700 Subject: [PATCH 6/6] Less inval of dependents --- .../chunked-forest/chunkedForest.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts index ec0da508706c..77b0133731f5 100644 --- a/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts +++ b/experimental/dds/tree2/src/feature-libraries/chunked-forest/chunkedForest.ts @@ -110,8 +110,14 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { ); return this.mutableChunkStack[this.mutableChunkStack.length - 1]; }, - moveIn(index: number, toAttach: DetachedField): number { - this.forest.invalidateDependents(); + moveIn( + index: number, + toAttach: DetachedField, + invalidateDependents: boolean = true, + ): number { + if (invalidateDependents) { + this.forest.invalidateDependents(); + } const detachedKey = detachedFieldAsKey(toAttach); const children = this.forest.roots.fields.get(detachedKey) ?? []; this.forest.roots.fields.delete(detachedKey); @@ -144,7 +150,7 @@ class ChunkedForest extends SimpleDependee implements IEditableForest { const chunks: TreeChunk[] = content.map((c) => chunkTree(c, this.forest.chunker)); const field = this.forest.newDetachedField(); this.forest.roots.fields.set(detachedFieldAsKey(field), chunks); - this.moveIn(index, field); + this.moveIn(index, field, false); }, onMoveOut(index: number, count: number, id?: Delta.MoveId): void { this.forest.invalidateDependents();