bench(net): sweep a front's route-swap copy walk

[kixelated]

Problem

A front resumes a route change by walking every track it serves and every copy those tracks still hold (drain_copies, then each reader's sync). Nothing measured that walk, so a cost that grows with the front would not show up as a slope.

Approach

origin/copy_walk in the existing moq-net origin bench sweeps tracks per front against copies still held per track. Setup builds the front and the next generation and stays outside the timed section. The timed step announces that newer same-epoch local route and waits until it is spliced. Subscribers are never polled, and every earlier generation stays held, so a replaced copy remains for the walk. The wait ends on demand, which bounds the splice only on a current-thread runtime; the bench says so.

Nightly already runs every moq-net Criterion target once (cargo bench --locked -p moq-net --features fuzz --bench '*' -- --test). This target has no required-features, so the new routine is in that smoke.

Local means, 10 samples, 1s warm-up, 2s measurement:

tracks	copies	mean
1	1	4.2 µs
1	32	6.3 µs
32	1	186 µs
64	1	769 µs
128	1	2.16 ms
8	32	25.7 µs
32	8	173 µs
64	16	591 µs

The per-track cost is superlinear: doubling tracks roughly triples to quadruples the swap. run_front's wait closure returns one Step::Info per pass and rescans every track from the start, so a swap over T local tracks is about T²/2 track visits. Extrapolated, a few hundred tracks is a 10 ms-scale hitch. Thirty-two extra copies on one track add about 2 µs. This change records the slopes; admission and failover behavior are unchanged.

Impact

• Public API: none.
• Wire: none.

Alternatives

A session bench would time the same driver through the network. The walk runs in-process on the front, and the origin bench reaches it through the public origin API.

Changing the walk in this quest would mix a measurement with a behavior change.

Follow-ups

• The per-track slope is quadratic. Recommendation: a quest to make one run_front pass drain every ready track query instead of returning after the first, measured against origin/copy_walk. It is separate from front deadlines (quest/m1/front-deadline-index.md), which covers deadline scans and driver wakes.
• The per-copy slope is real and small at these sizes. Recommendation: leave it until a profile shows Reader::sync walking retained copies as the dominant cost.

(Written by Grok 4.7, updated by Claude 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 24 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

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5e194fd0-01da-4347-95a1-9fb5806901d3

📥 Commits

Reviewing files that changed from the base of the PR and between 03529eb and 4327154.

📒 Files selected for processing (3)

• quest/m1/README.md
• quest/m1/admission-bench.md
• rs/moq-net/benches/origin.rs

• Autopilot · 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

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Grok review of a9aaedd9 (full review, since there was no earlier Grok review. The head is a main merge on top of 05d08cdd, and the PR's own changes are the bench plus the quest cleanup.)

The bench is set up the way the PR body says. Each new same-epoch local generation wins on route_order's newest-id tiebreak, a local Request resolves synchronously (origin.rs:2509), and readers that are never polled never reach let_go, so each Reader really does hold copies replaced copies when the timed swap runs. I found no blocking issues. The findings below are about what the numbers mean, because the Follow-ups section draws conclusions from them.

Non-blocking

1. The timed step also builds the new generation, so the per-track figure isn't only the splice. CopyWalk::swap calls publish_generation, which runs create_broadcast, tracks × (create_track, append_group, write_frame, finish), and then announce, all inside the timed section. That is O(tracks) work that has nothing to do with the walk, and it lands on the per-track slope the PR attributes to "the per-track splice". Fix: split publish_generation into a build step and an announce step. Build the next Generation without announcing it during setup (store it on CopyWalk), and have swap() only call announce and wait_spliced.

2. The per-track cost grows with the track count, which looks superlinear, and the sweep can't show the shape yet. Your own table goes from about 6 µs per track at 32 tracks to about 10 µs per track at 64 tracks. Hunch, please verify: run_front's kio::wait closure returns one Step::Info per pass and rescans tracks from the start each time (origin.rs:2768-2803), polling every earlier track's demand and closed state on the way. If so, a swap over T local tracks is about T²/2 track visits. That matters for the Follow-up recommendation. Extrapolating linearly, "a few hundred tracks" is a millisecond-scale hitch, but quadratically 256 tracks would be roughly 16 × 625 µs, which is about 10 ms. Also, 64 tracks is only measured at 16 copies, so tracks and copies are mixed together at the top of the sweep. Adding (64, 1) and (128, 1) (or (256, 1)) would show whether the slope is linear before anyone decides to leave it alone.

3. The completion signal is demand, not the splice. wait_spliced waits on track.demand().used(), which flips at Action::Query (copy.subscribe(demand), origin.rs:2618), not at Action::Splice. It only bounds the splice because the runtime is current_thread and every local query is ready within the same front-task poll. The swap() doc mostly says this. Consider stating outright that the bench must stay on a current-thread runtime, and that a query that isn't immediately ready would stop the clock before the walk. Otherwise a later "use the multi-thread runtime" change would quietly stop measuring the walk without failing anything.

4. Nit: Throughput::Elements((tracks * copies) as u64) makes Criterion's elements-per-second figure read as per-copy cost, when the cost is per track. Elements(tracks as u64), or no throughput at all, would be less misleading.

Quest cleanup: removing quest/m1/admission-bench.md and its quest/m1/README.md line is consistent, and nothing else on the head links to it. CI was still pending when I reviewed.

Verdict: MERGE once CI is green. Items 1 and 2 are worth fixing first if the Follow-ups recommendation is meant to stand on these numbers.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Addressed in 33f1c87 (head 4327154, which also merges main with #5016):

1. Fixed. Generation::build runs in setup and the timed swap only announces and waits.
2. Confirmed. Added (64, 1) and (128, 1): 186 µs, 769 µs, 2.16 ms at 32, 64, 128 tracks, so the per-track cost is superlinear. The PR body now reports the quadratic slope and recommends a follow-up quest for the run_front rescan.
3. Fixed. Generation::announce documents that the demand wait bounds the splice only on a current-thread runtime, and bench_copy_walk points at it.
4. Fixed. Throughput is per track.

(Written by Claude Opus 5.5)

[kixelated]

Grok follow-up review of 4327154f (re-review after the push from a9aaedd9. The new PR change is 33f1c87b; 4327154f is a main merge, and main's changes are left out.)

33f1c87b addresses all four earlier findings, and the reworked bench looks correct. Generation::build creates the broadcast hidden (create_broadcast keeps it out of cursors and requests until it announces, origin.rs:1583-1586), so building next during setup doesn't disturb the front. iter_batched_ref with BatchSize::PerIteration gives each timed swap() a fresh rig, so next.take().expect("one swap per setup") can't fire, and teardown stays untimed. I found no blocking issues.

Earlier findings

1. Fixed. The timed step now only announces the prebuilt generation and waits (CopyWalk::swap calls Generation::announce); all the build work moved to setup.
2. Fixed. (64, 1) and (128, 1) are in the sweep, and the new numbers confirm the superlinear slope (186 µs, then 769 µs, then 2.16 ms). I checked the hunch on this head: run_front's kio::wait closure walks the tracks HashMap from the start and returns on the first ready query (origin.rs:2766-2773), while still polling every earlier track's subscription, copy-closed, and demand state on the way. So the T²/2 description in the PR body holds.
3. Fixed. Generation::announce and bench_copy_walk now say the wait ends on demand and that the runtime must stay current-thread.
4. Fixed. Throughput::Elements(tracks as u64).

Non-blocking

1. The proposed follow-up quest overlaps quest/m1/front-deadline-index.md. The PR body says that quest only "covers deadline scans and driver wakes", but its Plan also says "Replace the driver's scan-every-track poll with per-track wakes, so an event on one track polls that track", which is the same run_front closure. With per-track wakes, each pass touches only the ready tracks, so a swap would already be O(tracks) without a separate "drain every ready query" quest. Suggestion: point the Follow-up at front-deadline-index.md and name origin/copy_walk as one of its proof benchmarks, or say why per-track wakes wouldn't flatten this slope.
2. The copies dimension runs backwards at larger track counts, so the per-copy claim rests on one data point. 32t_8c (173 µs) is faster than 32t_1c (186 µs), and 64t_16c (591 µs) is faster than 64t_1c (769 µs). Only 1t_1c to 1t_32c (4.2 µs to 6.3 µs) shows a per-copy cost. With 10 samples, run-to-run noise at those sizes is clearly bigger than the copy effect. The Follow-up's conclusion ("real and small... leave it") is probably still right. It would just be more honest to say the per-copy slope is only visible on the single-track points.

Quest cleanup is unchanged and still consistent; nothing on the head links to admission-bench.md. CI was still pending when I reviewed.

Verdict: MERGE once CI is green.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 4327154

No actionable correctness finding in the three-file change. rs/moq-net/benches/origin.rs:779–816 builds the next generation outside the timed swap; :827–840 uses a fresh batched rig and current-thread runtime. The unpolled subscribers and retained generations preserve the replaced copies being measured. I traced the demand-before-splice ordering through model/origin.rs:2614–2650 and the ready-query scan at :2766–2773; the benchmark's current-thread/immediately-ready restriction is important and is now explicit.

Direction: useful focused instrumentation, with per-track sweep points that can reveal the driver's rescan cost. The existing discussion already cautions about noisy per-copy comparisons and overlap with the per-track-wake quest; no duplicate findings. Reported timings were not independently reproduced.

Verification: GitHub-only static diff, relevant source and discussion review. No builds, tests, benchmarks or external-peer runs executed. Open state, exact head and prior reviews rechecked before posting.

[kixelated]

Merge summary: the OpenAI review of head 4327154f has no findings, and all Grok findings were fixed in 33f1c87b. CI is green. Auto-merge is enabled on 4327154f.

(Written by Claude Opus 5.5)