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

MergeTree: Remove SegmentGroup from internal #23401

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 2 additions & 18 deletions packages/dds/matrix/src/matrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,13 @@ import {
} from "@fluidframework/datastore-definitions/internal";
import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal";
import {
// eslint-disable-next-line import/no-deprecated
Client,
IJSONSegment,
IMergeTreeOp,
type ISegmentInternal,
type LocalReferencePosition,
MergeTreeDeltaType,
ReferenceType,
// eslint-disable-next-line import/no-deprecated
SegmentGroup,
segmentIsRemoved,
} from "@fluidframework/merge-tree/internal";
import { ISummaryTreeWithStats } from "@fluidframework/runtime-definitions/internal";
Expand Down Expand Up @@ -755,7 +752,6 @@ export class SharedMatrix<T = any>
}

private rebasePosition(
// eslint-disable-next-line import/no-deprecated
client: Client,
ref: LocalReferencePosition,
localSeq: number,
Expand Down Expand Up @@ -813,23 +809,11 @@ export class SharedMatrix<T = any>
} else {
switch (content.target) {
case SnapshotPath.cols: {
this.submitColMessage(
this.cols.regeneratePendingOp(
content,
// eslint-disable-next-line import/no-deprecated
localOpMetadata as SegmentGroup | SegmentGroup[],
),
);
this.submitColMessage(this.cols.regeneratePendingOp(content, localOpMetadata));
break;
}
case SnapshotPath.rows: {
this.submitRowMessage(
this.rows.regeneratePendingOp(
content,
// eslint-disable-next-line import/no-deprecated
localOpMetadata as SegmentGroup | SegmentGroup[],
),
);
this.submitRowMessage(this.rows.regeneratePendingOp(content, localOpMetadata));
break;
}
default: {
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/matrix/src/permutationvector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from "@fluidframework/datastore-definitions/internal";
import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal";
import {
BaseSegment, // eslint-disable-next-line import/no-deprecated
BaseSegment,
Client,
IJSONSegment,
IMergeTreeDeltaCallbackArgs,
Expand Down
16 changes: 4 additions & 12 deletions packages/dds/merge-tree/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,7 @@ export class Client extends TypedEventEmitter<IClientEvents> {
* @param count - The number segment groups to get peek from the tail of the queue. Default 1.
*/

public peekPendingSegmentGroups(): SegmentGroup | undefined;

public peekPendingSegmentGroups(count: number): SegmentGroup | SegmentGroup[] | undefined;
public peekPendingSegmentGroups(
count: number = 1,
): SegmentGroup | SegmentGroup[] | undefined {
public peekPendingSegmentGroups(count: number = 1): unknown {
const pending = this._mergeTree.pendingSegments;
let node = pending?.last;
if (count === 1 || pending === undefined) {
Expand Down Expand Up @@ -876,7 +871,7 @@ export class Client extends TypedEventEmitter<IClientEvents> {
private resetPendingDeltaToOps(
resetOp: IMergeTreeDeltaOp,

segmentGroup: SegmentGroup<ISegmentPrivate>,
segmentGroup: SegmentGroup,
): IMergeTreeDeltaOp[] {
assert(!!segmentGroup, 0x033 /* "Segment group undefined" */);
const NACKedSegmentGroup = this.pendingRebase?.shift()?.data;
Expand Down Expand Up @@ -1193,11 +1188,8 @@ export class Client extends TypedEventEmitter<IClientEvents> {
* @param resetOp - The op to reset
* @param segmentGroup - The segment group associated with the op
*/
public regeneratePendingOp(
resetOp: IMergeTreeOp,

segmentGroup: SegmentGroup | SegmentGroup[],
): IMergeTreeOp {
public regeneratePendingOp(resetOp: IMergeTreeOp, localOpMetadata: unknown): IMergeTreeOp {
const segmentGroup = localOpMetadata as SegmentGroup | SegmentGroup[];
if (this.pendingRebase === undefined || this.pendingRebase.empty) {
let firstGroup: SegmentGroup;
if (Array.isArray(segmentGroup)) {
Expand Down
3 changes: 0 additions & 3 deletions packages/dds/merge-tree/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ export {
export {
BaseSegment,
CollaborationWindow,
debugMarkerToString,
IJSONMarkerSegment,
IMergeNodeCommon,
segmentIsRemoved,
Expand All @@ -67,8 +66,6 @@ export {
Marker,
reservedMarkerIdKey,
reservedMarkerSimpleTypeKey,
SegmentGroup,
ObliterateInfo,
ISegmentInternal,
} from "./mergeTreeNodes.js";
export {
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/mergeTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2400,7 +2400,7 @@ export class MergeTree {
throw new Error("Rollback op doesn't match last edit");
}
let i = 0;
for (const segment of pendingSegmentGroup.segments as ISegmentPrivate[]) {
for (const segment of pendingSegmentGroup.segments) {
const segmentSegmentGroup = segment?.segmentGroups?.pop?.();
assert(
segmentSegmentGroup === pendingSegmentGroup,
Expand Down
116 changes: 7 additions & 109 deletions packages/dds/merge-tree/src/mergeTreeNodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,7 @@ import { IJSONSegment, IMarkerDef, ReferenceType } from "./ops.js";
import { computeHierarchicalOrdinal } from "./ordinal.js";
import type { PartialSequenceLengths } from "./partialLengths.js";
import { PropertySet, clone, createMap, type MapLike } from "./properties.js";
import {
ReferencePosition,
refGetTileLabels,
refTypeIncludesFlag,
} from "./referencePositions.js";
import { ReferencePosition } from "./referencePositions.js";
import { SegmentGroupCollection } from "./segmentGroupCollection.js";
import {
isInserted,
Expand Down Expand Up @@ -273,14 +269,6 @@ export function segmentIsRemoved(segment: ISegment): boolean {
return leaf.removedSeq !== undefined;
}

/**
* @internal
*/
export interface IMarkerModifiedAction {
// eslint-disable-next-line @typescript-eslint/prefer-function-type
(marker: Marker): void;
}

/**
* @legacy
* @alpha
Expand All @@ -297,16 +285,10 @@ export interface ISegmentAction<TClientData> {
accum: TClientData,
): boolean;
}
/**
* @internal
*/
export interface ISegmentChanges {
next?: ISegmentInternal;
replaceCurrent?: ISegmentInternal;
next?: ISegmentPrivate;
replaceCurrent?: ISegmentPrivate;
}
/**
* @internal
*/
export interface BlockAction<TClientData> {
// eslint-disable-next-line @typescript-eslint/prefer-function-type
(
Expand All @@ -320,49 +302,16 @@ export interface BlockAction<TClientData> {
): boolean;
}

/**
* @internal
*/
export interface NodeAction<TClientData> {
// eslint-disable-next-line @typescript-eslint/prefer-function-type
(
node: IMergeNode,
pos: number,
refSeq: number,
clientId: number,
start: number | undefined,
end: number | undefined,
clientData: TClientData,
): boolean;
}

/**
* @internal
*/
export interface InsertContext {
candidateSegment?: ISegmentInternal;
candidateSegment?: ISegmentPrivate;
leaf: (
segment: ISegmentInternal | undefined,
segment: ISegmentPrivate | undefined,
pos: number,
ic: InsertContext,
) => ISegmentChanges;
continuePredicate?: (continueFromBlock: MergeBlock) => boolean;
}

/**
* @internal
*/
export interface SegmentActions<TClientData> {
leaf?: ISegmentAction<TClientData>;
shift?: NodeAction<TClientData>;
contains?: NodeAction<TClientData>;
pre?: BlockAction<TClientData>;
post?: BlockAction<TClientData>;
}

/**
* @internal
*/
export interface ObliterateInfo {
start: LocalReferencePosition;
end: LocalReferencePosition;
Expand All @@ -373,11 +322,8 @@ export interface ObliterateInfo {
segmentGroup: SegmentGroup | undefined;
}

/**
* @internal
*/
export interface SegmentGroup<S extends ISegmentInternal = ISegmentInternal> {
segments: S[];
export interface SegmentGroup {
segments: ISegmentPrivate[];
previousProps?: PropertySet[];
localSeq?: number;
refSeq: number;
Expand All @@ -389,12 +335,8 @@ export interface SegmentGroup<S extends ISegmentInternal = ISegmentInternal> {
* the MergeTree always inserts first, then checks for overflow and splits if the child count equals
* `MaxNodesInBlock`. (i.e., `MaxNodesInBlock` contains 1 extra slot for temporary storage to
* facilitate splits.)
* @internal
*/
export const MaxNodesInBlock = 8;
/**
* @internal
*/
export class MergeBlock implements IMergeNodeCommon {
public children: IMergeNode[];
public needsScour?: boolean;
Expand Down Expand Up @@ -873,47 +815,3 @@ export const compareNumbers = (a: number, b: number): number => a - b;
* Compares two strings.
*/
export const compareStrings = (a: string, b: string): number => a.localeCompare(b);

/**
* Get a human-readable string for a given {@link Marker}.
*
* @remarks This function is intended for debugging only. The exact format of
* this string should not be relied upon between versions.
* @internal
*/
export function debugMarkerToString(marker: Marker): string {
let bbuf = "";
if (refTypeIncludesFlag(marker, ReferenceType.Tile)) {
bbuf += "Tile";
}
let lbuf = "";
const id = marker.getId();
if (id) {
bbuf += ` (${id}) `;
}
const tileLabels = refGetTileLabels(marker);
if (tileLabels) {
lbuf += "tile -- ";
for (let i = 0, len = tileLabels.length; i < len; i++) {
const tileLabel = tileLabels[i];
if (i > 0) {
lbuf += "; ";
}
lbuf += tileLabel;
}
}

let pbuf = "";
if (marker.properties) {
pbuf += JSON.stringify(marker.properties, (key, value) => {
// Avoid circular reference when stringifying makers containing handles.
// (Substitute a debug string instead.)
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
const handle = !!value && value.IFluidHandle;

// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access
return handle ? `#Handle(${handle.routeContext.path}/${handle.path})` : value;
});
}
return `M ${bbuf}: ${lbuf} ${pbuf}`;
}
17 changes: 7 additions & 10 deletions packages/dds/merge-tree/src/segmentGroupCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import { DoublyLinkedList, walkList } from "./collections/index.js";
import { SegmentGroup, type ISegmentPrivate } from "./mergeTreeNodes.js";

export class SegmentGroupCollection {
private readonly segmentGroups: DoublyLinkedList<SegmentGroup<ISegmentPrivate>>;
private readonly segmentGroups: DoublyLinkedList<SegmentGroup>;

constructor(private readonly segment: ISegmentPrivate) {
this.segmentGroups = new DoublyLinkedList<SegmentGroup<ISegmentPrivate>>();
this.segmentGroups = new DoublyLinkedList<SegmentGroup>();
}

public get size(): number {
Expand All @@ -21,16 +21,16 @@ export class SegmentGroupCollection {
return this.segmentGroups.empty;
}

public enqueue(segmentGroup: SegmentGroup<ISegmentPrivate>): void {
public enqueue(segmentGroup: SegmentGroup): void {
this.segmentGroups.push(segmentGroup);
segmentGroup.segments.push(this.segment);
}

public dequeue(): SegmentGroup<ISegmentPrivate> | undefined {
public dequeue(): SegmentGroup | undefined {
return this.segmentGroups.shift()?.data;
}

public remove?(segmentGroup: SegmentGroup<ISegmentPrivate>): boolean {
public remove?(segmentGroup: SegmentGroup): boolean {
anthony-murphy marked this conversation as resolved.
Show resolved Hide resolved
const found = this.segmentGroups.find((v) => v.data === segmentGroup);
if (found === undefined) {
return false;
Expand All @@ -39,18 +39,15 @@ export class SegmentGroupCollection {
return true;
}

public pop?(): SegmentGroup<ISegmentPrivate> | undefined {
public pop?(): SegmentGroup | undefined {
return this.segmentGroups.pop ? this.segmentGroups.pop()?.data : undefined;
}

public copyTo(segmentGroups: SegmentGroupCollection): void {
walkList(this.segmentGroups, (sg) => segmentGroups.enqueueOnCopy(sg.data, this.segment));
}

private enqueueOnCopy(
segmentGroup: SegmentGroup<ISegmentPrivate>,
sourceSegment: ISegmentPrivate,
): void {
private enqueueOnCopy(segmentGroup: SegmentGroup, sourceSegment: ISegmentPrivate): void {
this.enqueue(segmentGroup);
if (segmentGroup.previousProps) {
// duplicate the previousProps for this segment
Expand Down
2 changes: 0 additions & 2 deletions packages/dds/merge-tree/src/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export {
createInsertSegmentOp,
createMap,
createRemoveRangeOp,
debugMarkerToString,
DetachedReferencePosition,
discardMergeTreeDeltaRevertible,
IJSONMarkerSegment,
Expand Down Expand Up @@ -119,7 +118,6 @@ export {
reservedMarkerSimpleTypeKey,
reservedTileLabelsKey,
revertMergeTreeDeltaRevertibles,
SegmentGroup,
SortedSegmentSet,
SortedSegmentSetItem,
SortedSet,
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/test/reconnectHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import { strict as assert } from "node:assert";
import { ISequencedDocumentMessage } from "@fluidframework/driver-definitions/internal";

import {
SegmentGroup,
endpointPosAndSide,
type IMergeTreeOptions,
type InteriorSequencePlace,
type SequencePlace,
} from "../index.js";
import type { SegmentGroup } from "../mergeTreeNodes.js";
import {
IMergeTreeDeltaOp,
type IMergeTreeInsertMsg,
Expand Down
Loading
Loading