perf(counter): use DoublyLinkedList for pendingOps

[anthony-murphy]

Related PRs

• perf(cell): use DoublyLinkedList for pendingMessageIds with list node on metadata #27415 (prep-cell) — the SharedCell.pendingMessageIds change this PR mirrors.

Description

Replaces SharedCounter's pendingOps: IPendingOperation[] with DoublyLinkedList<IPendingOperation> from @fluidframework/core-utils/internal. Mirrors the merged SharedCell.pendingMessageIds change.

• Push at submit (unchanged signature).
• ACK: pendingOps.shift()?.data instead of Array.shift().
• Rollback: pendingOps.pop()?.data.

Why

Array.shift() is O(n) — every counter ACK reindexes the entire pending tail. Bursty increment() loops are the common counter use; for n in-flight ops, the drain is O(n²). DoublyLinkedList.shift() is O(1).

Notes

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 (104 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

Reviewed commit a7fb6fe on 2026-05-27.

Readiness: 9/10 — ALMOST READY

Both prior Path to Ready items are resolved: the description now links the mirrored SharedCell PR #27415, and counter.spec.ts adds the burst-drain and partial-rollback regression tests. One stale sentence in the description Notes is all that stands between this and sign-off.

Path to Ready

• Update the PR description "Notes" section: drop "no tests touched" and enumerate the two new regression tests in counter.spec.ts (FIFO burst drain; FIFO/LIFO partial rollback). One-line edit. Suggested replacement:

  ## Notes
  
  Pure perf prep — no `reSubmitSquashed`, no squash logic, no api-report changes.
  Adds two regression tests in `counter.spec.ts`:
  - `preserves FIFO order when draining a burst of increments` (FIFO ACK drain).
  - `preserves FIFO order across a burst with partial rollback` (FIFO prefix + LIFO tail rollback).

Context for Reviewers

• The pendingOps machinery being modified — the array, shift() at ack, pop() at rollback, the messageId discriminator, and asserts 0xc8e–0xc92 — was introduced together by scottn12 in PR feat(counter): Add rollback support for SharedCounter #25771 (merged 2025-10-31, reviewed by ChumpChief). counter.ts churn is 2 over 6mo; pendingOps has only the three call sites this PR edits.
• ChumpChief's load-bearing invariant from feat(counter): Add rollback support for SharedCounter #25771 — unique messageId discriminator rather than value-only comparison, because "normal use cases might increment by the same value (esp. +1) on every call and would otherwise be indistinguishable" — is preserved: counter.ts:177 still asserts pendingOp.messageId === messageId and counter.ts:224 still asserts pendingOp.messageId === localOpMetadata after the ?.data unwraps. The new preserves FIFO order when draining a burst of increments test is precisely the bursty-+1 scenario ChumpChief flagged and would trip the messageId assert on any FIFO regression.
• Mirrors companion SharedCell PR perf(cell): use DoublyLinkedList for pendingMessageIds with list node on metadata #27415 — same primitive (DoublyLinkedList from @fluidframework/core-utils/internal), same ?.data placement, same eslint-disable @typescript-eslint/prefer-optional-chain comment retention. The eslint-disable rationale still holds: pendingOp is still IPendingOperation | undefined, so collapsing to pendingOp?.messageId === … would let a missing pending op silently pass the assert.

For human reviewer

• Needs human judgment — Confirm the mirror against the just-landed SharedCell PR perf(cell): use DoublyLinkedList for pendingMessageIds with list node on metadata #27415 is faithful end-to-end (same ?.data idiom, same pop() rollback choice, no post-merge SharedCell fix-ups that should also be ported here).
• Suggested reviewers — ChumpChief (area owner; messageId-discriminator guidance from feat(counter): Add rollback support for SharedCounter #25771), scottn12 (authored pendingOps array + bookkeeping in feat(counter): Add rollback support for SharedCounter #25771; cache is hot), agarwal-navin (op-bunching context from (op bunching) Add processMessagesCore to SharedObject implementations #25745; original remote/reconnection tests).

Review history (1 prior review)

• 296dd15 2026-05-27 · 8/10 — perf swap correct; SharedCell link and burst-test follow-ups requested