[kixelated]

Finishes quest/m2/perf/single-delivery-write.md by accepting the measured residual, and deletes it with its link from quest/m2/perf/README.md.

Problem

moq-dev/moq#6922 made every streamed frame publish through a per-frame Publication mutex: admit, every write_chunk (plus an overlap check against bytes another delivery already published), and the admission's release. Nearly every frame has exactly one delivery, so that work protects nothing. moq-dev/moq#7141 recovered the allocations but left this.

Approach

Until anything else attaches to a frame's content, its creating delivery is the only writer:

• Write. The creator publishes without the lock or the overlap check. Its progress is the content's, so nothing published can overlap the chunk.
• Admission. The creator takes its admission with the content (FrameBuf::admitted), before anything else can see it, and keeps its reservation itself. Releasing it alone takes no lock either.
• Attaching. Everything that lets another party reach the content goes through FrameContent::attach under the publication lock: identify (the only way a fork can appear in production), Shared::fork, bill_content, and admit. The race the quest names, a second copy attaching while the creator is mid-write, is ruled out with a store-load pair on each side. The creator stores writing then loads attached, and the attacher stores attached then loads writing, all SeqCst. At least one side sees the other: either the creator takes the locked path, or the attacher yields until that write ends. After that, every write locks exactly as before.
• Reservation rule kept. When something attaches, the creator joins the shared producer count, and its reservation passes to any producer still active when it ends. One reservation per content is still shared and returns once the last producer ends.
• Admission no longer holds an Arc of the content. A frame::Producer releases it through its own buffer, which saves a refcount round trip per frame.
• A group without shared content now skips the per-frame canonicalize call in write_frame and on commit, instead of calling it to return at once.

Measurements

These are user-space instruction counts (cpu_core/instructions/u read per round through perf_event_open), pinned to one P-core. The host is loaded, but these counts are deterministic: repeat runs agree to within 0.01%. Each revision was built from its own export with its own target directory. main is 14ed0f05dd, and base is c67c124b20, the parent of #6922.

256-frame lite-07-wip FETCH round, 1 publisher, 1 viewer:

	base	#6922	#7141	main	this PR
instructions	4.594M	4.970M	4.757M	4.911M	4.835M (-1.55%)

Against main, this PR lowers every session_delivery_fetch_frames case (0.16% to 1.55%) and every fetch_range case but two moq-transport-22 ones, which rise 0.24% and 0.27% (table below). About 87k of #6922's 163k residual (measured at #7141) remains. A symbol profile spreads it over building and dropping the larger FrameContent, the commit path, and reader-side copies, with no locking left. So base parity is not met; the maintainer accepted the residual, see Decisions.

group_write_frames/batch32 is unchanged (512 frames: 243.3k on main, 243.5k here, 216.1k on base). The batch path takes no frame lock, so it shares none of this cost. Two experiments for its residual, which I measured and dropped:

• A one-pointer Entry owner slot (an AtomicPtr cell in place of Mutex<Option<Shared>>) saves 2.6 instructions per frame and adds unsafe.
• Forcing GroupState::commit inline into write_frames costs 3% more.

FETCH sweep, instructions per round (main, this PR)

