dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Measure the performance targets §8 has been promising, and fail on a regression

Browse Source 
docs/requirements.md §8 has said since it was written that performance is
verified by "an automated benchmark suite against a synthetic 50k catalog, run
per-commit … A regression beyond stated tolerance fails the build." There was
none. No benches/, no [[bench]], no criterion, no synthetic catalog, and three
CI workflows that between them measured nothing. Ten performance requirements
could therefore be neither passed nor failed, and five of them carried a
TRACES: tag regardless.

tools/bench is the half of that promise that can be kept honestly on a runner
with no GPU and no display.

# The fixture

Rows are cheap and pixels are not, so it builds fifty thousand catalog rows
over a pool of a dozen real files, each referenced by several thousand of them.
Everything the catalog half touches is rows and is exact at full scale;
everything the pixel half touches is one file at a time and does not care how
many rows point at it. Fourteen megabytes on disk instead of two terabytes, and
neither half is flattered by the trade. It is reproducible from a seed, and a
stamp beside it — seed, row count, source size, dr-catalog's schema version —
rebuilds it rather than letting a run be compared against a baseline that
describes a different library.

# What it can now pass or fail

NFR-P1, and R2's second sentence with it: Catalog::open plus the count, first
window and timeline the grid cannot paint without. The interesting part turned
out to be the open itself — schema::backfill runs three passes over the images
table on every open, which is O(library) work on a path whose budget is stated
in absolute seconds. Tagged TRACES: NFR-P1, on a gate that fails if it breaks.

NFR-P3: thumbnail throughput on the embedded preview path, through the same
per-image work spawn_thumbnail_sweep does and in the same shape — chunks of 96,
lanes owning disjoint slices, the single thread that owns the store writing the
finished chunk. Mirrored rather than called, because that function takes a
RemoteBackend and would measure somebody's network. Tagged TRACES: NFR-P3.

# What it deliberately does not claim

NFR-P7 is the whole chain, and only the encode half of it runs without an
adapter. So the export row is a one-sided gate — over two seconds in the encode
alone violates the requirement; under it proves nothing — and there is no
TRACES: NFR-P7 anywhere. NFR-P8 is about the application at idle, and the probe
is a process holding the catalog and nothing else, so it records the catalog
layer's share and carries no budget until somebody decides what that share
should be. No tag there either. CONTRIBUTING.md asks that a requirement be
closed by a test that would fail if the behaviour were removed, and two more
plumbing tags is what this repository already has too many of.

NFR-P8 also gets the answer §4.1 demands: RSS is exclusive of device-local GPU
allocations and cannot be made otherwise, because such an allocation never
enters the process's address space. The requirement should be restated as two
figures, and docs/benchmarks.md says so.

# Two gates, and why one of them steps aside off the reference desktop

The budget is the requirement's own number and never moves. The baseline is
what the reference desktop last measured, and drifting 15% past it fails the
build even while still inside the budget — which is how performance rot
actually arrives, never over the line, always a little worse.

A budget written for twenty-four threads cannot be asserted on a two-core
container. §8 names the reference desktop, not CI, so each metric declares
whether its budget is machine-sensitive; those are asserted under --reference
and reported everywhere else. Catalog open is not one of them: two seconds
against an expected figure two orders of magnitude smaller is a threshold any
machine can be held to. This is the trap core/dr-gpu/tests/frame_budget.rs
already refuses — a red gate everybody learns to ignore.

# The baseline ships with no numbers in it

Every recorded field is null, because nobody has run it yet. Writing
plausible-looking figures would make every later comparison a comparison
against a guess, and the first real regression would be invisible. Run
`dr-bench record --reference` on the reference desktop and commit the diff;
until then the budget gate works and the report says the other one cannot.

# CI

