From 09b341557de342ad30c164e77b08d784b8e0da8f Mon Sep 17 00:00:00 2001 From: Tony Murphy Date: Wed, 27 May 2026 15:44:38 -0700 Subject: [PATCH 1/4] perf(matrix): use DoublyLinkedList + side-map for pending cell changes --- packages/dds/matrix/src/matrix.ts | 42 +++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/dds/matrix/src/matrix.ts b/packages/dds/matrix/src/matrix.ts index 5816dc24ab84..46d26b8218a4 100644 --- a/packages/dds/matrix/src/matrix.ts +++ b/packages/dds/matrix/src/matrix.ts @@ -8,7 +8,12 @@ import type { IEventThisPlaceHolder, IEventProvider, } from "@fluidframework/core-interfaces"; -import { assert, unreachableCase } from "@fluidframework/core-utils/internal"; +import { + assert, + DoublyLinkedList, + type ListNode, + unreachableCase, +} from "@fluidframework/core-utils/internal"; import type { IChannelAttributes, IFluidDataStoreRuntime, @@ -226,7 +231,12 @@ interface PendingCellChanges { /** * The local changes including the local seq, and the value set at that local seq. */ - local: { localSeq: number; value: MatrixItem }[]; + local: DoublyLinkedList<{ localSeq: number; value: MatrixItem }>; + /** + * Side-map from localSeq to the corresponding node in `local`, enabling O(1) lookup + * during reSubmit instead of a linear `findIndex` scan. + */ + localByLocalSeq: Map }>>; /** * The latest consensus value across all clients. * this will either be a remote value or ack'd local @@ -513,9 +523,14 @@ export class SharedMatrix this.submitLocalMessage(op, metadata); const pendingCell: PendingCellChanges = this.pending.getCell(rowHandle, colHandle) ?? { - local: [], + local: new DoublyLinkedList<{ localSeq: number; value: MatrixItem }>(), + localByLocalSeq: new Map< + number, + ListNode<{ localSeq: number; value: MatrixItem }> + >(), }; - pendingCell.local.push({ localSeq, value }); + const { first: node } = pendingCell.local.push({ localSeq, value }); + pendingCell.localByLocalSeq.set(localSeq, node); this.pending.setCell(rowHandle, colHandle, pendingCell); return pendingCell; } @@ -842,11 +857,13 @@ export class SharedMatrix const pendingCell = this.pending.getCell(rowHandle, colHandle); assert(pendingCell !== undefined, 0xba4 /* local operation must have a pending array */); - const { local } = pendingCell; + const { local, localByLocalSeq } = pendingCell; assert(local !== undefined, 0xba5 /* local operation must have a pending array */); - const localSeqIndex = local.findIndex((p) => p.localSeq === localSeq); - assert(localSeqIndex >= 0, 0xba6 /* local operation must have a pending entry */); - const [change] = local.splice(localSeqIndex, 1); + const node = localByLocalSeq.get(localSeq); + assert(node !== undefined, 0xba6 /* local operation must have a pending entry */); + const change = node.data; + local.remove(node); + localByLocalSeq.delete(localSeq); assert(change.localSeq === localSeq, 0xba7 /* must match */); if ( @@ -905,12 +922,14 @@ export class SharedMatrix const pendingCell = this.pending.getCell(setMetadata.rowHandle, setMetadata.colHandle); assert(pendingCell !== undefined, 0xba9 /* must have pending */); - const change = pendingCell.local.pop(); + const change = pendingCell.local.pop()?.data; assert(change?.localSeq === setMetadata.localSeq, 0xbaa /* must have change */); + pendingCell.localByLocalSeq.delete(setMetadata.localSeq); const previous = pendingCell.local.length > 0 - ? pendingCell.local[pendingCell.local.length - 1].value + ? // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + pendingCell.local.last!.data.value : pendingCell.consensus; this.setCellCore( @@ -1075,8 +1094,9 @@ export class SharedMatrix this.cols.removeLocalReferencePosition(colsRef); const pendingCell = this.pending.getCell(rowHandle, colHandle); - const ackedChange = pendingCell?.local.shift(); + const ackedChange = pendingCell?.local.shift()?.data; assert(ackedChange?.localSeq === localSeq, 0xbab /* must match */); + pendingCell?.localByLocalSeq.delete(localSeq); if (pendingCell?.local.length === 0) { this.pending.setCell(rowHandle, colHandle, undefined); } From b065a1c8255ebcebeb545eca8533e4526032aab7 Mon Sep 17 00:00:00 2001 From: Tony Murphy Date: Wed, 27 May 2026 16:25:57 -0700 Subject: [PATCH 2/4] chore(matrix): biome format --- packages/dds/matrix/src/matrix.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/dds/matrix/src/matrix.ts b/packages/dds/matrix/src/matrix.ts index 46d26b8218a4..76211577ab57 100644 --- a/packages/dds/matrix/src/matrix.ts +++ b/packages/dds/matrix/src/matrix.ts @@ -524,10 +524,7 @@ export class SharedMatrix this.submitLocalMessage(op, metadata); const pendingCell: PendingCellChanges = this.pending.getCell(rowHandle, colHandle) ?? { local: new DoublyLinkedList<{ localSeq: number; value: MatrixItem }>(), - localByLocalSeq: new Map< - number, - ListNode<{ localSeq: number; value: MatrixItem }> - >(), + localByLocalSeq: new Map }>>(), }; const { first: node } = pendingCell.local.push({ localSeq, value }); pendingCell.localByLocalSeq.set(localSeq, node); From 42d0e7666b9282cec98bcc841659384f1a8b9730 Mon Sep 17 00:00:00 2001 From: Tony Murphy Date: Wed, 27 May 2026 16:50:40 -0700 Subject: [PATCH 3/4] refactor(matrix): encapsulate pending local cell changes in PendingLocalCellChanges wrapper --- packages/dds/matrix/src/matrix.ts | 88 ++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 20 deletions(-) diff --git a/packages/dds/matrix/src/matrix.ts b/packages/dds/matrix/src/matrix.ts index 76211577ab57..c14a78854468 100644 --- a/packages/dds/matrix/src/matrix.ts +++ b/packages/dds/matrix/src/matrix.ts @@ -224,6 +224,66 @@ type FirstWriterWinsPolicy = cellLastWriteTracker: SparseArray2D; }; +/** + * Encapsulates the list of pending local changes for a single cell, plus the side-index from + * `localSeq` to the corresponding `DoublyLinkedList` node enabling O(1) lookup during reSubmit + * (instead of a linear `findIndex` scan). + * + * The list and the index are kept in lock-step: every mutator updates both. + */ +class PendingLocalCellChanges { + private readonly list = new DoublyLinkedList<{ localSeq: number; value: MatrixItem }>(); + private readonly index = new Map< + number, + ListNode<{ localSeq: number; value: MatrixItem }> + >(); + + public get length(): number { + return this.list.length; + } + + public get last(): ListNode<{ localSeq: number; value: MatrixItem }> | undefined { + return this.list.last; + } + + public push(localSeq: number, value: MatrixItem): void { + assert(!this.index.has(localSeq), "duplicate localSeq in PendingLocalCellChanges"); + const { first: node } = this.list.push({ localSeq, value }); + // single-item push: first === last, both reference the newly inserted node. + this.index.set(localSeq, node); + } + + public removeByLocalSeq( + localSeq: number, + ): { localSeq: number; value: MatrixItem } | undefined { + const node = this.index.get(localSeq); + if (node === undefined) { + return undefined; + } + this.list.remove(node); + this.index.delete(localSeq); + return node.data; + } + + public shift(): { localSeq: number; value: MatrixItem } | undefined { + const node = this.list.shift(); + if (node === undefined) { + return undefined; + } + this.index.delete(node.data.localSeq); + return node.data; + } + + public pop(): { localSeq: number; value: MatrixItem } | undefined { + const node = this.list.pop(); + if (node === undefined) { + return undefined; + } + this.index.delete(node.data.localSeq); + return node.data; + } +} + /** * Tracks pending local changes for a cell. */ @@ -231,12 +291,7 @@ interface PendingCellChanges { /** * The local changes including the local seq, and the value set at that local seq. */ - local: DoublyLinkedList<{ localSeq: number; value: MatrixItem }>; - /** - * Side-map from localSeq to the corresponding node in `local`, enabling O(1) lookup - * during reSubmit instead of a linear `findIndex` scan. - */ - localByLocalSeq: Map }>>; + local: PendingLocalCellChanges; /** * The latest consensus value across all clients. * this will either be a remote value or ack'd local @@ -523,11 +578,9 @@ export class SharedMatrix this.submitLocalMessage(op, metadata); const pendingCell: PendingCellChanges = this.pending.getCell(rowHandle, colHandle) ?? { - local: new DoublyLinkedList<{ localSeq: number; value: MatrixItem }>(), - localByLocalSeq: new Map }>>(), + local: new PendingLocalCellChanges(), }; - const { first: node } = pendingCell.local.push({ localSeq, value }); - pendingCell.localByLocalSeq.set(localSeq, node); + pendingCell.local.push(localSeq, value); this.pending.setCell(rowHandle, colHandle, pendingCell); return pendingCell; } @@ -854,13 +907,10 @@ export class SharedMatrix const pendingCell = this.pending.getCell(rowHandle, colHandle); assert(pendingCell !== undefined, 0xba4 /* local operation must have a pending array */); - const { local, localByLocalSeq } = pendingCell; + const { local } = pendingCell; assert(local !== undefined, 0xba5 /* local operation must have a pending array */); - const node = localByLocalSeq.get(localSeq); - assert(node !== undefined, 0xba6 /* local operation must have a pending entry */); - const change = node.data; - local.remove(node); - localByLocalSeq.delete(localSeq); + const change = local.removeByLocalSeq(localSeq); + assert(change !== undefined, 0xba6 /* local operation must have a pending entry */); assert(change.localSeq === localSeq, 0xba7 /* must match */); if ( @@ -919,9 +969,8 @@ export class SharedMatrix const pendingCell = this.pending.getCell(setMetadata.rowHandle, setMetadata.colHandle); assert(pendingCell !== undefined, 0xba9 /* must have pending */); - const change = pendingCell.local.pop()?.data; + const change = pendingCell.local.pop(); assert(change?.localSeq === setMetadata.localSeq, 0xbaa /* must have change */); - pendingCell.localByLocalSeq.delete(setMetadata.localSeq); const previous = pendingCell.local.length > 0 @@ -1091,9 +1140,8 @@ export class SharedMatrix this.cols.removeLocalReferencePosition(colsRef); const pendingCell = this.pending.getCell(rowHandle, colHandle); - const ackedChange = pendingCell?.local.shift()?.data; + const ackedChange = pendingCell?.local.shift(); assert(ackedChange?.localSeq === localSeq, 0xbab /* must match */); - pendingCell?.localByLocalSeq.delete(localSeq); if (pendingCell?.local.length === 0) { this.pending.setCell(rowHandle, colHandle, undefined); } From abd04d4b1f9ddd6c29c3b3a39f6cff23a3d8abc7 Mon Sep 17 00:00:00 2001 From: Tony Murphy Date: Wed, 27 May 2026 17:25:59 -0700 Subject: [PATCH 4/4] test(matrix): hot-cell reconnect regression --- .../matrix/src/test/matrix.reconnect.spec.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/dds/matrix/src/test/matrix.reconnect.spec.ts b/packages/dds/matrix/src/test/matrix.reconnect.spec.ts index f642241d93b4..ae067c0e6f65 100644 --- a/packages/dds/matrix/src/test/matrix.reconnect.spec.ts +++ b/packages/dds/matrix/src/test/matrix.reconnect.spec.ts @@ -231,4 +231,54 @@ describe("SharedMatrix reconnect", () => { assert.deepEqual(extract(matrix1), expected); assert.deepEqual(extract(matrix3), expected); }); + + // Regression test for the resubmit path optimized when many pending writes target the + // same (row, col). Disconnect, write the same cell N times, reconnect, and assert that + // only the final value remains and that both clients converge. + it("resubmits N writes to the same cell after reconnect", () => { + const containerRuntimeFactory = new MockContainerRuntimeFactoryForReconnection(); + const dataRuntime1 = new MockFluidDataStoreRuntime(); + containerRuntimeFactory.createContainerRuntime(dataRuntime1); + const dataRuntime2 = new MockFluidDataStoreRuntime(); + const containerRuntime2 = containerRuntimeFactory.createContainerRuntime(dataRuntime2); + + const matrix1 = matrixFactory.create(dataRuntime1, "A"); + matrix1.connect({ + deltaConnection: dataRuntime1.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + + const matrix2 = matrixFactory.create(dataRuntime2, "B"); + matrix2.connect({ + deltaConnection: dataRuntime2.createDeltaConnection(), + objectStorage: new MockStorage(), + }); + + // Create a 1x1 matrix. + matrix1.insertRows(0, 1); + matrix1.insertCols(0, 1); + containerRuntimeFactory.processAllMessages(); + + // Disconnect matrix2, then write the same (row, col) N times. This stages a + // large run of pending setCell ops all targeting the same cell, which is the + // hot path being optimized by the pending-cell-list change. + containerRuntime2.connected = false; + const N = 200; + for (let i = 0; i < N; i++) { + matrix2.setCell(0, 0, i); + } + + // While disconnected, matrix2 sees the latest local write and matrix1 sees nothing. + assert.deepEqual(extract(matrix1), [[undefined]]); + assert.deepEqual(extract(matrix2), [[N - 1]]); + + // Reconnect matrix2 to force the resubmit path to re-emit all N pending writes. + containerRuntime2.connected = true; + containerRuntimeFactory.processAllMessages(); + + // Both clients should converge on the last-written value. + const expected = [[N - 1]]; + assert.deepEqual(extract(matrix1), expected); + assert.deepEqual(extract(matrix2), expected); + }); });