case	main	this PR	change
fetch_frames/moq-lite-06/publishers=1/viewers=1 frames=4	366,210	365,020	-0.32%
fetch_frames/moq-lite-06/publishers=1/viewers=1 frames=256	4,935,000	4,859,600	-1.53%
fetch_frames/moq-lite-06/publishers=1/viewers=16 frames=4	3,975,900	3,966,800	-0.23%
fetch_frames/moq-lite-06/publishers=1/viewers=16 frames=256	42,249,000	41,648,000	-1.42%
fetch_frames/moq-lite-06/publishers=4/viewers=1 frames=4	367,260	366,050	-0.33%
fetch_frames/moq-lite-06/publishers=4/viewers=1 frames=256	4,935,000	4,859,500	-1.53%
fetch_frames/moq-lite-06/publishers=4/viewers=16 frames=4	4,958,600	4,947,600	-0.22%
fetch_frames/moq-lite-06/publishers=4/viewers=16 frames=256	50,201,000	49,481,000	-1.43%
fetch_frames/moq-lite-07-wip/publishers=1/viewers=1 frames=4	400,310	399,140	-0.29%
fetch_frames/moq-lite-07-wip/publishers=1/viewers=1 frames=256	4,911,000	4,835,100	-1.55%
fetch_frames/moq-lite-07-wip/publishers=1/viewers=16/watc... frames=4	4,283,400	4,263,800	-0.46%
fetch_frames/moq-lite-07-wip/publishers=1/viewers=16/watc... frames=256	40,865,000	40,259,000	-1.48%
fetch_frames/moq-lite-07-wip/publishers=4/viewers=1 frames=4	400,350	399,190	-0.29%
fetch_frames/moq-lite-07-wip/publishers=4/viewers=1 frames=256	4,911,100	4,835,000	-1.55%
fetch_frames/moq-lite-07-wip/publishers=4/viewers=16/watc... frames=4	5,315,900	5,304,800	-0.21%
fetch_frames/moq-lite-07-wip/publishers=4/viewers=16/watc... frames=256	48,979,000	48,251,000	-1.49%
fetch_frames/moq-transport-22/publishers=1/viewers=1/watc... frames=4	729,710	728,560	-0.16%
fetch_frames/moq-transport-22/publishers=1/viewers=1/watc... frames=256	8,724,500	8,646,400	-0.90%
fetch_frames/moq-transport-22/publishers=1/viewers=16/wat... frames=4	5,922,100	5,912,800	-0.16%
fetch_frames/moq-transport-22/publishers=1/viewers=16/wat... frames=256	72,073,000	71,396,000	-0.94%
fetch_frames/moq-transport-22/publishers=4/viewers=1/watc... frames=4	730,170	727,750	-0.33%
fetch_frames/moq-transport-22/publishers=4/viewers=1/watc... frames=256	8,723,800	8,609,700	-1.31%
fetch_frames/moq-transport-22/publishers=4/viewers=16/wat... frames=4	7,236,400	7,187,300	-0.68%
fetch_frames/moq-transport-22/publishers=4/viewers=16/wat... frames=256	85,397,000	84,694,000	-0.82%
fetch_range/moq-lite-06/publishers=1/viewers=1/burst=2/fetch	738,670	736,320	-0.32%
fetch_range/moq-lite-06/publishers=1/viewers=1/burst=16/fetch	5,932,300	5,913,400	-0.32%
fetch_range/moq-lite-06/publishers=1/viewers=16/burst=2/fetch	7,198,900	7,159,300	-0.55%
fetch_range/moq-lite-06/publishers=1/viewers=16/burst=16/fetch	52,468,000	52,320,000	-0.28%
fetch_range/moq-lite-06/publishers=16/viewers=1/burst=2/fetch	738,670	736,330	-0.32%
fetch_range/moq-lite-06/publishers=16/viewers=1/burst=16/fetch	5,932,400	5,913,200	-0.32%
fetch_range/moq-lite-06/publishers=16/viewers=16/burst=2/fetch	17,112,000	17,072,000	-0.23%
fetch_range/moq-lite-06/publishers=16/viewers=16/burst=16/fetch	132,220,000	131,890,000	-0.25%
fetch_range/moq-lite-07-wip/publishers=1/viewers=1/burst=2/fetch	804,800	802,710	-0.26%
fetch_range/moq-lite-07-wip/publishers=1/viewers=1/burst=16/fetch	6,459,300	6,445,500	-0.21%
fetch_range/moq-lite-07-wip/publishers=1/viewers=16/burst=2/fetch	7,732,400	7,713,700	-0.24%
fetch_range/moq-lite-07-wip/publishers=1/viewers=16/burst=16/fetch	55,930,000	55,874,000	-0.10%
fetch_range/moq-lite-07-wip/publishers=16/viewers=1/burst=2/fetch	804,730	802,840	-0.23%
fetch_range/moq-lite-07-wip/publishers=16/viewers=1/burst=16/fetch	6,458,800	6,445,200	-0.21%
fetch_range/moq-lite-07-wip/publishers=16/viewers=16/burst=2/fetch	18,178,000	18,155,000	-0.13%
fetch_range/moq-lite-07-wip/publishers=16/viewers=16/burst=16/fetch	140,030,000	139,710,000	-0.23%
fetch_range/moq-transport-22/publishers=1/viewers=1/burst=2/fetch	1,461,200	1,460,800	-0.03%
fetch_range/moq-transport-22/publishers=1/viewers=1/burst=16/fetch	11,667,000	11,698,000	+0.27%
fetch_range/moq-transport-22/publishers=1/viewers=16/burst=2/fetch	11,021,000	11,012,000	-0.08%
fetch_range/moq-transport-22/publishers=1/viewers=16/burst=16/fetch	82,741,000	82,613,000	-0.15%
fetch_range/moq-transport-22/publishers=16/viewers=1/burst=2/fetch	1,458,300	1,461,800	+0.24%
fetch_range/moq-transport-22/publishers=16/viewers=1/burst=16/fetch	11,710,000	11,693,000	-0.15%
fetch_range/moq-transport-22/publishers=16/viewers=16/burst=2/fetch	24,082,000	24,047,000	-0.15%
fetch_range/moq-transport-22/publishers=16/viewers=16/burst=16/fetch	187,150,000	187,100,000	-0.03%

