Skip to content

Commit dc57cf2

Browse files
wan9chiclaude
andcommitted
test(fspy): report changes without judging them
The benchmark ran on thresholds set from measured noise and failed the job past them. Now that it runs on every pull request, most runs compare two identical fspy builds, where a threshold can only ever produce a false alarm. Print the change and its quartile spread and leave the judgment to the reader; the job stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f1cc2f7 commit dc57cf2

2 files changed

Lines changed: 13 additions & 42 deletions

File tree

crates/fspy_benchmark/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ So nothing is ever compared across runs. Both fspy revisions run side by side on
2323
- `fspy_benchmark_launcher` runs the target once, tracked through `fspy::Command` or untracked, and prints the launch wall clock plus the target's number. It is the only piece that links fspy.
2424
- `fspy_benchmark` is the harness. CI builds the launcher twice: against the fspy under review, and against the merge-base fspy. Both builds use the same launcher source, so both revisions are measured by identical code. The harness then launches both builds back to back, cycling every ordering. Whatever the runner does to the numbers, it does to both.
2525

26-
Each iteration gives one head/base ratio per row. The reported change is the median of those ratios. A row that moves past its threshold fails the job.
26+
Each iteration gives one head/base ratio per row. The reported change is the median of those ratios, printed with its quartile spread. The benchmark only reports; it never fails the job. Running the same fspy on both sides stays within a couple of percent, so read a change well past that as real.
2727

2828
An untracked launch runs in the same rotation. It prices tracking itself, and is reported as context, never gated.
2929

crates/fspy_benchmark/src/main.rs

Lines changed: 12 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ struct Suite {
5151
/// Unmeasured iterations run first, to fill caches and settle the runner.
5252
warmup: usize,
5353
metric: Metric,
54-
/// The change past which the suite fails the run, as a share. Set from the
55-
/// spread observed when benchmarking one commit on many runner instances.
56-
threshold: f64,
5754
}
5855

5956
/// Opens nothing, so the whole launch is the cost of starting a tracked
@@ -65,19 +62,12 @@ const LAUNCH_SUITE: Suite = Suite {
6562
iterations: if cfg!(windows) { 150 } else { 300 },
6663
warmup: 5,
6764
metric: Metric::Wall,
68-
threshold: 0.05,
6965
};
7066

7167
/// Opens timed from inside the target, so they price interception rather than
7268
/// the launch around it.
73-
const ACCESS_SUITE: Suite = Suite {
74-
name: "access",
75-
opens: "2048",
76-
iterations: 102,
77-
warmup: 3,
78-
metric: Metric::Typical,
79-
threshold: 0.08,
80-
};
69+
const ACCESS_SUITE: Suite =
70+
Suite { name: "access", opens: "2048", iterations: 102, warmup: 3, metric: Metric::Typical };
8171

8272
struct Backend {
8373
name: &'static str,
@@ -95,20 +85,15 @@ fn main() {
9585
#[cfg(not(all(target_os = "linux", target_arch = "x86_64")))]
9686
let backends = [Backend { name: "dynamic", target: DYNAMIC_TARGET }];
9787

98-
let mut regressed = false;
9988
for backend in &backends {
10089
validate(HEAD_LAUNCHER.as_ref(), backend.target);
10190
if let Some(base_launcher) = &base_launcher {
10291
validate(base_launcher, backend.target);
10392
}
10493
for suite in [&LAUNCH_SUITE, &ACCESS_SUITE] {
105-
regressed |= run_suite(backend, suite, base_launcher.as_deref());
94+
run_suite(backend, suite, base_launcher.as_deref());
10695
}
10796
}
108-
109-
if regressed {
110-
std::process::exit(1);
111-
}
11297
}
11398

11499
fn parse_base_launcher() -> Option<OsString> {
@@ -156,9 +141,8 @@ enum Arm {
156141
Base,
157142
}
158143

159-
/// Runs a suite and reports its row. Returns whether it regressed past its
160-
/// threshold.
161-
fn run_suite(backend: &Backend, suite: &Suite, base_launcher: Option<&OsStr>) -> bool {
144+
/// Runs a suite and reports its row.
145+
fn run_suite(backend: &Backend, suite: &Suite, base_launcher: Option<&OsStr>) {
162146
let orders = if base_launcher.is_some() { ORDERS_WITH_BASE } else { ORDERS_WITHOUT_BASE };
163147
// Whole cycles only, so that each ordering runs equally often.
164148
let iterations = suite.iterations.next_multiple_of(orders.len());
@@ -184,40 +168,27 @@ fn run_suite(backend: &Backend, suite: &Suite, base_launcher: Option<&OsStr>) ->
184168
}
185169
}
186170

187-
report(backend, suite, &measured, base_launcher.is_some())
171+
report(backend, suite, &measured, base_launcher.is_some());
188172
}
189173

190-
/// Prints the suite's row and returns whether it regressed past its threshold.
174+
/// Prints the suite's row.
191175
#[expect(clippy::print_stdout, reason = "the report is the benchmark's output")]
192-
fn report(backend: &Backend, suite: &Suite, iterations: &[Iteration], has_base: bool) -> bool {
176+
fn report(backend: &Backend, suite: &Suite, iterations: &[Iteration], has_base: bool) {
193177
let name = [backend.name, "/", suite.name].concat();
194178
let overheads = sorted(iterations.iter().map(|it| it.head / it.untracked - 1.0));
195179
let overhead = quantile(&overheads, 1, 2);
196180

197181
if has_base {
198182
let changes = sorted(iterations.iter().map(|it| it.head / it.base - 1.0));
199-
let change = quantile(&changes, 1, 2);
200-
let low = quantile(&changes, 1, 4);
201-
let high = quantile(&changes, 3, 4);
202-
let verdict = if change > suite.threshold {
203-
" REGRESSED"
204-
} else if change < -suite.threshold {
205-
" improved"
206-
} else {
207-
""
208-
};
209183
println!(
210-
"{name:<26} change {:>+6.2}% [{:>+6.2}% .. {:>+6.2}%] threshold {:>4.1}% overhead {:>+8.2}%{verdict}",
211-
change * 100.0,
212-
low * 100.0,
213-
high * 100.0,
214-
suite.threshold * 100.0,
184+
"{name:<26} change {:>+6.2}% [{:>+6.2}% .. {:>+6.2}%] overhead {:>+8.2}%",
185+
quantile(&changes, 1, 2) * 100.0,
186+
quantile(&changes, 1, 4) * 100.0,
187+
quantile(&changes, 3, 4) * 100.0,
215188
overhead * 100.0,
216189
);
217-
change > suite.threshold
218190
} else {
219191
println!("{name:<26} overhead {:>+8.2}%", overhead * 100.0);
220-
false
221192
}
222193
}
223194

0 commit comments

Comments
 (0)