diff --git a/quest/m1/README.md b/quest/m1/README.md index 63ad9130f00..51dc008d7ba 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -137,7 +137,6 @@ per-frame CPU growth since #5926. - [Vendored code has every upstream fix it needs](/quest/m1/vendor-sync/README.md) - quinn, libv4l-rs, nvidia-video-codec-sdk, rust-media-libs, cros-codecs, and UniFFI generator fixes are ported or planned, once per release cycle - [fMP4 refusals keep framing](/quest/m1/fmp4-refusal-framing.md) - a box refused at its header stops dropping the rest of its call's input, and the interleaved import benchmark is re-measured - [MKV leaf element limits](/quest/m1/mkv-leaf-limits.md) - `CodecID`, `DocType`, and `CodecPrivate` are refused past per-element size limits instead of read whole -- [Delete the tests that don't earn their cost](/quest/m1/prune-tests.md) - an off-cycle test cost sweep deletes tests whose cost outweighs their signal and makes the rest cheaper - [Remove the dependencies that don't earn their weight](/quest/m1/prune-deps.md) - Rust, JS, and vendored dependencies that are trivial, duplicated, or serve features not worth keeping are removed - [Remove what dominates build time](/quest/m1/prune-build-time.md) - the crates, features, and code that cost the most build time are removed or made cheaper - [Cluster routing](/quest/m1/cluster-routing/README.md) - prove shared routing contracts, implement per-origin routes, and connect outbound local clusters diff --git a/quest/m1/prune-tests.md b/quest/m1/prune-tests.md deleted file mode 100644 index 72d89a05887..00000000000 --- a/quest/m1/prune-tests.md +++ /dev/null @@ -1,34 +0,0 @@ -# [M] Delete the tests that don't earn their cost - -## Goal - -Every test in the merge gate and in nightly is worth its cost. A test whose -wall time, CPU, or flakiness outweighs the regression it alone catches is -deleted. A costly test that earns its keep is made cheaper: fast at its -source, or moved to nightly. CI workflows and jobs belong to -[CI workflow audit](/quest/m2/audit-ci.md). - -## Plan - -This is a one-off, off-cycle run of the [test cost audit](/quest/m2/audit-test-cost.md), -ahead of the cut. The recurring audit keeps its schedule, and this quest -creates no successor. Its PR rewrites that audit's baselines and its "check -first" list with what this run learned. - -Decided 2026-11-01 (maintainer): - -- Default to deleting. A test goes when its cost outweighs its signal, even - if nothing else covers it. Only a test that earns its keep is refactored to - be cheaper. -- Delete inline in this quest's PR, one commit per area. The PR body lists - each deletion with its measured cost and why it wasn't worth it, so review - catches a wrong call. Removing a public API or a user-facing feature is a - follow-up through `/quest-plan`. - -Start from the m2 audit's baselines and measurement pitfalls. Rank tests by -the slot time they take in a full-selection queue run and in Nightly, and -check Nightly's failure history for flakes. For each expensive test, name the -regression it alone would catch. If you can't, delete it. Watch for the same -behavior tested at several layers, such as a unit test, a session test, and -an end-to-end test of one property, and for tests that pin an old bug rather -than an edge case (AGENTS.md). A kept test still runs in CI, at least nightly. diff --git a/quest/m2/audit-test-cost.md b/quest/m2/audit-test-cost.md index c74950f51e9..29bdd125821 100644 --- a/quest/m2/audit-test-cost.md +++ b/quest/m2/audit-test-cost.md @@ -27,60 +27,97 @@ cost and why it wasn't worth it. Only what earns its keep is refactored to be cheaper instead. Removing a public API or a user-facing feature stays a follow-up. -Rank tests from nextest's lines in a full-selection queue log (`slow-timeout` -is 60 s under the `ci` profile). Flakes are tracked by hand in their own -quests, and `retries = 0` is deliberate. A test that sleeps on the wall clock -should mock time (AGENTS.md). A test whose cost outweighs the regression it -alone catches is deleted even when nothing else covers it. Repeat each local measurement: parallel agents load the host, so a -single run can be off by 2x. Pin to the queue VM's 8 CPUs with `taskset -c -0-7` when timing the whole suite. A test whose time differs between runs by -about 3 s is waiting out a QUIC draining period, not computing. +The off-cycle m1 pruning run (2026-11-01) refreshes this baseline in place; +it does not finish this recurring run, move its schedule, or create a successor. -What the m1 run (2026-10-21) fixed: the 900 s TS export past-limit failure the -jitter buffer's tests already cover (16.6 s), the close-code tests dialing -`localhost` against an IPv4-only server (3 s to 0.5 s each on the NixOS -runner, which lists `::1` first), and the split-accept driver tests' 333 ms -initial RTT (3 s to 0.2 s each). A benchmark-only edit now selects only its -own Rust crate or JS workspace; a moq-stats bench edit had run the relay and -CLI suites. To catch the `localhost` race on a host without it, run the -suite under an `/etc/hosts` that lists `::1` first (`unshare -rm`, then a -bind mount over `/etc/hosts`); only the close-code tests regressed. -[moq-dev/moq#6192](https://github.com/moq-dev/moq/pull/6192) landed alongside: -moq-auth's re-checks on the paused clock, moq-c's encoder probe, moq-gst's and -moq-relay's idle and retry waits, and moq-cli's export process tests moved to -nightly. +Measure nextest's per-test elapsed time as slot time, not CPU time. Sum only +one invocation: the full gate also runs feature-specific selections, and a +failed run may print the failed test twice. Keep the invocation's test count, +profile, features, thread count and completion state beside its timing. Repeat +local measurements under `quest gate`; concurrent work makes one sample +unreliable. Pin whole-suite comparisons to the same eight CPUs. A roughly +three-second swing can be QUIC draining rather than computation. The `ci` +profile marks a test slow at 60 seconds and uses zero retries. -Baselines from queue run -[39546261474](https://github.com/moq-dev/moq/actions/runs/39546261474), a full -selection on the 8-core runner VM: nextest ran 9,312 tests in 127 s. Their -summed time was 995 s, so the run is packed by slot time; the 48 tests over -5 s were 480 s of it. By crate: moq-cli 225 s, moq-relay 202 s, moq-mux 130 s, -moq-net 73 s, moq-tokio 59 s, moq-gst 56 s, moq-uring 41 s. +Current hosted baseline: + +- [2026-10-31 Nightly, gate job](https://github.com/moq-dev/moq/actions/runs/41204380716/job/124546373689): + the main nextest invocation passed 10,645 tests, skipped 115, and took + 196.586 seconds. Summed test slot time was 768.723 seconds; the 27 tests + above five seconds used 258.848 of it. By crate: moq-mux 193.621 seconds, + moq-net 142.320, moq-relay 80.813, moq-quic 48.398, moq-tokio 46.230, + moq-uring 42.835, and moq-cli 37.359. Other invocations in that job are + excluded from those sums. These hosted ARM64 timings replace the old + 9,312-test queue baseline for ranking, not for a same-machine speedup claim. +- [2026-11-01 Nightly, gate job](https://github.com/moq-dev/moq/actions/runs/41313466339/job/124868517727): + 9,163 of 10,891 tests ran in 249.218 seconds before the relay stack test + aborted. This is an incomplete selection, not a new whole-suite baseline. + [moq-dev/moq#7595](https://github.com/moq-dev/moq/pull/7595) has since + fixed the private configuration size without weakening that test. +- [2026-10-31 CLI export lane](https://github.com/moq-dev/moq/actions/runs/41204380716/job/124546373649): + 14 passed in 38.774 seconds, using 124.897 summed test seconds. The clean + and bufferbloated redundant-importer arms cost 18.669 and 26.278 seconds. + They exercise real process failover and impaired delivery. The route + identity change in [moq-dev/moq#7489](https://github.com/moq-dev/moq/pull/7489) + owns changes to that harness; do not duplicate them here. +- [2026-11-01 Rust benchmark-test lane](https://github.com/moq-dev/moq/actions/runs/41313466339/job/124868518054): + all 15 selected benchmark tests passed, using 114.177 summed seconds. + Receive-window sweeps used 80.370 seconds, raw audio 11.341. These measure + different transport, layout and fanout cases, rather than repeating one + behavior assertion. + +Local default-feature workspace selection on `21d6da9fc1`, pinned to CPUs +0-7 with 24 nextest threads and the `ci` profile: 10,623 tests ran in +50.433 seconds, using 997.332 summed seconds. One failed: +`backend::quic_client_close_drains_finished_track` with `Unroutable`, already +tracked by [moq-dev/moq#7590](https://github.com/moq-dev/moq/pull/7590). +This is not `just ci`'s device/runtime feature selection. Before the cut, the +99-test import/report selection passed; the deletion candidate cost 7.763 +seconds in the full run and 7.152 in that selection; do not attribute +whole-suite speed to it. + +The m1 cut ([moq-dev/moq#7600](https://github.com/moq-dev/moq/pull/7600)) +removed `report::import_reports_a_silent_video_pid` and its private +TS rewriting helpers. It cost 6.932 and 6.948 seconds in the two hosted gate +runs. The kept mid-import report test already checks the CLI-to-relay-to-report +path; `liveness_steps_over_a_corrupt_pcr_and_the_wrap` covers silence and +resumption directly, and the importer and stats tests cover stalled video +beside sparse data. The removed process test added a paced five-second clip, +two child processes, and another TS suppressor to repeat those properties. +The mid-import report and late-sink tests stay, as do the two stdin-EOF tests: +ongoing reports, cached final reports, catalog finish, and media announced only +at EOF are different process boundaries. Check first: -- Test threads. Pinned to 8 CPUs on a loaded host, the whole suite took - 105 to 113 s at 8 threads (about 185% CPU), 60 s at 16 (315%), 47 s at 24 - (420%), and 41 s at 32 (500%): the suite needs about 200 CPU-seconds, and - most slots wait on sockets, subprocesses, and wall-clock timeouts. - The maintainer selected 24 threads for the queue in - [CI housekeeping](/quest/m1/ci-housekeeping.md), which owns the loaded - proof before applying it. Only the known redundant-importer flake failed, - once, at 16 threads in this baseline. -- moq-gst's unanswered-dial close wait was fixed by - [moq-dev/moq#6201](https://github.com/moq-dev/moq/pull/6201): the close - stops counting an unanswered handshake as active immediately while the - closing period still answers late packets. Do not repeat the old request - to skip draining. [moq-dev/moq#6240](https://github.com/moq-dev/moq/pull/6240) - retired the silent-peer quest as complete. -- moq-uring: three endpoint tests wait out moq-quic's fixed 10 s handshake - idle timeout (30 s together). Neither moq-uring's `Transport` nor - `--quic-idle-timeout` sets it. #6201 does not change that timeout. The - [retirement](https://github.com/moq-dev/moq/pull/6240) settled keeping it - separate with no new knob; do not select another API in a cost audit. -- CPU-bound simulations in a debug build: five TS export timing cases (3 to - 11 s each) and the jitter buffer's day of drift (6 to 13 s), whose `Steer` - rebuilds the hull and median over its whole window on every step. +- Account for [moq-dev/moq#6987](https://github.com/moq-dev/moq/pull/6987) + before touching the TS export/jitter simulations, routing reference model, + JSON budgets, QUIC simulation payloads, or other CPU cuts it owns. Its + repeated cohort measurements are 61.02 and 67.49 CPU-seconds; neither is + this audit's saving. The hosted drift cases still cost 12 to 19 seconds + each before those changes land. +- [moq-dev/moq#7549](https://github.com/moq-dev/moq/pull/7549) owns the + refused-fill test and its scaling benchmark. Recovery fixture caching, + tracing isolation and stats oversize cuts have already landed; its body + identifies their owners. Do not repeat the old follow-up list. +- Compressed-cache bounds earn their cost: the long compressed group used + 22.150 and 42.928 hosted seconds, the large compressible batch 6.608 and + 10.849. They test stored-tail shedding while a plain reader lags, and + bounded inflation when tiny slices describe large frames. Smaller codec + round trips do not test either resource bound. Preserve those assertions. +- The three io_uring endpoint timeout/backlog tests used 30.284 seconds + together on October 31. Their unanswered handshake, pending admission and + completed-but-unaccepted connection cases are distinct. Keep them without + adding a timeout knob. [moq-dev/moq#7593](https://github.com/moq-dev/moq/pull/7593) + owns ring grouping and fixture startup diagnostics, not test deletion. +- Macro UI compile failures, ACME issuance/cache/rate-limit cases, and + GStreamer reconnects have distinct compiler, TLS, persistence and real + socket boundaries. Their elapsed cost alone is not evidence of redundancy. +- Classify failures before calling them flakes. October 30 and 31's feature + jobs stopped on an unused `moq-tokio::connect::Config::with_url` method; + November 1's gate and all-features run aborted on the relay stack, since + fixed. Neither is intermittent evidence against a test. Existing flake quests retain + their ownership; do not delete their tests to green the audit. ## Required diff --git a/rs/moq-cli/tests/integration/report.rs b/rs/moq-cli/tests/integration/report.rs index b625dd4cc51..53f5bd782fc 100644 --- a/rs/moq-cli/tests/integration/report.rs +++ b/rs/moq-cli/tests/integration/report.rs @@ -130,70 +130,6 @@ fn assert_clean(mpegts: &ts::stats::Snapshot) { } } -/// The PCR a packet's adaptation field carries, in 27 MHz ticks. -fn pcr(pkt: &[u8; 188]) -> Option { - if pkt[3] & 0x20 == 0 || pkt[4] < 7 || pkt[5] & 0x10 == 0 { - return None; - } - let base = (u64::from(pkt[6]) << 25) - | (u64::from(pkt[7]) << 17) - | (u64::from(pkt[8]) << 9) - | (u64::from(pkt[9]) << 1) - | (u64::from(pkt[10]) >> 7); - Some(base * 300 + ((u64::from(pkt[10] & 0x01) << 8) | u64::from(pkt[11]))) -} - -/// `clip` with the PES on `pid` suppressed from its first PES start at or after `from` seconds -/// of program clock to its first at or after `to`, as an encoder whose one input died behind a -/// running mux emits it: a packet that carried the PCR keeps it, every other becomes null -/// stuffing, and the counters after the gap are renumbered so continuity stays legal. The -/// moq-mux import tests' stimulus. -fn suppress(clip: &[u8], pid: u16, from: f64, to: f64) -> Vec { - let mut null = [0xff; 188]; - null[..4].copy_from_slice(&[0x47, 0x1f, 0xff, 0x10]); - - let mut out = Vec::with_capacity(clip.len()); - let (mut first, mut now) = (None, 0.0); - let (mut active, mut done) = (false, false); - let (mut last_cc, mut dropped) = (0, 0u8); - for pkt in clip.as_chunks::<188>().0 { - let mut pkt = *pkt; - if let Some(pcr) = pcr(&pkt) { - now = (pcr - *first.get_or_insert(pcr)) as f64 / 27e6; - } - if (u16::from(pkt[1] & 0x1f) << 8 | u16::from(pkt[2])) == pid { - if pkt[1] & 0x40 != 0 { - if !active && !done && now >= from { - active = true; - } else if active && now >= to { - (active, done) = (false, true); - } - } - let payload = pkt[3] & 0x10 != 0; - if active { - dropped = (dropped + u8::from(payload)) & 0x0f; - pkt = match pcr(&pkt) { - Some(_) => { - let mut clock = [0xff; 188]; - clock[..6].copy_from_slice(&[0x47, pkt[1] & 0x1f, pkt[2], 0x20 | last_cc, 183, 0x10]); - clock[6..12].copy_from_slice(&pkt[6..12]); - clock - } - None => null, - }; - } else { - pkt[3] = (pkt[3] & 0xf0) | (pkt[3].wrapping_sub(dropped) & 0x0f); - if payload { - last_cc = pkt[3] & 0x0f; - } - } - } - out.extend_from_slice(&pkt); - } - assert!(done, "the clip resumes the PID before it ends"); - out -} - /// Each PID's access units in `first` fell short of `last`'s: the rows advance while it runs. /// An SI row counts only the sections the program clock could not place, so it has none. fn assert_advancing(first: &ts::stats::Snapshot, last: &ts::stats::Snapshot) { @@ -270,73 +206,6 @@ async fn import_reports_count_what_each_rendition_wrote() { ); } -/// A video PID whose PES stops while the mux keeps its PCR and continuity legal: the import's -/// reports show that row's `quiet` growing while every other row keeps delivering, the row -/// counting again once it returns, and no TR 101 290 priority 1 check firing. -#[tokio::test] -async fn import_reports_a_silent_video_pid() { - let relay = relay().await; - let (_, clean) = written(CLIP); - let pid = |class| { - let mut rows = clean.streams.iter().filter(|(_, row)| row.class == class); - *rows.next().expect("the clip carries one").0 - }; - let (video, audio) = (pid(ts::stats::Class::Video), pid(ts::stats::Class::Audio)); - let stimulus = suppress(CLIP, video, 2.5, 4.5); - let (_, health) = written(&stimulus); - - let mut import = moq(&relay, "demo", &["import", "--report", "--linger", "2s", "ts"]) - .stdin(Stdio::piped()) - .spawn() - .expect("spawn the import"); - let mut stdin = import.stdin.take().expect("stdin"); - let (mut sink, mut lines) = sink(&relay, "demo", "report"); - - // Up to the silence, then a report, so the sink is reading before the video stops. - feed(&stimulus[..HALF], async |chunk| { - stdin.write_all(chunk).await.expect("write stdin") - }) - .await; - let mut reports: Vec> = - vec![next(&mut lines).await.expect("a report mid-import")]; - feed(&stimulus[HALF..], async |chunk| { - stdin.write_all(chunk).await.expect("write stdin") - }) - .await; - drop(stdin); - while let Some(report) = next(&mut lines).await { - reports.push(report); - } - assert!(wait(&mut import).await.success()); - assert!(wait(&mut sink).await.success()); - - let rows: Vec<&ts::stats::Snapshot> = reports.iter().filter_map(|report| report.ext.mpegts.as_ref()).collect(); - let quiet = |mpegts: &ts::stats::Snapshot, pid: u16| mpegts.streams[&pid].quiet.unwrap_or_default(); - let silent = rows.iter().map(|mpegts| quiet(mpegts, video)).max().unwrap(); - // Two seconds of silence, sampled about once a second. - assert!( - silent >= Duration::from_millis(500), - "the video row was quiet for {silent:?} at most" - ); - let busy = rows.iter().map(|mpegts| quiet(mpegts, audio)).max().unwrap(); - assert!( - busy < Duration::from_millis(500), - "the audio row was quiet for {busy:?}" - ); - - let last = rows.last().unwrap(); - assert_eq!(units(last), units(&health), "the last report holds every access unit"); - assert!(quiet(last, video) < silent, "the video row delivered again"); - let priority1 = [ - last.ts_sync_loss, - last.sync_byte_error, - last.pat_error, - last.continuity_count_error, - last.pmt_error, - ]; - assert_eq!(priority1, [0; 5], "the silence is legal on the wire"); -} - /// A sink that joins once the import has finished, while it lingers, still reads its final /// counts. #[tokio::test]