Decisions

Settled 2026-11-01 by the maintainer.

1. What to do with the remaining residual. Lite FETCH keeps about 87k instructions per round (1.9%) of fix(net): preserve transport frame bounds #6922's cost, and batched writes about 53 instructions per frame. Neither has a single removable step.

   • ✅ (a) Accept it and delete the quest. What's left is a few instructions here and there across FrameContent's size and write_frames' code layout. Inlining experiments move it by as much as they save.
   • (b) Keep the rewritten quest in m2 for a smaller single-delivery FrameContent, with the canonical-only fields behind a lazily allocated part. That costs one more allocation per frame on groups with shared content.
   • (c) Keep the rewritten quest but move it to m3.

   So this PR deletes the quest (quest rm) instead of rewriting it to the (b)/(c) work as the earlier head did, and drops its entry from the perf epic's Required list. The quest's goal allowed this outcome: "or the measured residual is accepted and this quest deleted".

Choices made without asking

• Attaching yields instead of blocking. attach runs under the group's write lock, and waits for at most one in-flight write: a bounded copy that takes no lock the attacher holds. It loops on std::thread::yield_now(). Attaching happens once per content, and only waits if it lands mid-write.
• Kept the publication mutex for forks. Forks still lock, overlap-check, and admit through it, unchanged.
• Dropped the Entry owner-slot change (2.6 instructions per frame, not worth new unsafe). Padding base's Entry to main's size costs only 3.4 per frame, so the slot is not the batch residual.

Tests

• a_creator_alone_publishes_and_releases_without_attaching: the creator publishes and returns its reservation without attaching. Once attached, a fork shares the reservation, the creator keeps it until it ends, and a mismatching fork is still refused.
• a_delivery_attaching_mid_write_publishes_one_payload: a fork attaches on another thread while the creator streams, for fixed and growing storage. Both see one payload, stored once. It passed 300 repeated runs.
• admission_ends_with_active_producers_not_cached_views now starts from a creator admission, so it covers the hand-off of the creator's reservation.

Impact

• Public API: none.
• Wire: none.

Validation

• just check: clippy, fmt, doc, shear, sort, and 8845 of 9240 tests passed. The one failure, moq-uring::integration mtu::discovery_finds_a_larger_mtu, is the host's shared RLIMIT_MEMLOCK refusing io_uring memory (Cannot allocate memory), not this change.
• After merging main: cargo clippy -p moq-net --all-targets --all-features -D warnings and cargo nextest run -p moq-net --all-features (3222 passed).
• After merging main again (bf4e726f66): just rs check -p moq-net and cargo nextest run -p moq-net --all-features (3223 passed).
• After merging main again (2ca588bd7d, head 33d7e13329): the merge was clean, since moq-dev/moq#7509's per-storage reader owners take their own locks and leave the publication alone. just rs check -p moq-net --all-features and cargo nextest run -p moq-net --all-features (3311 passed, 13 skipped) passed. Miri (just rs miri) did not run: the host has no nightly toolchain, and CI runs it only in Nightly.
• quest check --base origin/main: PASS (554 documents).

