Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(tree): move global and rename effects to the delta root #23484

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
47 changes: 21 additions & 26 deletions packages/dds/tree/src/core/tree/delta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,22 @@ export interface Root<TTree = ProtoNode> {
* The ordering has no significance.
*/
readonly refreshers?: readonly DetachedNodeBuild<TTree>[];
/**
* Changes to apply to detached nodes.
* The ordering has no significance.
*
* Nested changes for a root that is undergoing a rename should be listed under the starting name.
* For example, if one wishes to change a tree which is being renamed from ID A to ID B,
* then the changes should be listed under ID A.
*/
readonly global?: readonly DetachedNodeChanges[];
/**
* Detached whose associated ID needs to be updated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Detached whose associated ID needs to be updated.
* Detached roots whose associated ID needs to be updated.

* The ordering has no significance.
* Note that the renames may need to be performed in a specific order to avoid collisions.
* This ordering problem is left to the consumer of this format.
*/
readonly rename?: readonly DetachedNodeRename[];
}

/**
Expand Down Expand Up @@ -199,30 +215,9 @@ export interface DetachedNodeRename {
}

/**
* Represents the changes to perform on a given field.
* Represents a list of changes to the nodes in the field.
* The index of each mark within the range of nodes, before
* applying any of the changes, is not represented explicitly.
* It corresponds to the sum of `mark.count` values for all previous marks for which `isAttachMark(mark)` is false.
*/
export interface FieldChanges {
/**
* Represents a list of changes to the nodes in the field.
* The index of each mark within the range of nodes, before
* applying any of the changes, is not represented explicitly.
* It corresponds to the sum of `mark.count` values for all previous marks for which `isAttachMark(mark)` is false.
*/
readonly local?: readonly Mark[];
/**
* Changes to apply to detached nodes.
* The ordering has no significance.
*
* Nested changes for a root that is undergoing a rename should be listed under the starting name.
* For example, if one wishes to change a tree which is being renamed from ID A to ID B,
* then the changes should be listed under ID A.
*/
readonly global?: readonly DetachedNodeChanges[];
/**
* Detached whose associated ID needs to be updated.
* The ordering has no significance.
* Note that the renames may need to be performed in a specific order to avoid collisions.
* This ordering problem is left to the consumer of this format.
*/
readonly rename?: readonly DetachedNodeRename[];
}
export type FieldChanges = readonly Mark[];
15 changes: 3 additions & 12 deletions packages/dds/tree/src/core/tree/deltaUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { rootFieldKey } from "./types.js";

export const emptyDelta: Root<never> = {};

export const emptyFieldChanges: FieldChanges = {};
export const emptyFieldChanges: FieldChanges = [];