.gitea/workflows/benchmark.yml, and its own workflow rather than a step in
build-and-test.yml: a red "Build and test" says the code is wrong, a red
"Benchmarks" says it got slower, and the second must not be reachable by
retrying a flaky compile. The cpu job runs on every push and builds -p dr-bench
alone — which is why that crate depends on no GPU and no UI crate. The gpu job
is the frame budget that already exists and already skips without an adapter,
on workflow_dispatch, because building wgpu on every commit to rediscover that
the runner has no device is not a use of anybody's minutes.
This commit is contained in:
dtourolle
2026-08-30 10:40:10 +02:00
parent ef07e6ca3e
commit e8e96eed40
16 changed files with 2821 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/bench/src/baseline.rs
+300
View File
@@ -0,0 +1,300 @@
//! The committed numbers, and what counts as a regression against them.
//!
//! `docs/frame-budget.md` commits its measurements by hand and says why: *"a
//! regression should be a diff rather than somebody's memory."* This is the
//! same idea in a form a program can read, because §8 asks for more than a
//! record — *"a regression beyond stated tolerance fails the build"*.
//!
//! # Two gates, and they are not the same gate
//!
//! **The budget** is the requirement's own number: 2 s to open a catalog, 100
//! images a second through the preview path. It does not move. A build that
//! violates it has violated a requirement, and no amount of "but it was always
//! like that" changes it.
//!
//! **The baseline** is what this machine last measured. It moves — deliberately
//! and by hand, through `dr-bench record` — and its job is to catch the change
//! that is still inside the budget but has halved the headroom. Most real
//! performance rot arrives that way: never over the line, always a little
//! worse, until one day the line is crossed by a change that was not the cause.
//!
//! # Why a machine-sensitive metric skips its budget off the reference desktop
//!
//! §8 names *"the reference desktop"*, not CI, and it is right to. A container
//! with two cores cannot speak to a throughput target written for a
//! twenty-four-thread machine, and asserting it there would produce exactly
//! what `core/dr-gpu/tests/frame_budget.rs` refused to produce: *"a red suite
//! that everyone learns to ignore"*. So a metric declares whether its budget
//! is machine-sensitive. Those budgets are asserted under `--reference` and
//! reported everywhere else; the ones with orders of magnitude of headroom —
//! catalog open against two seconds — are asserted everywhere, because a
//! failure there is a real failure on any machine.
//!
//! # Why the committed file starts with no numbers in it
//!
//! Because nobody had run it yet. Writing plausible-looking figures into a
//! baseline is the one thing that would make the whole suite worthless: every
//! later comparison would be against a guess, and the first genuine regression
//! would be invisible or, worse, a fabricated improvement. `recorded` is
//! therefore `null` until somebody runs `dr-bench record --reference` on the
//! reference desktop and commits the diff. Until then the budget gate works
//! and the regression gate says so rather than pretending.
use std::collections::BTreeMap;
use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use serde::{Deserialize, Serialize};
use crate::fixture::Stamp;
/// Which way is better.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum Direction {
LowerIsBetter,
HigherIsBetter,
}
/// One measured quantity: what it is, what it must be, and what it was.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Metric {
/// The requirement ID this speaks to, or a note saying it speaks to only
/// part of one. Free text on purpose: several of these describe a fraction
/// of a requirement, and a bare ID here would read as the whole of it.
pub requirement: String,
/// One sentence a reader of the JSON can understand without the code.
pub what: String,
pub unit: String,
pub direction: Direction,
/// Whether the budget below is a statement about a machine as much as
/// about the code. See this module's header.
pub machine_sensitive: bool,
/// The requirement's own threshold, where it has one this can check.
pub budget: Option<f64>,
/// What the last `record` measured. `null` until one has been taken.
pub recorded: Option<f64>,
}
/// The committed file.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Baseline {
/// Prose for whoever opens the JSON first. Kept in the file rather than
/// only in `docs/benchmarks.md`, because the person who finds this in a
/// failing CI log is not reading the docs directory at that moment.
#[serde(rename = "_readme")]
pub readme: Vec<String>,
/// How much worse than [`Metric::recorded`] a metric may get before the
/// build fails, as a fraction. 0.15 is fifteen per cent.
pub tolerance: f64,
/// The machine the recorded figures came from, as [`machine_id`] spells
/// it. Compared before a regression is judged: drift against a different
/// machine's numbers is not a regression, it is a different machine.
pub recorded_on: Option<String>,
/// Unix seconds. An integer rather than a formatted date because this
/// workspace has no date library and adding one for a comment would be a
/// poor trade.
pub recorded_at_unix: Option<i64>,
/// The fixture the recorded figures describe. A number measured against a
/// different workload is not comparable, and this is what says so.
pub fixture: Option<Stamp>,
pub metrics: BTreeMap<String, Metric>,
}
/// How a measurement compares.
#[derive(Debug, Clone, Copy)]
pub struct Judgement {
/// Past the requirement's own threshold. A build failure.
pub over_budget: bool,
/// Worse than the recorded baseline by more than the tolerance. A build
/// failure.
pub regressed: bool,
/// Fractional change against the baseline, positive meaning worse.
pub drift: Option<f64>,
/// A budget exists but was not asserted, because it is machine-sensitive
/// and this is not the reference desktop.
pub budget_deferred: bool,
}
/// What a judgement is made in the light of.
#[derive(Debug, Clone, Copy)]
pub struct Judging {
/// This run declares itself the reference desktop.
pub reference: bool,
/// This run is on the machine the baseline was recorded on.
pub same_machine: bool,
pub tolerance: f64,
}
impl Metric {
/// Judge `measured` against the budget and the baseline.
pub fn judge(&self, measured: f64, cx: Judging) -> Judgement {
let violates = |threshold: f64| match self.direction {
Direction::LowerIsBetter => measured > threshold,
Direction::HigherIsBetter => measured < threshold,
};
let assert_budget = self.budget.is_some() && (cx.reference || !self.machine_sensitive);
let drift = match self.recorded {
// A recorded zero would divide by nothing, and a recorded figure
// of zero is a broken record rather than a very fast one.
Some(was) if was > 0.0 => Some(match self.direction {
Direction::LowerIsBetter => (measured - was) / was,
Direction::HigherIsBetter => (was - measured) / was,
}),
_ => None,
};
Judgement {
over_budget: assert_budget && self.budget.is_some_and(violates),
regressed: cx.same_machine && drift.is_some_and(|d| d > cx.tolerance),
drift,
budget_deferred: self.budget.is_some() && !assert_budget,
}
}
}
impl Baseline {
pub fn load(path: &Path) -> Result<Baseline> {
let text = std::fs::read_to_string(path)
.with_context(|| format!("reading the baseline at {}", path.display()))?;
serde_json::from_str(&text)
.with_context(|| format!("parsing the baseline at {}", path.display()))
}
pub fn save(&self, path: &Path) -> Result<()> {
let mut text = serde_json::to_string_pretty(self)?;
// A trailing newline, so the file is a well-behaved text file and a
// `record` that changed nothing produces an empty diff.
text.push('\n');
std::fs::write(path, text)
.with_context(|| format!("writing the baseline to {}", path.display()))
}
/// Where the committed baseline lives, found the way `tools/traceability`
/// finds the repo root: by walking up from this crate's manifest until
/// `docs/requirements.md` appears.
pub fn default_path() -> Result<PathBuf> {
let mut dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
while !dir.join("docs/requirements.md").exists() {
if !dir.pop() {
anyhow::bail!("could not locate the repo root above this crate");
}
}
Ok(dir.join("docs/bench-baseline.json"))
}
}
/// How this machine is named in the baseline.
///
/// Host name plus thread count. Not a hardware inventory — it exists to answer
/// one question, "are these numbers from here?", and to answer it the same way
/// twice on the same box. A container whose hostname changes per run therefore
/// never matches, which is the correct answer for CI: its drift is information,
/// not a verdict.
pub fn machine_id() -> String {
let host = std::fs::read_to_string("/proc/sys/kernel/hostname")
.map(|s| s.trim().to_string())
.unwrap_or_else(|_| "unknown-host".to_string());
let threads = std::thread::available_parallelism()
.map(|n| n.get())
.unwrap_or(0);
format!("{host} ({threads} threads)")
}
/// Unix seconds now, or 0 if the clock is before 1970, which it is not.
pub fn now_unix() -> i64 {
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs() as i64)
.unwrap_or(0)
}
#[cfg(test)]
mod tests {
use super::*;
fn metric(direction: Direction, budget: Option<f64>, recorded: Option<f64>) -> Metric {
Metric {
requirement: "NFR-TEST".into(),
what: "a test metric".into(),
unit: "ms".into(),
direction,
machine_sensitive: false,
budget,
recorded,
}
}
fn cx(same_machine: bool) -> Judging {
Judging {
reference: true,
same_machine,
tolerance: 0.15,
}
}
#[test]
fn a_budget_is_directional() {
// The bug this exists to prevent: judging a throughput target the way
// a latency target is judged, so a suite that got twice as slow passes
// and one that got twice as fast fails.
let latency = metric(Direction::LowerIsBetter, Some(100.0), None);
assert!(latency.judge(101.0, cx(true)).over_budget);
assert!(!latency.judge(99.0, cx(true)).over_budget);
let throughput = metric(Direction::HigherIsBetter, Some(100.0), None);
assert!(throughput.judge(99.0, cx(true)).over_budget);
assert!(!throughput.judge(101.0, cx(true)).over_budget);
}
#[test]
fn drift_is_positive_when_things_got_worse_whichever_way_that_is() {
let latency = metric(Direction::LowerIsBetter, None, Some(100.0));
assert!(latency.judge(120.0, cx(true)).drift.unwrap() > 0.0);
let throughput = metric(Direction::HigherIsBetter, None, Some(100.0));
assert!(throughput.judge(80.0, cx(true)).drift.unwrap() > 0.0);
}
#[test]
fn drift_against_another_machine_is_reported_but_never_a_failure() {
// CI is not the reference desktop. Its numbers are worth printing and
// are not a verdict on anybody's commit.
let m = metric(Direction::LowerIsBetter, None, Some(100.0));
let elsewhere = m.judge(400.0, cx(false));
assert!(elsewhere.drift.unwrap() > 0.15);
assert!(!elsewhere.regressed);
assert!(m.judge(400.0, cx(true)).regressed);
}
#[test]
fn an_unrecorded_metric_cannot_regress() {
// The state the committed file ships in. It must gate on the budget
// and stay silent about drift, rather than treating null as zero and
// declaring an infinite regression.
let m = metric(Direction::LowerIsBetter, Some(100.0), None);
let j = m.judge(50.0, cx(true));
assert!(j.drift.is_none());
assert!(!j.regressed);
assert!(!j.over_budget);
}
#[test]
fn a_machine_sensitive_budget_defers_off_the_reference_desktop() {
let mut m = metric(Direction::HigherIsBetter, Some(100.0), None);
m.machine_sensitive = true;
let on_ci = Judging {
reference: false,
same_machine: false,
tolerance: 0.15,
};
let j = m.judge(10.0, on_ci);
// A two-core runner must not fail a target written for twenty-four.
assert!(!j.over_budget);
// And the report has to say the budget was not applied, rather than
// letting a deferred budget read as a passed one.
assert!(j.budget_deferred);
// On the reference desktop the same figure is judged.
assert!(m.judge(10.0, cx(false)).over_budget);
}
}