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 e2cafc3 on 2026-06-02.

Readiness: 9/10 — ALMOST READY

A clean, mechanically-equivalent swap of pendingOps: IPendingOperation[] for DoublyLinkedList<IPendingOperation>, mirroring the merged SharedCell PR #27415, with two well-scoped FIFO/rollback regression tests. No correctness defects. One editorial item — the description's "Notes" still claims "no tests touched" — 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

• Mirrors the merged 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 at both read sites, same eslint-disable @typescript-eslint/prefer-optional-chain retention, same pop() rollback choice. If perf(cell): use DoublyLinkedList for pendingMessageIds with list node on metadata #27415 was approved as-is this should be a quick rubber-stamp; worth a sanity-check that no post-merge SharedCell fix-ups need to be ported here.
• 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.

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 (DDS area owner; messageId-discriminator guidance from feat(counter): Add rollback support for SharedCounter #25771), scottn12 (authored pendingOps array + tagged asserts in feat(counter): Add rollback support for SharedCounter #25771/Tag asserts for 2.71.0 release. #25794), agarwal-navin (consult only if batched-processing interaction surfaces).

Review history (2 prior reviews)

• a7fb6fe 2026-05-27 · 9/10 — both prior Path to Ready items resolved; one stale description sentence remains
• 296dd15 2026-05-27 · 8/10 — perf swap correct; SharedCell link and burst-test follow-ups requested