From 584861c7326a5a017dba80b8fc9c10991010dad0 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Wed, 26 Aug 2026 09:45:34 +1000 Subject: [PATCH] test(harness): report the CPU feature set, so a green run says what it tested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub's hosted Windows runners are a MIXED fleet for AVX-512, not a migrated one, so which kernels a job exercises is a draw — and nothing in the output said which draw it got. Several bundled plugins pick a code path from exactly those bits. That is how the CTMF AVX-512 crash reached main and then looked spontaneous: the nightly passed for three nights on non-AVX-512 runners, went red the night it drew one that had it (against a tree unchanged for four days), and two deliberate re-rolls afterwards both landed back on non-AVX-512 machines and passed vacuously. WorkerHarness's banner now carries a `cpu=` line: cpu=x86_64 [sse2 sse4.1 avx avx2 fma avx512f] It comes from the worker's own `--probe-cpu` rather than being derived Dart-side, so it reports what the actual pipeline process sees — including under emulation, where an x86_64 worker on Apple Silicon reports what Rosetta exposes rather than what it was compiled for. `describeCpu()` never throws: a diagnostic that fails a run would be worse than one saying "unknown". That means a broken probe degrades silently exactly when it matters, so both sides are pinned — worker_cpu_probe_test.dart in the push gate and a Rust unit test. The feature list must be non-empty, must contain sse2 on x86-64 (it is in the baseline, so its absence means a broken probe rather than a modest CPU), and must not claim AVX-512 without AVX2. Diagnostic only — nothing in the pipeline reads it. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 40 ++++++++++ app/test/support/worker_harness.dart | 32 +++++++- app/test/worker_cpu_probe_test.dart | 41 ++++++++++ worker/src/main.rs | 114 +++++++++++++++++++++++++++ 4 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 app/test/worker_cpu_probe_test.dart diff --git a/CLAUDE.md b/CLAUDE.md index c79abd2..5c9b9b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2222,6 +2222,46 @@ exception); both are in `licenses/NOTICES.txt`. It adds ~61 MB uncompressed per platform, but only about **21 MB to each deps zip** — the earlier "the zips roughly double" estimate was wrong, because it compared uncompressed size against compressed zips. + +### Every run states which hardware it tested + +`WorkerHarness`'s banner carries a `cpu=` line, from the worker's own +`--probe-cpu` (`{"arch":..., "features":[...]}`) rather than derived Dart-side, +so it reports what the actual pipeline process sees — including under emulation, +where an x86_64 worker on Apple Silicon reports what Rosetta exposes rather than +what the binary was compiled for. + +``` +WorkerHarness: platform=windows-x64 + ... + cpu=x86_64 [sse2 sse4.1 avx avx2 fma avx512f] +``` + +> **A green run on unknown hardware is not evidence.** GitHub's hosted Windows +> fleet is **mixed** for AVX-512 — not migrated — so which kernels a job +> exercises is a draw. That is how the CTMF crash (see "A plugin's own CPU +> auto-detect is not trustworthy") reached `main` and then looked spontaneous: +> the nightly passed for three nights on non-AVX-512 runners, went red the night +> it drew one that had it, and two deliberate re-rolls afterwards both landed +> back on non-AVX-512 machines and passed **vacuously**. Before reading a green +> Windows run as proof about anything CPU-dispatched, check this line. + +Two consequences worth keeping in mind: + +- **The durable guard for a dispatch bug is a script-generation assertion**, not + the heavy end-to-end test. `test_152` runs on every platform whatever hardware + it draws; the end-to-end CTMF case can only confirm opportunistically. +- **Local hardware can beat CI here.** The AVX-512 crash reproduces + deterministically on a dev machine that has AVX-512, which is a more + controlled experiment than re-rolling runners. + +`describeCpu()` never throws — a diagnostic that fails the run would be worse +than one that says "unknown" — so a broken probe would degrade silently exactly +when it matters. `app/test/worker_cpu_probe_test.dart` (push gate) and +`cpu_features_are_reported_and_plausible` (Rust) pin it from both sides: the +feature list must be non-empty, must contain `sse2` on x86-64 (it is in the +baseline, so its absence means a broken probe rather than a modest CPU), and +must not claim AVX-512 without AVX2. ### Linux builds on ubuntu-24.04, and that sets the glibc floor The Linux **deps** builds and the runners that test against them diff --git a/app/test/support/worker_harness.dart b/app/test/support/worker_harness.dart index cda41bb..3343f14 100644 --- a/app/test/support/worker_harness.dart +++ b/app/test/support/worker_harness.dart @@ -217,10 +217,40 @@ class WorkerHarness { // ignore: avoid_print print('WorkerHarness: platform=$platform\n' - ' deps=$_depsDir\n worker=$_workerPath\n input=$inputFile'); + ' deps=$_depsDir\n worker=$_workerPath\n input=$inputFile\n' + ' cpu=${await describeCpu()}'); _ready = true; } + /// The CPU architecture and dispatch-relevant instruction set extensions, + /// asked of the worker binary itself (`--probe-cpu`) rather than derived + /// here, so the answer is the one the actual pipeline process sees. + /// + /// Printed in the banner so a run **states which hardware it tested**. That + /// is not cosmetic: bundled plugins pick a kernel from these bits, and + /// GitHub's hosted Windows fleet is mixed for AVX-512 — so two green Windows + /// runs can have exercised different code. `ctmf.CTMF`'s AVX-512 kernel for + /// 8-bit input crashes the process, and the nightly passed for three days on + /// non-AVX-512 runners before drawing one that had it, at which point it + /// looked like a spontaneous failure against a four-day-old tree. + /// + /// Never fails the run: this is diagnostic, and a harness that refuses to + /// start because a probe misbehaved would be worse than one that says + /// "unknown". + static Future describeCpu() async { + try { + final result = await Process.run(_workerPath!, ['--probe-cpu']); + if (result.exitCode != 0) return 'unknown (probe exit ${result.exitCode})'; + final json = jsonDecode(result.stdout as String) as Map; + final features = (json['features'] as List).cast(); + return '${json['arch']} [${features.isEmpty ? 'none detected' : features.join(' ')}]'; + } catch (e) { + // An older worker predates --probe-cpu and exits non-zero on the unknown + // flag; that is reported above rather than thrown. + return 'unknown ($e)'; + } + } + // --------------------------------------------------------------------------- // Worker invocation // --------------------------------------------------------------------------- diff --git a/app/test/worker_cpu_probe_test.dart b/app/test/worker_cpu_probe_test.dart new file mode 100644 index 0000000..615408a --- /dev/null +++ b/app/test/worker_cpu_probe_test.dart @@ -0,0 +1,41 @@ +/// The harness prints the CPU feature set in its banner so a run states which +/// hardware it tested. That line degrades to "unknown" rather than throwing — +/// deliberately, since a diagnostic must never fail a run — which means a +/// broken probe would go unnoticed exactly when it matters. This pins it. +/// +/// Why it matters: bundled plugins select a kernel from these bits, and +/// GitHub's hosted Windows fleet is mixed for AVX-512. `ctmf.CTMF`'s AVX-512 +/// kernel for 8-bit input crashes the process, so a green Windows run means +/// nothing about that path unless the run says which hardware it drew. +library; + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +import 'support/worker_harness.dart'; + +void main() { + setUpAll(() async => WorkerHarness.ensureReady()); + + test('the worker reports a CPU feature set the harness can read', () async { + final described = await WorkerHarness.describeCpu(); + + expect( + described, + isNot(startsWith('unknown')), + reason: 'the probe degrades silently, so a broken --probe-cpu would ' + 'otherwise leave every run claiming unknown hardware', + ); + // " [ ...]" + expect(described, matches(RegExp(r'^\S+ \[.+\]$'))); + + if (Platform.version.contains('x64') || + described.startsWith('x86_64') || + described.startsWith('x86')) { + // SSE2 is part of the x86-64 baseline, so its absence means the probe is + // reporting nothing rather than reporting a modest CPU. + expect(described, contains('sse2')); + } + }); +} diff --git a/worker/src/main.rs b/worker/src/main.rs index 42e9a9e..13c7719 100644 --- a/worker/src/main.rs +++ b/worker/src/main.rs @@ -77,6 +77,17 @@ struct Args { /// when OpenCL-only options (knlmeanscl denoiser, QTGMC OpenCL) won't work. #[arg(long)] probe_opencl: bool, + + /// Probe mode: report this machine's CPU architecture and the instruction + /// set extensions relevant to plugin dispatch, as JSON, and exit. + /// + /// Exists because a green test run is otherwise silent about the hardware + /// it ran on, and several bundled plugins pick a code path from exactly + /// these bits. GitHub's hosted Windows fleet is mixed for AVX-512, so a + /// passing Windows job may or may not have exercised the AVX-512 kernels — + /// which is how the CTMF crash reached main and then looked spontaneous. + #[arg(long)] + probe_cpu: bool, } fn main() -> ExitCode { @@ -118,6 +129,10 @@ fn main() -> ExitCode { } // OpenCL probe mode: emit availability as JSON and exit (no progress stream). + if args.probe_cpu { + return run_probe_cpu(); + } + if args.probe_opencl { return run_probe_opencl(); } @@ -187,6 +202,71 @@ fn run_probe_opencl() -> ExitCode { ExitCode::SUCCESS } + +/// Report the CPU architecture and the dispatch-relevant instruction set +/// extensions as JSON. +/// +/// This is diagnostic, not a decision: nothing in the pipeline reads it. It +/// exists so a test run can *state* which hardware it tested, because several +/// bundled plugins select a code path from these bits and a green run is +/// otherwise silent about which path it took. +/// +/// The motivating case: `ctmf.CTMF`'s AVX-512 kernel for 8-bit input crashes +/// the process, and GitHub's hosted Windows runners are a mixed fleet — so the +/// nightly passed for days on non-AVX-512 machines, went red the night it drew +/// an AVX-512 one, and looked like a spontaneous failure against an unchanged +/// tree. Printing this next to the result turns "it passed" into "it passed on +/// this hardware". +fn run_probe_cpu() -> ExitCode { + let features = detected_cpu_features(); + println!( + "{}", + serde_json::json!({ + "arch": std::env::consts::ARCH, + "features": features, + }) + ); + ExitCode::SUCCESS +} + +/// The instruction set extensions that bundled plugins actually dispatch on. +/// +/// Deliberately a short list rather than everything detectable: these are the +/// ones that change which kernel a plugin runs here. Detection is a runtime +/// CPUID query, so it reports what the process can really execute — including +/// under emulation, where an x86_64 worker on Apple Silicon correctly reports +/// whatever Rosetta exposes rather than what the binary was compiled for. +#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +fn detected_cpu_features() -> Vec<&'static str> { + let mut features = Vec::new(); + for (name, present) in [ + ("sse2", std::is_x86_feature_detected!("sse2")), + ("sse4.1", std::is_x86_feature_detected!("sse4.1")), + ("avx", std::is_x86_feature_detected!("avx")), + ("avx2", std::is_x86_feature_detected!("avx2")), + ("fma", std::is_x86_feature_detected!("fma")), + ("avx512f", std::is_x86_feature_detected!("avx512f")), + ] { + if present { + features.push(name); + } + } + features +} + +#[cfg(target_arch = "aarch64")] +fn detected_cpu_features() -> Vec<&'static str> { + // NEON is architectural on aarch64, so it is reported unconditionally + // rather than probed. It is worth naming: it is why the ARM bundles prefer + // dubhater's nnedi3 over znedi3, whose SIMD kernels are x86-only. + vec!["neon"] +} + +#[cfg(not(any(target_arch = "x86", target_arch = "x86_64", target_arch = "aarch64")))] +fn detected_cpu_features() -> Vec<&'static str> { + Vec::new() +} + /// Run DVD info mode: enumerate titles and output JSON to stdout. fn run_dvd_info(dvd_path: &str) -> ExitCode { match dvd_reader::enumerate_titles(dvd_path) { @@ -629,3 +709,37 @@ fn run_subtitle_generation( } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cpu_features_are_reported_and_plausible() { + let features = detected_cpu_features(); + + // Nothing here is a decision, so the assertion is only that the probe + // reports *something* real — a probe that silently returns nothing + // would leave every CI run claiming it tested unknown hardware, which + // is the whole failure this exists to prevent. + #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] + assert!( + features.contains(&"sse2"), + "SSE2 is part of the x86-64 baseline, so its absence means the \ + probe is broken rather than the CPU being modest: {features:?}" + ); + + #[cfg(target_arch = "aarch64")] + assert!(features.contains(&"neon"), "{features:?}"); + + // AVX-512 implies AVX2 implies AVX: a set that breaks that ordering + // means the flags have been mis-wired to the wrong detections. + let has = |f: &str| features.contains(&f); + if has("avx512f") { + assert!(has("avx2"), "avx512f without avx2: {features:?}"); + } + if has("avx2") { + assert!(has("avx"), "avx2 without avx: {features:?}"); + } + } +}