From d8a77855dac94c24248d6d4e8912ce8ab6c06dcd Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 29 Oct 2026 09:44:47 -0700 Subject: [PATCH 1/4] chore(quest): claim quest/m0/nightly/bench-regressions.md Quest-Claim: 0ad8547a79796bdf From 427cd4a9ba13d41f4f67fd2d705ce02c6d3f2560 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 29 Oct 2026 09:54:03 -0700 Subject: [PATCH 2/4] docs(quest): checkpoint paired regression evidence Co-Authored-By: GPT-6.1 Sol --- quest/m0/nightly/bench-regressions.md | 64 +++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 9 deletions(-) diff --git a/quest/m0/nightly/bench-regressions.md b/quest/m0/nightly/bench-regressions.md index fa4b6ec4a4..94d338f981 100644 --- a/quest/m0/nightly/bench-regressions.md +++ b/quest/m0/nightly/bench-regressions.md @@ -1,4 +1,4 @@ -# [M] Paired bench regressions bisected +# [M] Paired bench regressions attributed ## Goal @@ -8,14 +8,60 @@ 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. +Nightly [40506938673](https://github.com/moq-dev/moq/actions/runs/40506938673) +compared `22808bbb2f4aab9fdaf9a60507f77efbaef4bd0d` against +`8810d736518185eb813dc8758f8f95f31d50cc9e`. Its +[bench-rust artifact](https://github.com/moq-dev/moq/actions/runs/40506938673/artifacts/12739050694) +contains the estimates, confirmation rounds, and paired verdicts. The five +confirmed cases and their three confirmation deltas, in round order, are: + +| Case | Round 1 | Round 2 | Round 3 | Median | +| --- | --- | --- | --- | --- | +| `retirement/selection/8x32` | +24.67% | +22.58% | +23.35% | +23.35% | +| `retirement/selection/32x32` | +13.31% | +20.86% | +22.55% | +20.86% | +| `retirement/selection/128x32` | +17.92% | +26.79% | +23.81% | +23.81% | +| `server_admission/workers=4/0` | +10.17% | +14.39% | +8.39% | +10.17% | +| `track_range_demand/1000r/1000` | +16.25% | +7.87% | +10.23% | +10.23% | + +The original runner used aarch64 Linux with four allowed CPUs, rustc 1.98.1, +and LLVM 22.1.8. These are timing observations, not yet production attributions. +The newer [bench CI evidence](/quest/m1/bench-ci.md) supersedes the earlier +instruction to treat selection's agreeing sizes as proof: repeated rounds +reuse the same binaries, so layout or LTO changes can produce an agreeing +slowdown without extra production work. Keep thresholds and behavior intact. + +Source inspection on 2026-10-29 narrowed the candidates, without measuring them: + +- Admission's `Activity`, the admission benchmark, and all of `rs/kio` are + unchanged across the original pair. Its four-worker contention requires + paired timing and A/A controls; instruction counts alone cannot attribute + scheduling or cache-line contention. +- [moq-dev/moq#6816](https://github.com/moq-dev/moq/pull/6816) added selection's + answering lease so an unanswered epochless request is refused on replacement + but survives a withdrawal. Count the replacement operation separately from + fixture setup. Also account for `fuzz::RequestValidity::start` retaining only + the current lease while production retains both leases: the benchmark can + deallocate the answering channel during replacement instead of holding it + until the request ends. This is a candidate measurement difference, not a + demonstrated explanation for the ARM slowdown. +- Range-demand aggregation itself is unchanged. + [moq-dev/moq#6815](https://github.com/moq-dev/moq/pull/6815) added the range + refusal path to `Subscriber::update`. It reads the old preferences, constructs + a consumer, and resolves/locks track state even for absolute group ranges, + which need no cached anchor. Measure that path before deciding whether to + move the wire-range check ahead of state lookup. Preserve unresolved-anchor + refusal, covered by `draft22_resolves_cached_time_ends`. + +Remaining work: reproduce these exact cases with separate target directories +per revision, three alternating paired rounds, same-binary A/A controls, and +instruction counts isolated to timed production operations. Attribute any +extra work before fixing it; equal counts alone do not prove timing noise. +Use the existing reader/range and route/consumer sweeps to verify scaling. +[moq-dev/moq#7253](https://github.com/moq-dev/moq/pull/7253) is an unfinished +checkpoint: both isolated builds waited at `quest gate` without reaching Cargo, +so it contains no new timings, instruction counts, or validated code fix. +Agents never dispatch Nightly. Bench-ci owns calibration; this quest owns the +five cases above. Public API: none. Wire: none. From 4cd1f2cfcf68651eb0bb2ec4d02490f44cf95aa4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 29 Oct 2026 09:56:30 -0700 Subject: [PATCH 3/4] docs(quest): record local gate limitation Co-Authored-By: GPT-6.1 Sol --- quest/m0/nightly/bench-regressions.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/quest/m0/nightly/bench-regressions.md b/quest/m0/nightly/bench-regressions.md index 94d338f981..eba54c2a94 100644 --- a/quest/m0/nightly/bench-regressions.md +++ b/quest/m0/nightly/bench-regressions.md @@ -60,6 +60,9 @@ Use the existing reader/range and route/consumer sweeps to verify scaling. [moq-dev/moq#7253](https://github.com/moq-dev/moq/pull/7253) is an unfinished checkpoint: both isolated builds waited at `quest gate` without reaching Cargo, so it contains no new timings, instruction counts, or validated code fix. +`quest check` and the Markdown check passed. `just ci` stopped in the unchanged +Clippy-gate tooling test because `mbx` could not start Cargo (`Argument list too +long`, OS error 7); the full local gate remains outstanding. Agents never dispatch Nightly. Bench-ci owns calibration; this quest owns the five cases above. From 91231371cfc3b66490b1d49dfde45af6077f1eb4 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Sat, 31 Oct 2026 20:20:24 -0700 Subject: [PATCH 4/4] perf(net): skip refusal for group ranges; attribute the paired regressions Only an unresolved time anchor is ever refused, so a range change without one no longer builds a consumer and locks the track state. This recovers track_range_demand's +15.5% from moq-dev/moq#6815 (-12.4% on x86). The selection benchmark's helper dropped the captured answering lease, so each replacement freed it inside the timed operation, unlike production. It now retains both leases (-8.4% instructions at 8x32). The remaining cost of moq-dev/moq#6816's answering lease is inherent and stays. server_admission runs identical machine code on both builds, recorded in bench-ci. Every case is attributed, so the quest is deleted. Co-Authored-By: Claude Opus 5.5 --- quest/m0/nightly/README.md | 1 - quest/m0/nightly/bench-regressions.md | 73 --------------------------- quest/m1/bench-ci.md | 4 ++ rs/moq-net/benches/retirement.rs | 2 +- rs/moq-net/src/fuzz.rs | 28 +++++++--- rs/moq-net/src/model/track.rs | 13 +++-- 6 files changed, 34 insertions(+), 87 deletions(-) delete mode 100644 quest/m0/nightly/bench-regressions.md diff --git a/quest/m0/nightly/README.md b/quest/m0/nightly/README.md index f74e2ae04a..b7e5c549d9 100644 --- a/quest/m0/nightly/README.md +++ b/quest/m0/nightly/README.md @@ -38,7 +38,6 @@ Public API: none. Wire: none. - [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 - [Probing beside Cubic](/quest/m0/nightly/competing-cubic.md) - the `competing_cubic` probe test stays within its deadline and p99 budgets - [OBS TSan passes without a wall-clock bound](/quest/m0/nightly/obs-tsan.md) - `destroySource` asserts ordering instead of `elapsed < 500ms` -- [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 - [moq-tokio's isolated feature builds pass](/quest/m0/nightly/tokio-features.md) - `Incoming::refuse` stops failing `tokio-features --all` on dead_code, so the rest of `rs features` runs - [moq-uring's tests build on macOS and Windows](/quest/m0/nightly/kio-e0433.md) - the platform jobs stop failing with E0433 on `kio` in moq-uring's integration tests diff --git a/quest/m0/nightly/bench-regressions.md b/quest/m0/nightly/bench-regressions.md deleted file mode 100644 index eba54c2a94..0000000000 --- a/quest/m0/nightly/bench-regressions.md +++ /dev/null @@ -1,73 +0,0 @@ -# [M] Paired bench regressions attributed - -## 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) -compared `22808bbb2f4aab9fdaf9a60507f77efbaef4bd0d` against -`8810d736518185eb813dc8758f8f95f31d50cc9e`. Its -[bench-rust artifact](https://github.com/moq-dev/moq/actions/runs/40506938673/artifacts/12739050694) -contains the estimates, confirmation rounds, and paired verdicts. The five -confirmed cases and their three confirmation deltas, in round order, are: - -| Case | Round 1 | Round 2 | Round 3 | Median | -| --- | --- | --- | --- | --- | -| `retirement/selection/8x32` | +24.67% | +22.58% | +23.35% | +23.35% | -| `retirement/selection/32x32` | +13.31% | +20.86% | +22.55% | +20.86% | -| `retirement/selection/128x32` | +17.92% | +26.79% | +23.81% | +23.81% | -| `server_admission/workers=4/0` | +10.17% | +14.39% | +8.39% | +10.17% | -| `track_range_demand/1000r/1000` | +16.25% | +7.87% | +10.23% | +10.23% | - -The original runner used aarch64 Linux with four allowed CPUs, rustc 1.98.1, -and LLVM 22.1.8. These are timing observations, not yet production attributions. -The newer [bench CI evidence](/quest/m1/bench-ci.md) supersedes the earlier -instruction to treat selection's agreeing sizes as proof: repeated rounds -reuse the same binaries, so layout or LTO changes can produce an agreeing -slowdown without extra production work. Keep thresholds and behavior intact. - -Source inspection on 2026-10-29 narrowed the candidates, without measuring them: - -- Admission's `Activity`, the admission benchmark, and all of `rs/kio` are - unchanged across the original pair. Its four-worker contention requires - paired timing and A/A controls; instruction counts alone cannot attribute - scheduling or cache-line contention. -- [moq-dev/moq#6816](https://github.com/moq-dev/moq/pull/6816) added selection's - answering lease so an unanswered epochless request is refused on replacement - but survives a withdrawal. Count the replacement operation separately from - fixture setup. Also account for `fuzz::RequestValidity::start` retaining only - the current lease while production retains both leases: the benchmark can - deallocate the answering channel during replacement instead of holding it - until the request ends. This is a candidate measurement difference, not a - demonstrated explanation for the ARM slowdown. -- Range-demand aggregation itself is unchanged. - [moq-dev/moq#6815](https://github.com/moq-dev/moq/pull/6815) added the range - refusal path to `Subscriber::update`. It reads the old preferences, constructs - a consumer, and resolves/locks track state even for absolute group ranges, - which need no cached anchor. Measure that path before deciding whether to - move the wire-range check ahead of state lookup. Preserve unresolved-anchor - refusal, covered by `draft22_resolves_cached_time_ends`. - -Remaining work: reproduce these exact cases with separate target directories -per revision, three alternating paired rounds, same-binary A/A controls, and -instruction counts isolated to timed production operations. Attribute any -extra work before fixing it; equal counts alone do not prove timing noise. -Use the existing reader/range and route/consumer sweeps to verify scaling. -[moq-dev/moq#7253](https://github.com/moq-dev/moq/pull/7253) is an unfinished -checkpoint: both isolated builds waited at `quest gate` without reaching Cargo, -so it contains no new timings, instruction counts, or validated code fix. -`quest check` and the Markdown check passed. `just ci` stopped in the unchanged -Clippy-gate tooling test because `mbx` could not start Cargo (`Argument list too -long`, OS error 7); the full local gate remains outstanding. -Agents never dispatch Nightly. Bench-ci owns calibration; this quest owns the -five cases above. - -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 052d2d938c..d72d1e0b2b 100644 --- a/rs/moq-net/src/fuzz.rs +++ b/rs/moq-net/src/fuzz.rs @@ -1670,20 +1670,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 b17660de93..7558d99d31 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -4661,6 +4661,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); @@ -5229,7 +5232,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 { @@ -5256,7 +5261,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) @@ -7146,7 +7151,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)?; @@ -7839,7 +7844,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)?;