diff --git a/quest/m0/nightly/README.md b/quest/m0/nightly/README.md index b678433dca..81531aefad 100644 --- a/quest/m0/nightly/README.md +++ b/quest/m0/nightly/README.md @@ -36,5 +36,4 @@ Public API: none. Wire: none. ## Required - [The browser backend baseline completes on IETF22](/quest/m0/nightly/browser-baseline.md) - the WASM and UniFFI backend baseline completes every supported transport case without an internal error -- [Paired bench regressions bisected](/quest/m0/nightly/bench-regressions.md) - retirement/selection's +21-24% and two 10% rows fixed or shown to be noise - [Firefox's player closes every VideoFrame](/quest/m0/nightly/firefox-videoframe.md) - the Media job's Firefox player stops leaking an unclosed VideoFrame diff --git a/quest/m0/nightly/bench-regressions.md b/quest/m0/nightly/bench-regressions.md deleted file mode 100644 index fa4b6ec4a4..0000000000 --- a/quest/m0/nightly/bench-regressions.md +++ /dev/null @@ -1,24 +0,0 @@ -# [M] Paired bench regressions bisected - -## Goal - -Each regression the paired Rust comparison confirmed on 2026-10-27 is fixed at -its cause or shown to be noise by an A/A rerun, and Nightly's Rust bench -targets job passes. - -## Plan - -Nightly [40506938673](https://github.com/moq-dev/moq/actions/runs/40506938673) (2026-10-27, at 22808bbb2f) confirmed five regressions: -retirement/selection at 8x32, 32x32, and 128x32 (+21 to 24%), -server_admission/workers=4 (+10%), and track_range_demand/1000r (+10%). -The retirement/selection rows agree across three sizes, so treat them as -real: bisect with the paired base across the previous night's commits. The two -10% rows sit at the threshold that [bench CI](/quest/m1/bench-ci.md) has yet -to calibrate; rerun them before bisecting. bench-ci owns the threshold, this -quest owns the regressions. - -Public API: none. Wire: none. - -## Related - -- [Calibrate the paired nightly benchmarks](/quest/m1/bench-ci.md) - the threshold these crossed diff --git a/quest/m1/bench-ci.md b/quest/m1/bench-ci.md index 9f41bec2ff..fd50a8e047 100644 --- a/quest/m1/bench-ci.md +++ b/quest/m1/bench-ci.md @@ -89,6 +89,10 @@ poll on both builds. A paired round reruns the same two binaries, so a code or heap layout shift between them reads as a stable regression that rounds cannot filter. `codec_messages/*/lite` (+31%, +68%) moved only because a new bench group in the same binary changed what LTO inlines. +[moq-dev/moq#7253](https://github.com/moq-dev/moq/pull/7253) found a third: +`server_admission/workers=4/0` (+10%) runs machine code identical apart from +addresses on both builds of the 2026-10-27 pair. The other four 2026-10-27 cases +were real, and were fixed or accepted there. ## Related diff --git a/rs/moq-net/benches/retirement.rs b/rs/moq-net/benches/retirement.rs index caf3427f0a..9981821d53 100644 --- a/rs/moq-net/benches/retirement.rs +++ b/rs/moq-net/benches/retirement.rs @@ -30,7 +30,7 @@ fn retirement(c: &mut Criterion) { for (i, route) in routes.iter().enumerate() { for reader in 0..if i == 0 { consumers } else { 1 } { if selected { - let watch = RequestValidity::new( + let mut watch = RequestValidity::new( route, &Path::new(&format!("route/{i}/jobs/{reader}")).unwrap(), ); diff --git a/rs/moq-net/src/fuzz.rs b/rs/moq-net/src/fuzz.rs index a4010893d8..762cfbea50 100644 --- a/rs/moq-net/src/fuzz.rs +++ b/rs/moq-net/src/fuzz.rs @@ -1674,20 +1674,32 @@ impl crate::transport::Error for NoError { } /// A source-selection watch for the private request-lifetime benchmark. -pub struct RequestValidity(crate::model::broadcast::RequestValidity); +pub struct RequestValidity { + watch: crate::model::broadcast::RequestValidity, + /// Every period [`Self::start`] captured. A pending request holds its route and + /// answering leases until it ends, so a replacement closes both without freeing + /// either, and the benchmark must not charge it for freeing them. + periods: Vec, +} impl RequestValidity { /// Watch which instance this route's peer selects for a path. pub fn new(route: &crate::origin::Requests, path: &crate::Path) -> Self { - Self(crate::model::broadcast::RequestValidity::for_source(route, path)) + Self { + watch: crate::model::broadcast::RequestValidity::for_source(route, path), + periods: Vec::new(), + } } - /// Capture the lifetime of one pending request. - pub fn start(&self) -> kio::Consumer<()> { - match self.0.start() { - crate::model::broadcast::RequestValidity::Period(route, ..) => route, - _ => unreachable!("a source watch always captures a route lease"), - } + /// Capture the lifetime of one pending request, retaining its leases until this drops. + pub fn start(&mut self) -> kio::Consumer<()> { + let period = self.watch.start(); + let crate::model::broadcast::RequestValidity::Period(route, ..) = &period else { + unreachable!("a source watch always captures a route lease"); + }; + let route = route.clone(); + self.periods.push(period); + route } } diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 08f464c646..8be25636ef 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -4758,6 +4758,9 @@ impl Consumer { } /// Refuse a new range before it changes the demand shared by this route's readers. + /// + /// Only an unresolved time anchor is refused, so callers skip ranges without one + /// before building a consumer or locking the track's state. fn refuse_subscription(&self, subscription: &Subscription) -> Result<()> { if let Some(serving) = self.serving() { return serving.refuse_subscription(subscription); @@ -5326,7 +5329,9 @@ impl Subscribing { let info = ready!(self.state.poll(waiter, |state| state.poll_info())) .map_err(|e| e.abort.clone().unwrap_or(Error::Dropped))??; refuse_untimed(&info, &self.subscription.read())?; - Consumer::new(self.name.clone(), self.state.weak()).refuse_subscription(&self.subscription.read())?; + if self.subscription.read().has_time() { + Consumer::new(self.name.clone(), self.state.weak()).refuse_subscription(&self.subscription.read())?; + } let resume = self.state.read().routes.clone(); let inner = match resume { @@ -5353,7 +5358,7 @@ impl Subscribing { /// Returns [`Error::Closed`] if the track already ended; the update is /// meaningless at that point and can usually be ignored. pub fn update(&mut self, subscription: Subscription) -> Result<()> { - if subscription.ranges != self.subscription.read().ranges { + if subscription.has_time() && subscription.ranges != self.subscription.read().ranges { Consumer::new(self.name.clone(), self.state.weak()).refuse_subscription(&subscription)?; } self.subscription.update(subscription) @@ -7235,7 +7240,7 @@ impl Control { if !self.timed && subscription.has_time() { return Err(Error::TimestampMismatch); } - if subscription.ranges != self.subscription.read().ranges { + if subscription.has_time() && subscription.ranges != self.subscription.read().ranges { self.track.refuse_subscription(&subscription)?; } self.subscription.update(subscription)?; @@ -7925,7 +7930,7 @@ impl Subscriber { Inner::Plain(cursor) => &cursor.subscription, Inner::Resume(resume, _) => resume.subscription(), }; - if subscription.ranges != channel.read().ranges { + if subscription.has_time() && subscription.ranges != channel.read().ranges { self.consume().refuse_subscription(&subscription)?; } channel.update(subscription)?;