perf(matrix): use DoublyLinkedList + side-map for pending cell changes

[anthony-murphy]

Description

Replaces PendingCellChanges<T>.local: { localSeq, value }[] with DoublyLinkedList<{ localSeq, value }> and adds a sibling side-map localByLocalSeq: Map<number, ListNode<{ localSeq, value }>> for O(1) findIndex-by-localSeq lookup.

Sites converted:

• sendSetCellOp push: insert into list, register node in side-map.
• reSubmitCore: was findIndex + splice per pending op; now localByLocalSeq.get(localSeq) → list.remove(node) → localByLocalSeq.delete(localSeq). O(1).
• ACK shift: list.shift()?.data + side-map delete.
• Rollback pop() + peek-tail list.last!.data.value.
• Length probes use list.length.

Why

For a hot cell with k pending writes, the previous reSubmit-per-op was O(k) find + O(k) splice (O(k²) total) and per-ACK was O(k) shift. For n ops to a single hot cell, the whole rebase pass was O(n²). The list-plus-side-map makes every per-op step O(1).

Notes

Snapshot/wire format unaffected (_pendingCliSeqData is unknown). Pure perf prep — no reSubmitSquashed, no squash logic, no tests touched, no api-report changes.

[github-actions]

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (131 lines, 2 files), I've queued these reviewers:

• Correctness — logic errors, race conditions, lifecycle issues
• Security — vulnerabilities, secret exposure, injection
• API Compatibility — breaking changes, release tags, type design
• Performance — algorithmic regressions, memory leaks
• Testing — coverage gaps, hollow tests

How this works

• Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

• Tick Start review below to dispatch the review fleet.

• After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

• This comment updates as new commits land; your reviewer selections are preserved.

• Start review

[anthony-murphy]

Deep Review: pop() here is correct — rollback is LIFO, the operation being rolled back is always the tail, and assert 0xbaa enforces the match. But on #24604 the Copilot bot flagged this exact construct ("Using pop() to remove the pending operation may remove the most recently added op rather than the one corresponding to the rollback"), and now that this PR adds the localByLocalSeq index — making targeted removal trivial and applying it in reSubmitCore — a reader will reasonably ask why rollback didn't switch too. Switching would be semantically wrong (rollback must restore the previous cell value, which requires popping the tail — see matrix.rollback.spec.ts:163-187, :232-290), so the code is right; only the intent is implicit.

A one-line comment above the pop() pre-empts the same #24604 round-trip.

Suggested change

	pendingCell.localByLocalSeq.delete(setMetadata.localSeq);
	// Rollback is LIFO: the operation being rolled back is always the tail. `pop()` is correct; assert 0xbaa enforces the match.
	const change = pendingCell.local.pop()?.data;

[anthony-murphy]

Deep Review

Reviewed commit abd04d4 on 2026-05-27.

Readiness: 8/10 — ALMOST READY

Faithful, well-scoped perf prep: PendingCellChanges<T>.local is now a private PendingLocalCellChanges<T> wrapper around DoublyLinkedList + Map<localSeq, ListNode>, turning the per-cell reSubmit hot path from O(k²) to O(k). The hot-cell reconnect regression asked for in the prior review landed at matrix.reconnect.spec.ts:234-283 (N=200), and the lock-step-invariant thread is resolved by the wrapper-class adoption that defines this commit. No correctness defects. Five Tier 3 polish items remain — the last-leaks-ListNode ergonomic gap (the PR's only eslint-disable), an untagged assert, the rollback pop()-is-LIFO-only note, undocumented per-member contracts on the wrapper (esp. the push uniqueness precondition), and no micro-bench guarding the asymptotic claim.

Path to Ready

• Resolve inline threads
• File a follow-up issue for a hot-cell micro-bench in packages/dds/matrix/src/test/matrix.bench.ts (N disconnected setCell to the same (row, col) + reconnect/ACK drain) so a future regression that reintroduces an O(N) per-op scan is caught

Context for Reviewers

• Third step on a 5-year localSeq-keyed pending-op trajectory: DLehenbauer Matrix: Use 'localOpMetadata' instead of private queue #2540 (2020) introduced localOpMetadata/localSeq; Abe27342 fix(matrix): Properly detect row/cell deletion when resubmitting a set op #19862 (2024) hardened resubmit with local reference positions; anthony-murphy Matrix: Enable rollback and local server stress #24604 (2025) introduced PendingCellChanges<T>. This PR is the internal data-structure swap inside that container, not a re-opening of the pending-cell design.
• Wire/snapshot format untouched (_pendingCliSeqData: unknown); evaluate as incremental optimisation.
• Rollback (matrix.ts:972-979) and ACK (:1142-1146) share the changed structure — validation should not focus only on the reSubmit hot path.
• Recurring failure modes in this file's history: pending-op identity vs position (Copilot flagged pop() and a wrong-index splice on Matrix: Enable rollback and local server stress #24604), pending-cell order sensitivity on resubmit (Implement First Write Win for Shared Matrix #18018 vladsud), resubmit rebase over deleted rows/cols (fix(matrix): Properly detect row/cell deletion when resubmitting a set op #19862 Abe27342, adjacent to the new removeByLocalSeq → list.remove(node) in reSubmitCore), and FWW eventual-consistency surfaced by fuzz (Matrix: Combine state, fix bugs, and add fuzz for FFW #24652).
• Suggested domain reviewers: Josmithr (Matrix: Enable rollback and local server stress #24604 design owner; cares about last-shape and member docs), jatgarg (bulk-tagged matrix asserts in build(client): Tag asserts for release 2.42.0 #24797 establishing the convention the new assert breaks; also owns FFW interacting with ACK), Abe27342 (resubmit correctness fix(matrix): Properly detect row/cell deletion when resubmitting a set op #19862), DLehenbauer (foundational localSeq design Matrix: Use 'localOpMetadata' instead of private queue #2540), optional vladsud (Implement First Write Win for Shared Matrix #18018 pending-cell ordering).

For human reviewer

• Encapsulation taste call (Josmithr) — the wrapper-class direction was already adopted at the design-owner's request (thread 3314510112, resolved). The remaining ergonomic leak (last returning ListNode) is matrix-area taste; if peekNewest() is acceptable, the inline thread's recommendation lands cleanly.
• Perf-direction validation — the pipeline cannot measure runtime. Once the bench follow-up lands, confirm the measured improvement matches the O(k²) → O(k) claim under Matrix: Enable rollback and local server stress #24604's stress workloads.
• FWW ACK glance (jatgarg) — the ACK-path conversion from array shift() to list.shift()?.data (matrix.ts:1142-1146) is mechanically equivalent but worth a domain-expert look given Update shared matrix FWW policy to immediately switch mode on switch rather than on ack #18709's pending-write sequencing history.
• Resubmit-over-deleted-row/col (Abe27342) — fix(matrix): Properly detect row/cell deletion when resubmitting a set op #19862's fix sits adjacent to the new removeByLocalSeq → list.remove(node) in reSubmitCore. The local-ref-position dance is unchanged but worth a confirming glance.

Review history (3 prior reviews)

• 42d0e76 2026-05-27 · 7/10 — perf prep verified; four polish items, hot-cell regression test most material
• b065a1c 2026-05-27 · 7/10 — perf prep verified; three polish items, hot-cell regression test most material
• 09b3415 2026-05-27 · 6/10 — perf prep with a single hot-cell regression-test blocker