[kixelated]

Finishes quest/m2/cluster-routing/routes/show-cost.md.

Problem

moq-dev/moq#8352 measured moq-lite-07 session setup at 4096 broadcasts at about 5x moq-lite-06's CPU. It blamed Router::show. That diagnosis was wrong. Re-measured on main after moq-dev/moq#8374, the router is already incremental. The quadratic work sits in the lite-07 announce stream:

• The subscriber (lite/subscriber/routed.rs) checks every ANNOUNCE_START for a duplicate path by scanning all of the route's live claims. Receiving n claims costs O(n²), and a peer can force that cost on us.
• The publisher (lite/publisher/routed.rs) scans all of the route's claims for a withheld one on every ANNOUNCE_FLOOR. A single claim change then costs O(claims held) on each answer stream.

Approach

• Subscriber: WireRoute::claims maps each path to its Announce ID, so the duplicate check is a single lookup.
• Publisher: Sent keeps a count of the claims its link withholds. Sent::set updates the count on every claim change, so the floor reads it instead of scanning.
• New session_announce bench: one broadcast announced and then withdrawn beside many that are already announced, timed until every viewer session has seen both. It sweeps the broadcasts held (16, 4096) against the viewers (1, 64), per the fan-out rule. The route bench's routes/received already sweeps claims held against claims changed for the router, so no new route-bench case is added.
• Deletes the quest. Also links node-creation-order.md in the routes epic: main left it unlinked after moq-dev/moq#8388, and that failed quest check.

Measurements

All numbers are on an M-series Mac under load. Setup and teardown come from one 4096-broadcast session_delivery_broadcasts cell (1 publisher, 16 viewers, 1 relay), in thread CPU, using temporary instrumentation that is not committed.

	lite-06	lite-07 main	lite-07 this PR
setup, 256 broadcasts	67 ms	106 ms	78 ms
setup, 4096 broadcasts	813 ms	4236 ms	1021 ms
teardown, 4096 broadcasts	643 ms	713 ms	639 ms

The subscriber fix alone accounts for the whole setup gain: 998 ms at 4096 broadcasts. Setup sends all claims in one batch, so the publisher scan does not show there.

session_announce: time per announce+withdraw, criterion mean.

shape	lite-06	lite-07 main	lite-07 this PR	mt-22
broadcasts=16, viewers=1	26 µs	49 µs	49 µs	32 µs
broadcasts=4096, viewers=1	26 µs	154 µs	57 µs	50 µs
broadcasts=16, viewers=64	787 µs	1.34 ms	1.21 ms	1.02 ms
broadcasts=4096, viewers=64	947 µs	16.2 ms	2.58 ms	4.42 ms

The publisher fix carries this table. With the subscriber fix alone, one change at 4096 broadcasts and 16 viewers took 5.3 ms of CPU; with both fixes it takes 1.4 ms. The remaining lite-07 slope over broadcasts held is about 2x across a 256x sweep. A profile of that cell shows BTreeMap lookups (memcmp on paths) inside Router::batch, advertise, and show. That is a logarithmic cost, not a scan.

session_join_broadcasts at broadcasts=1024 (lite-07): relays=1 goes from 24.3 to 10.5 ms, relays=2 from 24.1 to 10.3 ms. lite-06 is 9.3 to 10.0 ms.

Impact

• Public API: none. Wire: none.

Alternatives

• Making Router::show incremental, as the quest planned: it already is. Profiles put no router function at the top.

Validation

• quest check: ok.
• just fix: no changes.
• just rs check-test -p moq-net -p moq-relay: 4207 tests passed, lint ok.
• cargo clippy --all-targets -p moq-net -p moq-relay --features moq-net/fuzz -D warnings: clean. This covers both session benches.
• just check: fails on two problems that are not this PR's. Both reproduce unchanged in a checkout of origin/main (ac3cb8c):
  • python3 sh/version.test.py: test_cut_promotes_only_published_library_dependencies_and_preserves_exemptions and test_release_plz_keeps_manual_cut_despite_breaking_commits fail.
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps -p moq-carla-bridge: unresolved links to Bridge, Source, and protocol in demo/carla/bridge/src/lib.rs. The rustdoc failure stopped check rs before its tests ran, so I ran them per crate above.