export function isAttachMark(mark: Mark): boolean {
return mark.attach !== undefined && mark.detach === undefined;
Expand All @@ -27,11 +27,7 @@ export function isReplaceMark(mark: Mark): boolean {
}

export function isEmptyFieldChanges(fieldChanges: FieldChanges): boolean {
return (
fieldChanges.local === undefined &&
fieldChanges.global === undefined &&
fieldChanges.rename === undefined
);
return fieldChanges.length === 0;
}

export function deltaForRootInitialization(content: readonly ITreeCursorSynchronous[]): Root {
Expand All @@ -42,12 +38,7 @@ export function deltaForRootInitialization(content: readonly ITreeCursorSynchron
const delta: Root = {
build: [{ id: buildId, trees: content }],
fields: new Map<FieldKey, FieldChanges>([
[
rootFieldKey,
{
local: [{ count: content.length, attach: buildId }],
},
],
[rootFieldKey, [{ count: content.length, attach: buildId }]],
]),
};
return delta;
Expand Down
166 changes: 82 additions & 84 deletions packages/dds/tree/src/core/tree/visitDelta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,14 @@ function visitNode(
* (because we want to wait until we are sure content to attach is available as a root)
*/
function detachPass(
delta: Delta.FieldChanges,
fieldChanges: Delta.FieldChanges,
visitor: DeltaVisitor,
config: PassConfig,
global?: readonly Delta.DetachedNodeChanges[],
rename?: readonly Delta.DetachedNodeRename[],
): void {
if (delta.global !== undefined) {
for (const { id, fields } of delta.global) {
if (global !== undefined) {
for (const { id, fields } of global) {
let root = config.detachedFieldIndex.tryGetEntry(id);
if (root === undefined) {
const tree = tryGetFromNestedMap(config.refreshers, id.major, id.minor);
Expand All @@ -439,33 +441,31 @@ function detachPass(
config.attachPassRoots.set(root, fields);
}
}
if (delta.rename !== undefined) {
config.rootTransfers.push(...delta.rename);
if (rename !== undefined) {
config.rootTransfers.push(...rename);
}
if (delta.local !== undefined) {
let index = 0;
for (const mark of delta.local) {
if (mark.fields !== undefined) {
assert(
mark.attach === undefined || mark.detach !== undefined,
0x7d0 /* Invalid nested changes on an additive mark */,
);
visitNode(index, mark.fields, visitor, config);
}
if (isDetachMark(mark)) {
for (let i = 0; i < mark.count; i += 1) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const id = offsetDetachId(mark.detach!, i);
const root = config.detachedFieldIndex.createEntry(id, config.latestRevision);
if (mark.fields !== undefined) {
config.attachPassRoots.set(root, mark.fields);
}
const field = config.detachedFieldIndex.toFieldKey(root);
visitor.detach({ start: index, end: index + 1 }, field);
let index = 0;
for (const mark of fieldChanges) {
if (mark.fields !== undefined) {
assert(
mark.attach === undefined || mark.detach !== undefined,
0x7d0 /* Invalid nested changes on an additive mark */,
);
visitNode(index, mark.fields, visitor, config);
}
if (isDetachMark(mark)) {
for (let i = 0; i < mark.count; i += 1) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const id = offsetDetachId(mark.detach!, i);
const root = config.detachedFieldIndex.createEntry(id, config.latestRevision);
if (mark.fields !== undefined) {
config.attachPassRoots.set(root, mark.fields);
}
} else if (!isAttachMark(mark)) {
index += mark.count;
const field = config.detachedFieldIndex.toFieldKey(root);
visitor.detach({ start: index, end: index + 1 }, field);
}
} else if (!isAttachMark(mark)) {
index += mark.count;
}
}
}
Expand Down Expand Up @@ -515,69 +515,67 @@ function collectDestroys(
* - Collects detached roots (from replaces) that need an attach pass
*/
function attachPass(
delta: Delta.FieldChanges,
fieldChanges: Delta.FieldChanges,
visitor: DeltaVisitor,
config: PassConfig,
): void {
if (delta.local !== undefined) {
let index = 0;
for (const mark of delta.local) {
if (isAttachMark(mark) || isReplaceMark(mark)) {
for (let i = 0; i < mark.count; i += 1) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const offsetAttachId = offsetDetachId(mark.attach!, i);
let sourceRoot = config.detachedFieldIndex.tryGetEntry(offsetAttachId);
if (sourceRoot === undefined) {
const tree = tryGetFromNestedMap(
config.refreshers,
offsetAttachId.major,
offsetAttachId.minor,
);
assert(tree !== undefined, 0x92a /* refresher data not found */);
buildTrees(
offsetAttachId,
[tree],
config.detachedFieldIndex,
config.latestRevision,
visitor,
);
sourceRoot = config.detachedFieldIndex.getEntry(offsetAttachId);
}
const sourceField = config.detachedFieldIndex.toFieldKey(sourceRoot);
const offsetIndex = index + i;
if (isReplaceMark(mark)) {
const rootDestination = config.detachedFieldIndex.createEntry(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
offsetDetachId(mark.detach!, i),
config.latestRevision,
);
const destinationField = config.detachedFieldIndex.toFieldKey(rootDestination);
visitor.replace(
sourceField,
{ start: offsetIndex, end: offsetIndex + 1 },
destinationField,
);
// We may need to do a second pass on the detached nodes
if (mark.fields !== undefined) {
config.attachPassRoots.set(rootDestination, mark.fields);
}
} else {
// This a simple attach
visitor.attach(sourceField, 1, offsetIndex);
}
config.detachedFieldIndex.deleteEntry(offsetAttachId);
const fields = config.attachPassRoots.get(sourceRoot);
if (fields !== undefined) {
config.attachPassRoots.delete(sourceRoot);
visitNode(offsetIndex, fields, visitor, config);
let index = 0;
for (const mark of fieldChanges) {
if (isAttachMark(mark) || isReplaceMark(mark)) {
for (let i = 0; i < mark.count; i += 1) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const offsetAttachId = offsetDetachId(mark.attach!, i);
let sourceRoot = config.detachedFieldIndex.tryGetEntry(offsetAttachId);
if (sourceRoot === undefined) {
const tree = tryGetFromNestedMap(
config.refreshers,
offsetAttachId.major,
offsetAttachId.minor,
);
assert(tree !== undefined, 0x92a /* refresher data not found */);
buildTrees(
offsetAttachId,
[tree],
config.detachedFieldIndex,
config.latestRevision,
visitor,
);
sourceRoot = config.detachedFieldIndex.getEntry(offsetAttachId);
}
const sourceField = config.detachedFieldIndex.toFieldKey(sourceRoot);
const offsetIndex = index + i;
if (isReplaceMark(mark)) {
const rootDestination = config.detachedFieldIndex.createEntry(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
offsetDetachId(mark.detach!, i),
config.latestRevision,
);
const destinationField = config.detachedFieldIndex.toFieldKey(rootDestination);
visitor.replace(
sourceField,
{ start: offsetIndex, end: offsetIndex + 1 },
destinationField,
);
// We may need to do a second pass on the detached nodes
if (mark.fields !== undefined) {
config.attachPassRoots.set(rootDestination, mark.fields);
}
} else {
// This a simple attach
visitor.attach(sourceField, 1, offsetIndex);
}
config.detachedFieldIndex.deleteEntry(offsetAttachId);
const fields = config.attachPassRoots.get(sourceRoot);
if (fields !== undefined) {
config.attachPassRoots.delete(sourceRoot);
visitNode(offsetIndex, fields, visitor, config);
}
} else if (!isDetachMark(mark) && mark.fields !== undefined) {
visitNode(index, mark.fields, visitor, config);
}
if (!isDetachMark(mark)) {
index += mark.count;
}
} else if (!isDetachMark(mark) && mark.fields !== undefined) {
visitNode(index, mark.fields, visitor, config);
}
if (!isDetachMark(mark)) {
index += mark.count;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const noChangeHandler: FieldChangeHandler<0> = {
}),
codecsFactory: () => noChangeCodecFamily,
editor: { buildChildChange: (index, change) => fail("Child changes not supported") },
intoDelta: (change, deltaFromChild: ToDelta): DeltaFieldChanges => ({}),
intoDelta: (change, deltaFromChild: ToDelta) => [[], [], []],
relevantRemovedRoots: (change): Iterable<DeltaDetachedNodeId> => [],
isEmpty: (change: 0) => true,
getNestedChanges: (change: 0) => [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,7 @@ export class ForestSummarizer implements Summarizable {
id: buildId,
trees: nodeCursors,
});
fieldChanges.push([
fieldKey,
{
local: [{ count: nodeCursors.length, attach: buildId }],
},
]);
fieldChanges.push([fieldKey, [{ count: nodeCursors.length, attach: buildId }]]);
}

assert(this.forest.isEmpty, 0x797 /* forest must be empty */);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
import type { ICodecFamily, IJsonCodec } from "../../codec/index.js";
import type {
ChangeEncodingContext,
DeltaDetachedNodeChanges,
DeltaDetachedNodeId,
DeltaDetachedNodeRename,
DeltaFieldChanges,
DeltaFieldMap,
EncodedRevisionTag,
Expand Down Expand Up @@ -49,7 +51,7 @@ export interface FieldChangeHandler<
change: TChangeset,
deltaFromChild: ToDelta,
idAllocator: MemoizedIdRangeAllocator,
): DeltaFieldChanges;
): [DeltaFieldChanges, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]];
/**
* Returns the set of removed roots that should be in memory for the given change to be applied.
* A removed root is relevant if any of the following is true:
Expand Down Expand Up @@ -195,7 +197,9 @@ export interface FieldEditor<TChangeset> {
* The `index` represents the index of the child node in the input context.
* The `index` should be `undefined` iff the child node does not exist in the input context (e.g., an inserted node).
*/
export type ToDelta = (child: NodeId) => DeltaFieldMap;
export type ToDelta = (
child: NodeId,
) => [DeltaFieldMap, DeltaDetachedNodeChanges[], DeltaDetachedNodeRename[]] | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this contract needs to change? (See the second paragraph in https://dev.azure.com/fluidframework/internal/_workitems/edit/13504)


/**
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import {
type DeltaDetachedNodeId,
type DeltaFieldChanges,
type DeltaMark,
type RevisionMetadataSource,
Multiplicity,
Expand Down Expand Up @@ -47,7 +46,7 @@ export const genericChangeHandler: FieldChangeHandler<GenericChangeset> = {
return newGenericChangeset([[index, change]]);
},
},
intoDelta: (change: GenericChangeset, deltaFromChild: ToDelta): DeltaFieldChanges => {
intoDelta: (change: GenericChangeset, deltaFromChild: ToDelta) => {
let nodeIndex = 0;
const markList: DeltaMark[] = [];
for (const [index, nodeChange] of change.entries()) {
Expand All @@ -56,10 +55,15 @@ export const genericChangeHandler: FieldChangeHandler<GenericChangeset> = {
markList.push({ count: offset });
nodeIndex = index;
}
markList.push({ count: 1, fields: deltaFromChild(nodeChange) });
const childDelta = deltaFromChild(nodeChange);
if (childDelta !== undefined) {
// TODO accumulate the child global and rename
const [fields, _childGlobal, _childRename] = childDelta;
markList.push({ count: 1, fields });
}
nodeIndex += 1;
}
return { local: markList };
return [markList, [], []];
},
relevantRemovedRoots,
isEmpty: (change: GenericChangeset): boolean => change.length === 0,
Expand Down
Loading
Loading