Skip to content

Split OriginConsumer into cheap read handle and announcement cursor#1434

Merged
kixelated merged 1 commit into
devfrom
claude/originconsumer-performance-jKMzS
May 28, 2026
Merged

Split OriginConsumer into cheap read handle and announcement cursor#1434
kixelated merged 1 commit into
devfrom
claude/originconsumer-performance-jKMzS

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

Refactor the announcement subscription API to separate concerns: OriginConsumer is now a cheap, cloneable read handle over the broadcast tree, while AnnounceConsumer is the stateful cursor that receives announcement events. This eliminates the expensive per-clone channel allocation and allows multiple independent announcement streams from the same consumer.

Key Changes

  • OriginConsumer refactored to be cheap to clone: Removed the per-instance mpsc::UnboundedReceiver and ConsumerId. Cloning now shares the underlying tree state without allocating any per-cursor resources.

  • New AnnounceConsumer type: Extracted the announcement stream logic into a separate struct that holds the channel receiver and consumer ID. Created via OriginConsumer::announced() or AnnounceProducer::consume().

  • New AnnounceProducer type: Symmetric counterpart to AnnounceConsumer. Provides a cheap read handle to the announcement stream for a subtree, similar to how OriginConsumer works for the broadcast tree.

  • API method renames:

    • OriginConsumer::announced() now returns AnnounceConsumer (was async method returning Option<OriginAnnounce>)
    • OriginConsumer::try_announced()AnnounceConsumer::try_next()
    • OriginConsumer::announced() (async) → AnnounceConsumer::next() (async)
    • Added AnnounceConsumer::is_closed() to check cursor state
  • OriginProducer::announces() method: New method to get an AnnounceProducer for the subtree, providing symmetric API to consume().

  • Updated internal struct: Renamed OriginConsumerNotify to AnnounceConsumerNotify for clarity.

  • Documentation improvements: Updated doc comments to clarify the cheap-to-clone nature of OriginConsumer and the allocation semantics of AnnounceConsumer.

Implementation Details

  • OriginConsumer now derives Clone and implements it directly (no custom impl needed).
  • AnnounceConsumer maintains the original per-cursor state: channel receiver, consumer ID, and root path.
  • Both types support the same root path stripping and absolute path conversion utilities.
  • Test helpers (assert_next, assert_try_next, etc.) moved to AnnounceConsumer impl block.
  • All call sites updated to call .announced() on OriginConsumer to get an AnnounceConsumer before awaiting announcements.
  • Added comprehensive test test_consumer_clone_is_side_effect_free to verify that cloning OriginConsumer does not drain announcement channels and that fresh AnnounceConsumer instances still receive the active backlog.

https://claude.ai/code/session_01BSuKtGPPntcBMEQiGb3Pn6

Comment thread rs/libmoq/src/origin.rs Outdated
@kixelated kixelated marked this pull request as ready for review May 21, 2026 17:09
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1b8346c7-93c1-490a-a32b-de5a78fe6c1e

📥 Commits

Reviewing files that changed from the base of the PR and between 9021565 and 106d480.

📒 Files selected for processing (12)
  • rs/hang/examples/subscribe.rs
  • rs/libmoq/src/origin.rs
  • rs/moq-boy/src/input.rs
  • rs/moq-boy/src/main.rs
  • rs/moq-clock/src/main.rs
  • rs/moq-ffi/src/origin.rs
  • rs/moq-native/tests/backend.rs
  • rs/moq-native/tests/broadcast.rs
  • rs/moq-net/src/ietf/publisher.rs
  • rs/moq-net/src/lite/publisher.rs
  • rs/moq-net/src/model/origin.rs
  • rs/moq-relay/src/web.rs

Walkthrough

This PR refactors the origin announcement streaming API by separating the cheap read handle (OriginConsumer) from the event cursor (AnnounceConsumer). OriginConsumer::announced() changes from an async method that drains announcements to a synchronous factory returning an AnnounceConsumer cursor. Internal notification plumbing switches to AnnounceConsumerNotify. All code that iterates announcements is updated to create persistent cursors and consume them via next() or try_next() rather than re-awaiting on the origin consumer each iteration. Tests are updated to use the new cursor API throughout.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main refactoring: splitting OriginConsumer into two types with distinct responsibilities (cheap read handle vs. announcement cursor).
Description check ✅ Passed The description comprehensively explains the refactoring's rationale, key API changes, and implementation details, all directly related to the changeset across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/originconsumer-performance-jKMzS
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/originconsumer-performance-jKMzS

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@kixelated kixelated changed the base branch from main to dev May 28, 2026 18:57
@kixelated kixelated force-pushed the claude/originconsumer-performance-jKMzS branch from 106d480 to 9588bfd Compare May 28, 2026 19:12
@kixelated kixelated enabled auto-merge (squash) May 28, 2026 20:10
OriginConsumer used to bundle a tree-read handle with an mpsc cursor
and a registered ConsumerId. Every clone (and every transient
.consume() / .scope() / .with_root() chain) paid for a fresh channel
plus a consume_initial() walk that replayed every active broadcast as
backlog. Callers like get_broadcast, announced_broadcast, and the FFI
wrappers paid that cost just to look up one path or re-root a handle.