Choices made without asking

• The quest is finished rather than rewritten: its goal (setup and teardown linear, as on lite-06) holds. Its suspected cause did not.
• Kept the publisher fix: the setup numbers miss it, but session_announce shows it.
• Linked node-creation-order.md here to restore quest check on main. No open PR fixed it.

Follow-ups

• Plan: after a lite-07 room drops, its sessions leave tasks alive: 18 in the 16-viewer delivery cell, against 0 on lite-06. Each looks like one task per session that never ends. The per-cell runtimes from fix(net): the session bench gives each cell its own runtime and a proxying relay #8352 hide this in the bench. Check whether a lite-07 session's driver outlives its handles.
• Plan: demo/carla/bridge rustdoc fails -D warnings on main, which fails just check for any change that selects it.
• Plan: two sh/version.test.py cases fail on main locally. Check whether this is environment-dependent before fixing.

Review

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1c393-0a22-7650-8562-67b8714b983f, round 1
• Head: 5767f82
• Base: origin/main at f8a0c76
• Findings: none

Models

Size: M
Implementer: Claude Code, Opus 5.5

(Written by Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: badf700e-85fe-46ac-9d43-11387271a12a

Commits

Reviewing files that changed from the base of the PR and between ac3cb8c and 5767f82.

Files selected for processing: 5

• quest/m2/cluster-routing/routes/README.md
• quest/m2/cluster-routing/routes/show-cost.md
• rs/moq-net/benches/session/mod.rs
• rs/moq-net/src/lite/publisher/routed.rs
• rs/moq-net/src/lite/subscriber/routed.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head: 5767f82
Base: ac3cb8c

No actionable findings. The subscriber's path index preserves duplicate-path rejection and removes entries on both claim and route teardown. The publisher's withheld count covers insertions, replacements, removals, and mounted-source withdrawal; its upstream/proxy predicate is fixed for the answer's lifetime, so the cached count preserves the prior floor-filtering behavior.

Direction: removing the two announce-stream scans targets the observed cost without redesigning the already-incremental router. The new benchmark excludes room setup and waits for every viewer's announce and withdrawal, sweeping held broadcasts and viewers independently.

Limits: GitHub-only source review, including surrounding lifecycle code, decoder retirement, existing tests, and benchmark wiring. I did not run tests or benchmarks or reproduce the reported speedups. GitHub Check was in progress when inspected; the PR also reports local full-check failures outside this diff, which I have not independently reproduced.

[pullfrog]

✅ No new issues found.

Reviewed changes

• Subscriber path index: WireRoute::claims becomes BTreeMap<Path, u64>, so a duplicate path under a route is one lookup rather than a scan of every claim the route holds; end, RouteEnd, and teardown were updated to match, and the decoder's order-independent retirement keeps path-order iteration equivalent to the old ID order.
• Publisher withheld count: Sent now maintains withheld through Sent::set, which replaces both direct claims mutation sites and folds insert, replace, and remove into one adjustment; floor reads the count instead of scanning, with the link fixed for the answer's lifetime and claims only touched through set.
• New session_announce bench: one announce plus withdrawal beside many held broadcasts, awaited at every viewer, swept over broadcasts held and viewers and run on a per-cell runtime with setup excluded from the sample; Cluster._origins is exposed as origins for it.
• Quest finish: show-cost.md is deleted, its epic bullet is replaced, and node-creation-order.md is linked (the file exists on the branch, and no show-cost reference remains in the tree).

I traced the counter through every mutation path and the duplicate check through start/end/RouteEnd and found no behavioral difference from the removed scans. The only gap worth noting is non-blocking: no test pins Floor.revision when a claim is withheld (existing assertions use ..), so a future regression there would be silent.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}