Follow-ups

• One rebuild of a reused export produced a group bench binary whose counts matched neither source state, and fresh export paths were consistent. A paired A/B harness should build each revision at its own path, not just with its own target dir, which extends perf(net): stream a frame's single delivery without extra allocations #7141's note.

Review

Codex gpt-6.1-sol at medium effort (quest review --base origin/main), round 1 on head 33d7e13329: no findings.

The online reviews on 3c678f25dd (OpenAI and pullfrog) reported no issues, and no review threads are open.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[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 13 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: ab191c1e-9be3-4875-8989-e5913c17c981

Commits

Reviewing files that changed from the base of the PR and between 2ca588b and 33d7e13.

Files selected for processing: 4

• quest/m2/perf/README.md
• quest/m2/perf/single-delivery-write.md
• rs/moq-net/src/model/frame.rs
• rs/moq-net/src/model/group.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

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 3c678f25dd5b184655e09a3abd906ead23e04f42.

No actionable bugs found in the changed publication, attachment, reservation-release, and group commit paths. The creator/attacher store-load handshake preserves exclusive publication before switching permanently to the existing mutex path, and producer lifetime still governs reservation release rather than cached readers.

Direction: optimizing the common single-delivery case while retaining the existing shared-content path is reasonable. Keeping the mutex for forks is simpler than introducing another lock-free shared-writer mechanism; the documented decision to accept the remaining measured cost avoids expanding this into a larger ownership redesign. No public API or wire change identified.

Verification limits: static GitHub source and test inspection only; I did not run tests, concurrency instrumentation, or reproduce the benchmark measurements.

(Written by OpenAI)

[pullfrog]

✅ No new issues found.

Reviewed changes

This finishes the single-delivery-write quest by letting a frame's creating delivery publish without the publication lock until another party attaches, then deleting the quest.

• Lock-free creator write: write_chunk now guards a new write_alone (no mutex, no overlap check) with a writing/attached SeqCst store-load pair while the creating delivery is the only writer.
• FrameContent::attach: the single choke point under the publication lock that waits out any in-flight creator write and folds the creator's admission into Publication.producers, called from Shared::fork, identify, bill_content, and admit.
• Admission rework: Admission becomes Creator(Option<Reservation>) | Shared with an explicit FrameBuf::release, and FrameBuf::admitted takes the creator's reservation with the content so Producer::drop returns it without a refcount round trip.
• Group fast path: canonicalize is skipped when the group has no shared content, in both the batch write and commit paths.
• Quest removal: deletes quest/m2/perf/single-delivery-write.md and its README entry.

I traced the StoreLoad pairing (write_chunk/release versus attach) and the reservation handoff across admitted/attach/admit/release, including Shared::fork's fast path and the release-during-attach interleaving, and found no double-release, leak, or missed/spurious producer increment. Two independent reads reached the same conclusion. Public API and wire are unchanged.

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 33d7e13

No new actionable issue found in the sampled attachment and storage paths. Including main's new frame-owner storage, the creator/attacher handshake remains intact and storage-specific locking remains in place.

Static GitHub review only; I did not run tests, Miri or benchmarks. The earlier benchmark predates the merged storage change.

[pullfrog]

✅ No new issues found.

Reviewed changes

This run re-reviewed the delta since the prior Pullfrog review, which is the merge of origin/main into the branch. The PR's own lock-free creator-write logic is unchanged.

• Merged origin/main (2ca588bd7): integrated the per-storage reader-owner refactor (FrameStorage::Fixed { segment, shared }, Open, share, and Filled over MaybeUninit segments) and the group Pace machinery onto the branch.
• No conflict-induced change to FrameContent::attach, Admission::Creator | Shared, FrameBuf::admitted/release, write_alone, or the group canonicalize fast path.

The new integration surface the prior review could not see is main's reader owners. I verified they do not race the creator's unlocked write to a Fixed segment: share takes a Filled owner's length only from an Acquire load of written, and the creator publishes that with a Release store only after copying, so a reader never builds an owner covering uninitialized bytes. The missing shared lock on the writer is sound because writes are append-only and written is monotonic. An independent read reached the same conclusion. Public API and wire are unchanged.

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