Split announcement subscription into AnnounceProducer and
AnnounceConsumer. OriginConsumer is now a cheap, Clone-friendly read
handle; call .announced() on it to allocate the channel and register
the cursor. AnnounceConsumer owns the Drop-side unregistration. The
deprecated OriginProducer::get_broadcast shim and its libmoq

Updates the lite/ietf publishers, FFI, libmoq, relay, clock, moq-boy,
hang examples, and integration tests to grab a cursor explicitly
(consumer.announced() -> AnnounceConsumer::next/try_next). Adds a
regression test that cloning OriginConsumer does not drain another
cursor's channel and that a freshly-built AnnounceConsumer still
receives the active backlog.

https://claude.ai/code/session_01BSuKtGPPntcBMEQiGb3Pn6
@kixelated kixelated force-pushed the claude/originconsumer-performance-jKMzS branch from 9588bfd to f2b8dd9 Compare May 28, 2026 20:15
@kixelated kixelated merged commit 3eb8203 into dev May 28, 2026
2 checks passed
@kixelated kixelated deleted the claude/originconsumer-performance-jKMzS branch May 28, 2026 20:42
kixelated added a commit that referenced this pull request May 28, 2026
Rebasing onto dev picked up #1495 (moq-mux::Error is thiserror, not
anyhow) and #1434 (OriginConsumer lost announced_broadcast, gained
get_broadcast/announced). Track both in our error enum and binary;
the WHIP-client path is gated on the per-codec re-packetizer anyway,
so first-miss is good enough until that lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kixelated pushed a commit that referenced this pull request May 29, 2026
…sor API

broadcast.rs's linger_resubscribe_keeps_flowing_moq_lite_03 still awaited
OriginConsumer::announced() as a future, but #1434 made it return an
AnnounceConsumer cursor. The three sibling tests were updated; this one was
missed, leaving the workspace test build broken on dev. Mirror the sibling
pattern: build the cursor up front and poll it with .next().
kixelated added a commit that referenced this pull request May 30, 2026
Reconcile main into dev. Key conflict resolutions:

- conducer crate renamed to kio (main #1547): applied across all of dev's
  newer code; dropped the stale conducer path-dep, kept dev's new flate2 dep.
- moq-mux: kept dev's thiserror Result (#1495); dropped main's CatalogSource
  as dead code since dev's catalog::Consumer already unifies Hang/MSF.
- moq-net: kept dev's OriginConsumer/AnnounceConsumer split (#1434) and the
  TrackConsumer end_at cap; kept dev's non-optional auto-created origins on the
  lite session/publisher (#e770).
- stats: combined main's StatsConfig + liveness retention (#1537, #1548) with
  dev's AnnounceConsumer usage.
- libmoq + moq-native: kept main's auto-reconnect (#1544), terminal-callback
  contract (#1546), and consume_announced (#1552), adapted to dev's
  AnnounceConsumer and OriginProducer connect API. Restored the InitFailed
  error variant and made moq-rtc handle the now-fallible Log::init.

cargo check/clippy/test all pass on the merged workspace.
kixelated added a commit that referenced this pull request Jun 1, 2026
Reconciles the PR with dev's landed work:

* `moq-net: runtime Timescale/Timestamp; container::Frame keeps source
  scale (#1473)` — adopts dev's canonical `Timescale(NonZero<u64>)` and
  `Timestamp` design. The PR keeps `Track.timescale: Option<Timescale>`,
  `Frame.timestamp: Option<Timestamp>`, and `SubscribeOk.timescale` for
  the Lite05 wire format on top.
* `moq-mux: catalog filter/target and Annex-B exporters (#1487)` and the
  surrounding mux reorganization (codec/* + container/{fmp4,mkv,loc}
  split). The PR's per-file Lite05 additions to these layers are dropped
  in favor of dev's structure; per-frame timestamp encoding stays opt-in
  at the moq-net layer until the mux gets explicit timescale support.
* `Split OriginConsumer into cheap read handle and announcement cursor
  (#1434)` and other broadcast/origin updates land via dev.
* `moq-net: add Lite05Wip version variant (unadvertised) (#1518)` — the
  PR's wire format additions now hang off dev's `Lite05Wip` name (was
  `Lite05` in the PR), keeping a single ALPN identifier (`moq-lite-05-wip`)
  across both sides of the merge.
* Lite05 publisher logic and Lite05 subscriber timestamp decoding kept
  from the PR. `SubscribeOk.timescale: Option<Timescale>` carries the
  negotiated scale on Lite05+; older versions decode it as `None`.
* `subscribe_track` reverts to dev's sync `(&Track)` signature. The PR's
  async `Subscription`-based variant is dropped here because it cascaded
  async through every mux exporter and tipped over dev's poll-driven
  exporters. Timescale negotiation still works: the publisher carries
  it on the `Track` it creates, and Lite05 subscribers stash the value
  from `SubscribeOk` on the `TrackEntry` before frame decoding begins.
* `lite05_timestamp_roundtrip` integration test ported to dev's API and
  passes over WebTransport (negative delta frames included).

JS net (`js/net`) is still at Lite04, so the Lite05 wire field doesn't
need a JS-side update in this merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants