From 4594ef9bb99dc7aca625fe0bee4bb4e54ad0a1ea Mon Sep 17 00:00:00 2001 From: plotarmordev <299844489+plotarmordev@users.noreply.github.com> Date: Tue, 20 Oct 2026 20:33:48 +0800 Subject: [PATCH 1/3] test(perf): keep SIGINT and envelope fixtures stable under load The drain-cancellation test raised a process-wide SIGINT. When the bounded run test installed its interrupt latch during that window, the signal reached the run latch instead: the process test waited for pipe EOF and the run test stopped before dispatch. Invoke the guard's handler directly; CLI tests still deliver real SIGINT/SIGTERM to the collector. The System One compare test assumed a 40 ms reference delay outlasts every candidate burst. Loaded hosts stretched candidate bursts past 1.4 s. Capture the candidate first and delay each reference request by its slowest retained burst, so the declared change holds by construction. --- crates/grill-perf/src/process.rs | 7 ++++--- crates/grill-perf/tests/support/systemone.rs | 20 ++++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/grill-perf/src/process.rs b/crates/grill-perf/src/process.rs index 6e23326..0409ca2 100644 --- a/crates/grill-perf/src/process.rs +++ b/crates/grill-perf/src/process.rs @@ -55,7 +55,7 @@ pub mod program_signals { static ACTIVE: Mutex<()> = Mutex::new(()); static CANCELLED: AtomicBool = AtomicBool::new(false); - extern "C" fn cancel(_: libc::c_int) { + pub(super) extern "C" fn cancel(_: libc::c_int) { CANCELLED.store(true, Ordering::SeqCst); } @@ -433,8 +433,9 @@ mod tests { if cancel { // The leader is already reaped, so cancellation must interrupt // normal-exit drainage rather than the running-program path. - // SAFETY: the collector guard has installed the SIGINT handler. - assert_eq!(unsafe { libc::raise(libc::SIGINT) }, 0); + // A raised SIGINT would reach whichever handler a concurrent + // test installed last, so invoke the guard's handler directly. + program_signals::cancel(libc::SIGINT); let completed = completion.recv_timeout(Duration::from_secs(5)).is_ok(); drop((stdout, stderr)); assert!(completed, "cancellation waited for pipe EOF"); diff --git a/crates/grill-perf/tests/support/systemone.rs b/crates/grill-perf/tests/support/systemone.rs index e2323c4..0460c58 100644 --- a/crates/grill-perf/tests/support/systemone.rs +++ b/crates/grill-perf/tests/support/systemone.rs @@ -404,20 +404,28 @@ fn inspect_detects_changed_retained_response_bytes() { #[test] fn compare_reports_a_declared_change_under_the_observed_envelope() { let temp = Temp::new(); - let slow = DecisionServer::new(|request| { - thread::sleep(Duration::from_millis(40)); - immediate(request) - }); let fast = DecisionServer::new(immediate); - // Hosted macOS runners oversleep the 40 ms fixture delay by up to ~150 ms, so a + // Hosted macOS runners oversleep the fixture delay by up to ~150 ms, so a // tight reference-spread bound tests the host timer, not the comparison. let mut wide = plan_json(); for gate in wide["gates"].as_array_mut().unwrap() { gate["max_reference_spread_bps"] = json!(1000000); } let plan = fixtures(&temp, "compare", &wide); - successful(&capture_server(&temp, &slow, &plan, 0, "a")); + // The candidate is captured first so every reference request can outlast + // its slowest burst, however long load stretched that burst. successful(&capture(&temp, &fast.endpoint, &plan, 1, "b")); + let slowest = (0..) + .map(|index| temp.path(&format!("b/wave-{index:06}.json"))) + .take_while(|path| path.exists()) + .map(|path| value(path)["makespan_us"].as_u64().unwrap()) + .max() + .unwrap(); + let slow = DecisionServer::new(move |request| { + thread::sleep(Duration::from_micros(slowest)); + immediate(request) + }); + successful(&capture_server(&temp, &slow, &plan, 0, "a")); successful(&capture_server(&temp, &slow, &plan, 2, "a2")); let output = cli() .args(["systemone", "compare", "--plan"]) From db0bb1df5d83940311e0df591f1ee2e277f1c1a0 Mon Sep 17 00:00:00 2001 From: plotarmordev <299844489+plotarmordev@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:31:29 +0800 Subject: [PATCH 2/3] test(perf): assert retained System One pooling envelopes --- CONTRIBUTING.md | 5 ++ crates/grill-perf/tests/support/systemone.rs | 65 +++++++++++++++++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f22a480..c2887c4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,6 +38,11 @@ run is `cargo test -p grill-perf --locked --test study`. For recipe-facing changes, use the [same baseline/check interface and reviewed PR summary](docs/performance/RECIPES.md#contributor-pr-report). Recipe facts are explicit data, not a reason to add a provider wrapper. +The System One comparison fixture derives its expected reference and candidate +envelopes from retained measured waves. Its reference delays follow the preceding +captures' observed makespans so reference pooling is checked without a fixed +timing margin. + ## Changes and review - Use a feature branch and submit a pull request. Describe the behavior changed, assumptions, and checks performed. diff --git a/crates/grill-perf/tests/support/systemone.rs b/crates/grill-perf/tests/support/systemone.rs index 0460c58..11cbabf 100644 --- a/crates/grill-perf/tests/support/systemone.rs +++ b/crates/grill-perf/tests/support/systemone.rs @@ -415,18 +415,33 @@ fn compare_reports_a_declared_change_under_the_observed_envelope() { // The candidate is captured first so every reference request can outlast // its slowest burst, however long load stretched that burst. successful(&capture(&temp, &fast.endpoint, &plan, 1, "b")); - let slowest = (0..) - .map(|index| temp.path(&format!("b/wave-{index:06}.json"))) - .take_while(|path| path.exists()) - .map(|path| value(path)["makespan_us"].as_u64().unwrap()) - .max() - .unwrap(); + let waves = |arm: &str| { + (0..) + .map(|index| temp.path(&format!("{arm}/wave-{index:06}.json"))) + .take_while(|path| path.exists()) + .map(value) + .collect::>() + }; + let slowest_burst = |waves: &[Value]| { + waves + .iter() + .map(|wave| wave["makespan_us"].as_u64().unwrap()) + .max() + .unwrap() + }; + let b = waves("b"); + let delay = Arc::new(std::sync::atomic::AtomicU64::new(slowest_burst(&b))); + let server_delay = delay.clone(); let slow = DecisionServer::new(move |request| { - thread::sleep(Duration::from_micros(slowest)); + thread::sleep(Duration::from_micros(server_delay.load(Ordering::SeqCst))); immediate(request) }); successful(&capture_server(&temp, &slow, &plan, 0, "a")); + let a = waves("a"); + // A2 must extend A's envelope even when scheduling stretches A's capture. + delay.store(slowest_burst(&a), Ordering::SeqCst); successful(&capture_server(&temp, &slow, &plan, 2, "a2")); + let a2 = waves("a2"); let output = cli() .args(["systemone", "compare", "--plan"]) .arg(&plan) @@ -446,11 +461,41 @@ fn compare_reports_a_declared_change_under_the_observed_envelope() { assert_eq!(report["acquisitions"].as_array().unwrap().len(), 3); let gates = report["gates"].as_array().unwrap(); assert_eq!(gates.len(), 2); - for gate in gates { + for (gate, declared) in gates.iter().zip(wide["gates"].as_array().unwrap()) { assert_eq!(gate["counts"], json!([3, 3, 3])); assert_eq!(gate["outcome"], json!("PASS")); - assert!(gate["reference"].is_array()); - assert!(gate["candidate"].is_array()); + let extrema = |arms: &[&[Value]]| { + let mut samples = Vec::new(); + for wave in arms + .iter() + .flat_map(|waves| waves.iter()) + .filter(|wave| wave["phase"] == "measured" && wave["cell"] == declared["cell"]) + { + assert_eq!(wave["complete"], json!(true)); + match declared["metric"].as_str().unwrap() { + "latency_us" => { + for attempt in wave["attempts"].as_array().unwrap() { + samples.push((attempt["latency_us"].as_u64().unwrap(), 1)); + } + } + "decisions_per_second" => samples.push(( + wave["decisions"].as_u64().unwrap() * 1_000_000, + wave["makespan_us"].as_u64().unwrap(), + )), + metric => panic!("unexpected fixture metric {metric}"), + } + } + samples.sort_unstable_by(|&(an, ad), &(bn, bd)| { + (u128::from(an) * u128::from(bd)).cmp(&(u128::from(bn) * u128::from(ad))) + }); + [samples[0], *samples.last().unwrap()].map(|(numerator, denominator)| { + json!({"numerator": numerator, "denominator": denominator}) + }) + }; + let reference = extrema(&[&a, &a2]); + assert_ne!(reference, extrema(&[&a]), "A2 must extend the envelope"); + assert_eq!(gate["reference"], json!(reference)); + assert_eq!(gate["candidate"], json!(extrema(&[&b]))); } let latency = &gates[0]["candidate"][1]; let reference = &gates[0]["reference"][0]; From 85159448912e86420fa2601fd89f0d5a222d737b Mon Sep 17 00:00:00 2001 From: plotarmordev <299844489+plotarmordev@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:32:51 +0800 Subject: [PATCH 3/3] Keep fixture design notes in the test, not CONTRIBUTING --- CONTRIBUTING.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c2887c4..f22a480 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,11 +38,6 @@ run is `cargo test -p grill-perf --locked --test study`. For recipe-facing changes, use the [same baseline/check interface and reviewed PR summary](docs/performance/RECIPES.md#contributor-pr-report). Recipe facts are explicit data, not a reason to add a provider wrapper. -The System One comparison fixture derives its expected reference and candidate -envelopes from retained measured waves. Its reference delays follow the preceding -captures' observed makespans so reference pooling is checked without a fixed -timing margin. - ## Changes and review - Use a feature branch and submit a pull request. Describe the behavior changed, assumptions, and checks performed.