cargo perf --baseline <report.json>: a diff that refuses a difference the spread cannot support #301

Closed
opened 2026-09-11 00:39:57 +00:00 by viberfox-agent · 6 comments
Collaborator

Problem

cargo perf measures a run and writes a JSON report (crates/cartopolis/src/systems/dev/perf.rs:366, alias at .cargo/config.toml:88), but nothing reads one back. Comparing two runs today means opening two JSON files and subtracting columns by eye — which is precisely how the withdrawn sky-shell A/B happened: three runs of one unchanged configuration reported fill counts of 160 836, 218 436 and 218 436, two of them differing by exactly one full screen, and an A/B differenced against single readings produced a clean-looking table saying that turning the clouds off increased fill (docs/notes/performance-suite.md:71).

The report already carries what is needed to refuse that: each window samples fragment_shader_invocations every frame and records frag_min, frag_max and frag_samples per render pass (perf.rs:291, sampled at perf.rs:573), and the field's own doc says a point sample "is not reproducible and will mislead you" (perf.rs:283). ReportMeta exists so "a report says what it is comparable with" (perf.rs:349). Nothing consumes either.

Three further facts the diff has to respect, all already written down:

  • Not every column can carry a difference. docs/notes/render-performance-survey.md:26-31 is an explicit table of what is trustworthy in this container: draws/tris_* yes, asset counts yes, worker_outstanding timing-dependent (report only), frame_ms no, elapsed_gpu "theatre". Window::fps repeats it at the field: "Reported, never gated" (perf.rs:306), as does GpuPass::gpu_ms — "Structure yes, magnitude no" (perf.rs:273).
  • Columns can be absent rather than zero. process_cpu_percent/process_mem_percent only exist under --features perf (lib.rs:1781), and the GPU passes only when RenderDiagnosticsPlugin is added (lib.rs:2209). Option::None on one side is not measured, not a delta from zero.
  • A steady window that is not quiet is a streaming measurement wearing the wrong label (perf.rs:344, warned at perf.rs:707), and city does not go quiet on this container (docs/perf.toml, the city description).

Approach

One new module, crates/cartopolis/src/systems/dev/perf_diff.rs, holding a pure function over two already-parsed Reports plus a logger for the result. Nothing in it touches Bevy, so it is unit-testable without an App.

Data model.

pub enum Verdict {
    Unchanged,       // both sides present and equal
    Established,     // the delta is larger than the combined spread
    NotEstablished,  // the two runs' ranges overlap — refuse to call it a difference
    NotMeasured,     // absent, or too few samples, on at least one side
    NotEvidence,     // present in both, but this column is never evidence here
}

pub struct DeltaRow { column: String, before: Option<f64>, after: Option<f64>,
                      delta: Option<f64>, percent: Option<f64>,
                      verdict: Verdict, note: Option<String> }

pub struct WindowDiff { scenario: String, phase: String,
                        quiet_before: bool, quiet_after: bool, rows: Vec<DeltaRow> }

pub struct Diff { meta_differences: Vec<String>,
                  windows: Vec<WindowDiff>,
                  only_in_baseline: Vec<(String, String)>,   // (scenario, phase)
                  only_in_current:  Vec<(String, String)> }

pub fn diff(baseline: &Report, current: &Report) -> Result<Diff, String>;
pub fn log_diff(d: &Diff);

Which verdict each column gets, and why. The classification is a table in the module, each row carrying the citation for its bucket — the survey table above is the authority, and a column it does not name is placed by the closest row it does.

columns kind reason
draws, tris_total, entity_count, mesh_slabs, mesh_slab_mb, resident_kb, resident_image_kb, resident_mesh_kb exact count — Unchanged/Established survey:26-28; residency's totals reproduced an independent measurement a year apart (docs/notes/performance-suite.md, "Where the gigabyte is")
per-pass frag_min..frag_max spread — interval test perf.rs:283
fps, frame_time_ms, per-pass gpu_ms, cpu_ms, process_cpu_percent, process_mem_percent NotEvidence survey:30-31; perf.rs:273, perf.rs:306, perf.rs:310
upload_kb, worker_jobs, mesh_added_per_frame, frames, wall_s NotEvidence survey:29 — window traffic is timing-dependent, report only. These are the inputs to the quiet test (perf.rs:705), so they are printed for context
per-pass fragment_invocations (the smoothed point value) not diffed at all perf.rs:283: differencing it is the withdrawn experiment

NotEvidence rows are still printed with their before/after and delta — that is the "structure yes, magnitude no" reading — but the renderer marks them and they are never counted as a difference.

The spread test, for each pass path present in both windows:

  • Either side missing frag_min/frag_max, or frag_samples < 2 on either side → NotMeasured, note saying a single sample cannot support a difference. This guard is the point: a one-frame load window is reachable (perf.rs:643) and would otherwise produce a zero-width interval and a confident wrong answer.
  • Intervals overlap (a.min <= b.max && b.min <= a.max) → NotEstablished, note carrying both ranges and both sample counts.
  • Otherwise Established, with delta the conservative gap between the nearest edges (b.min - a.max when the new run is higher, b.max - a.min when lower) — the smallest difference consistent with both ranges — and percent against the baseline midpoint. Note that this test is identical to "the midpoint delta exceeds half the combined spread"; it is stated as an interval test because that is what the data is.
  • A pass present in one window only → one row naming it, NotMeasured.

Matching and meta. Windows are matched by (scenario, phase); unmatched ones go into only_in_baseline / only_in_current and are logged by name. A duplicate (scenario, phase) key in either report is an error naming the key rather than a silent first-match (the manifest already refuses duplicate scenario names at perf.rs:146; a report with duplicates is malformed and pairing it silently is the "baseline nobody can reproduce" failure perf.rs:80 guards against). Every ReportMeta field that differs becomes one line in meta_differences — a warning, not an error, because CARTO_QUALITY=phone cargo perf --only city against a desktop report is a question somebody legitimately asks.

Wiring (perf.rs, lib.rs):

  • PerfRun gains pub baseline: Option<Report> — the parsed report, not a path.
  • perf::build() (perf.rs:444) gains a baseline-path parameter and loads and parses it eagerly, returning Err in the same shape as PerfManifest::load (perf.rs:139, "{path}: {e}"). A typo'd path must fail before the first teleport, not after a suite run; this is the same doctrine as an unknown manifest key being a startup error.
  • perf::finish() (perf.rs:772) calls perf_diff::diff + log_diff after the report is written, so the diff describes the report on disk.
  • lib.rs gains #[arg(long, alias = "baseline", value_name = "PATH")] perf_baseline: Option<PathBuf> beside --perf-out/--perf-frames (lib.rs:113-120). The canonical spelling is --perf-baseline, matching its two siblings; --baseline is kept as an alias so the spelling in this ticket's title works. Passing it without --perf is an error in the shape of lib.rs:265.
  • Output goes through tracing, one structured line per row plus a summary, matching log_window/"perf gpu pass" (perf.rs:724-768). No println! — nothing in systems/dev/ uses one.

Docs. A "Comparing two runs" section in docs/notes/performance-suite.md (it already ends on the fill-counter reproducibility problem, which is what this answers), and the cargo perf comment block at .cargo/config.toml:83-88 gains the flag.

Acceptance criteria

  • cargo perf --perf-baseline <report.json> runs the suite, writes its report as before, and then logs a per-window, per-column diff. --baseline is accepted as an alias.
  • --perf-baseline without --perf is a startup error naming the flag.
  • A missing or unparseable baseline fails at startup, before any scenario is flown, with a message containing the path.
  • Windows are matched by (scenario, phase). A window present in only one report is logged by name under "only in the baseline" / "only in the new report" and is never silently dropped.
  • A duplicate (scenario, phase) in either report is an error naming the key.
  • Differing ReportMeta fields are logged as a warning listing each field with both values; the diff still runs.
  • Per-pass fill is compared as an interval [frag_min, frag_max]: overlapping intervals give NotEstablished with both ranges in the note; disjoint intervals give Established with the conservative edge-to-edge delta.
  • frag_samples < 2 on either side gives NotMeasured, not a delta.
  • The smoothed fragment_invocations point value is not diffed.
  • Timing and window-traffic columns are printed with their deltas but always carry NotEvidence and are excluded from the "established differences" count in the summary line.
  • Each window's quiet flag is reported for both sides; a steady pair where either side is not quiet logs the warning that its numbers describe a world being built. Deltas are still printed — city never goes quiet on this container, and suppressing it would delete the only ground measurement.
  • Unit tests in perf_diff.rs, over fixture Reports built in the test module (inline JSON, so the loader is exercised too — no new files, and no dependence on the repo root the way the_shipped_manifest_parses needs at perf.rs:854):
    • a real change — disjoint fill intervals, and a changed draws — is reported as a delta with the expected sign and magnitude;
    • a change inside the spread — overlapping fill intervals whose midpoints differ — is NotEstablished;
    • a window present in one report only is named in only_in_baseline / only_in_current;
    • a frag_samples == 1 pair is NotMeasured;
    • a column None on one side is NotMeasured, not a delta from zero;
    • a report diffed against itself yields no Established row.
  • docs/notes/performance-suite.md gains a section saying which columns can carry a difference and which cannot, and why; .cargo/config.toml's cargo perf comment mentions the flag.
  • The commit body names the value from docs/direction.md that settled the column classification (value 6, ship the smallest thing a machine can judge — counts and spreads are judgeable, timings on this host are not) — required anyway, since this is a feat touching more than five files.

Verification

cargo test -p cartopolis perf_diff          # the unit tests above
cargo fmt --check -p cartopolis             # plus the other four owned crates
cargo check -p cartopolis

End to end, on a host that can render (lavapipe here is enough — the measurements in docs/notes/performance-suite.md were taken on this container; the container this refinement pass runs in must not build):

cargo perf --only orbit --perf-out /tmp/perf-a.json
cargo perf --only orbit --perf-out /tmp/perf-b.json --perf-baseline /tmp/perf-a.json

orbit is the scenario that reliably goes quiet, so it is the one to smoke-test with. Two unchanged runs of it should produce no Established row; if they do, the diff is reporting run-to-run noise as a finding and the column that did it needs reclassifying before this lands. A --features perf run diffed against a non-perf one is the check that absent columns come back NotMeasured.

Not checkable here or on this container at all: whether the classification holds on a real GPU. The Steam Deck runner (deck-gpu, see the steamdeck-gpu-runner note) is where a desktop-GPU pair of reports would come from.

Out of scope

  • Any exit code or gate. A regression does not fail the run. Thresholds would have to be guessed, and --expect already exists for gating a scripted run.
  • Diffing two saved reports without flying anything (--perf-diff a.json b.json). The pure function makes it a few lines later; it is a second entry path into lib.rs and a separate ticket.
  • Writing the diff to a file. Log only.
  • Recording a spread for any column other than fill — e.g. per-frame min/max of frame time, which the frag sampler's shape would fit. Timings are not evidence here, so a spread on them buys nothing today; it becomes worth doing when a report comes off real hardware.
  • Repeating a scenario N times within one run to get a cross-run spread. That is a change to the measuring path, not the diff.
  • Making Window/GpuPass fields #[serde(default)] so an older baseline still parses. Deliberately not done: a missing column would silently read 0 and be reported as a large delta, which is worse than the parse error. A baseline from a build with different columns fails loudly, with the path in the message.
  • Any new scenario in docs/perf.toml.

Open questions

None.


Branch: feat/301-perf-baseline-diff

Original request

From the maintainer's focus for this lane:

Improving performance on the steamdeck. keep it concrete and simple. use or extend harness tooling to find useful improvements.

cargo perf --baseline <report.json>: a diff that refuses a difference the spread cannot support

Same module as the ticket above and the half that makes it useful. Reads a previous report, matches windows by (scenario, phase), prints per-column deltas, and where a delta is smaller than the combined spread of the two runs reports it as not established rather than as a number. This is the tool every later ticket here is judged with, and it is what stops a fourth repetition of the sky-shell A/B that produced a clean-looking table saying turning the clouds off increased fill. Judged by: unit tests over two fixture reports — a real change reported as a delta, a change inside the spread reported as not established, and a window present in one report and not the other named rather than silently dropped.

This is item 3 of 20 of the lane's queue. It is ordered by a person in the
admin console, so build this one rather than the one you would have picked — and keep it
the size it is. A ticket that turns out to be three tickets is one ticket: finish this
part and say on it what the other two are.

If the item is simply wrong — already done, impossible, or a bad idea against the values
— say so here and close it. That is a result, not a failure.

Decide it yourself. This ticket is not being watched, so a question asked here is a
ticket that stops. The seven values at the top of docs/direction.md exist to settle
exactly that kind of ambiguity: pick the reading they support, say in the commit body
which one you applied, and build. Only a decision needing something nobody can derive
from the repository — a credential, a licence somebody must accept, a choice about what
the project is for — is a reason to stop.

Filed by the autopilot.

🤖 Refined by the viberfox issue agent. Reply with @agent refine and what is wrong to have this rewritten.

## Problem `cargo perf` measures a run and writes a JSON report (`crates/cartopolis/src/systems/dev/perf.rs:366`, alias at `.cargo/config.toml:88`), but nothing reads one back. Comparing two runs today means opening two JSON files and subtracting columns by eye — which is precisely how the withdrawn sky-shell A/B happened: three runs of one unchanged configuration reported fill counts of 160 836, 218 436 and 218 436, two of them differing by exactly one full screen, and an A/B differenced against single readings produced a clean-looking table saying that turning the clouds *off* increased fill (`docs/notes/performance-suite.md:71`). The report already carries what is needed to refuse that: each window samples `fragment_shader_invocations` every frame and records `frag_min`, `frag_max` and `frag_samples` per render pass (`perf.rs:291`, sampled at `perf.rs:573`), and the field's own doc says a point sample "is not reproducible and will mislead you" (`perf.rs:283`). `ReportMeta` exists so "a report says what it is comparable with" (`perf.rs:349`). Nothing consumes either. Three further facts the diff has to respect, all already written down: - **Not every column can carry a difference.** `docs/notes/render-performance-survey.md:26-31` is an explicit table of what is trustworthy in this container: `draws`/`tris_*` yes, asset counts yes, `worker_outstanding` timing-dependent (report only), `frame_ms` no, `elapsed_gpu` "theatre". `Window::fps` repeats it at the field: "Reported, never gated" (`perf.rs:306`), as does `GpuPass::gpu_ms` — "Structure yes, magnitude no" (`perf.rs:273`). - **Columns can be absent rather than zero.** `process_cpu_percent`/`process_mem_percent` only exist under `--features perf` (`lib.rs:1781`), and the GPU passes only when `RenderDiagnosticsPlugin` is added (`lib.rs:2209`). `Option::None` on one side is *not measured*, not a delta from zero. - **A `steady` window that is not `quiet` is a streaming measurement wearing the wrong label** (`perf.rs:344`, warned at `perf.rs:707`), and `city` does not go quiet on this container (`docs/perf.toml`, the `city` description). ## Approach One new module, `crates/cartopolis/src/systems/dev/perf_diff.rs`, holding a pure function over two already-parsed `Report`s plus a logger for the result. Nothing in it touches Bevy, so it is unit-testable without an App. **Data model.** ```rust pub enum Verdict { Unchanged, // both sides present and equal Established, // the delta is larger than the combined spread NotEstablished, // the two runs' ranges overlap — refuse to call it a difference NotMeasured, // absent, or too few samples, on at least one side NotEvidence, // present in both, but this column is never evidence here } pub struct DeltaRow { column: String, before: Option<f64>, after: Option<f64>, delta: Option<f64>, percent: Option<f64>, verdict: Verdict, note: Option<String> } pub struct WindowDiff { scenario: String, phase: String, quiet_before: bool, quiet_after: bool, rows: Vec<DeltaRow> } pub struct Diff { meta_differences: Vec<String>, windows: Vec<WindowDiff>, only_in_baseline: Vec<(String, String)>, // (scenario, phase) only_in_current: Vec<(String, String)> } pub fn diff(baseline: &Report, current: &Report) -> Result<Diff, String>; pub fn log_diff(d: &Diff); ``` **Which verdict each column gets, and why.** The classification is a table in the module, each row carrying the citation for its bucket — the survey table above is the authority, and a column it does not name is placed by the closest row it does. | columns | kind | reason | |---|---|---| | `draws`, `tris_total`, `entity_count`, `mesh_slabs`, `mesh_slab_mb`, `resident_kb`, `resident_image_kb`, `resident_mesh_kb` | exact count — `Unchanged`/`Established` | survey:26-28; residency's totals reproduced an independent measurement a year apart (`docs/notes/performance-suite.md`, "Where the gigabyte is") | | per-pass `frag_min..frag_max` | spread — interval test | `perf.rs:283` | | `fps`, `frame_time_ms`, per-pass `gpu_ms`, `cpu_ms`, `process_cpu_percent`, `process_mem_percent` | `NotEvidence` | survey:30-31; `perf.rs:273`, `perf.rs:306`, `perf.rs:310` | | `upload_kb`, `worker_jobs`, `mesh_added_per_frame`, `frames`, `wall_s` | `NotEvidence` | survey:29 — window traffic is timing-dependent, report only. These are the inputs to the `quiet` test (`perf.rs:705`), so they are printed for context | | per-pass `fragment_invocations` (the smoothed point value) | **not diffed at all** | `perf.rs:283`: differencing it is the withdrawn experiment | `NotEvidence` rows are still printed with their before/after and delta — that is the "structure yes, magnitude no" reading — but the renderer marks them and they are never counted as a difference. **The spread test**, for each pass path present in both windows: - Either side missing `frag_min`/`frag_max`, **or `frag_samples < 2` on either side** → `NotMeasured`, note saying a single sample cannot support a difference. This guard is the point: a one-frame `load` window is reachable (`perf.rs:643`) and would otherwise produce a zero-width interval and a confident wrong answer. - Intervals overlap (`a.min <= b.max && b.min <= a.max`) → `NotEstablished`, note carrying both ranges and both sample counts. - Otherwise `Established`, with `delta` the **conservative** gap between the nearest edges (`b.min - a.max` when the new run is higher, `b.max - a.min` when lower) — the smallest difference consistent with both ranges — and `percent` against the baseline midpoint. Note that this test is identical to "the midpoint delta exceeds half the combined spread"; it is stated as an interval test because that is what the data is. - A pass present in one window only → one row naming it, `NotMeasured`. **Matching and meta.** Windows are matched by `(scenario, phase)`; unmatched ones go into `only_in_baseline` / `only_in_current` and are logged by name. A duplicate `(scenario, phase)` key in either report is an error naming the key rather than a silent first-match (the manifest already refuses duplicate scenario names at `perf.rs:146`; a report with duplicates is malformed and pairing it silently is the "baseline nobody can reproduce" failure `perf.rs:80` guards against). Every `ReportMeta` field that differs becomes one line in `meta_differences` — a warning, not an error, because `CARTO_QUALITY=phone cargo perf --only city` against a desktop report is a question somebody legitimately asks. **Wiring** (`perf.rs`, `lib.rs`): - `PerfRun` gains `pub baseline: Option<Report>` — the parsed report, not a path. - `perf::build()` (`perf.rs:444`) gains a baseline-path parameter and **loads and parses it eagerly**, returning `Err` in the same shape as `PerfManifest::load` (`perf.rs:139`, `"{path}: {e}"`). A typo'd path must fail before the first teleport, not after a suite run; this is the same doctrine as an unknown manifest key being a startup error. - `perf::finish()` (`perf.rs:772`) calls `perf_diff::diff` + `log_diff` after the report is written, so the diff describes the report on disk. - `lib.rs` gains `#[arg(long, alias = "baseline", value_name = "PATH")] perf_baseline: Option<PathBuf>` beside `--perf-out`/`--perf-frames` (`lib.rs:113-120`). The canonical spelling is `--perf-baseline`, matching its two siblings; `--baseline` is kept as an alias so the spelling in this ticket's title works. Passing it without `--perf` is an error in the shape of `lib.rs:265`. - Output goes through `tracing`, one structured line per row plus a summary, matching `log_window`/`"perf gpu pass"` (`perf.rs:724-768`). No `println!` — nothing in `systems/dev/` uses one. **Docs.** A "Comparing two runs" section in `docs/notes/performance-suite.md` (it already ends on the fill-counter reproducibility problem, which is what this answers), and the `cargo perf` comment block at `.cargo/config.toml:83-88` gains the flag. ## Acceptance criteria - [ ] `cargo perf --perf-baseline <report.json>` runs the suite, writes its report as before, and then logs a per-window, per-column diff. `--baseline` is accepted as an alias. - [ ] `--perf-baseline` without `--perf` is a startup error naming the flag. - [ ] A missing or unparseable baseline fails **at startup**, before any scenario is flown, with a message containing the path. - [ ] Windows are matched by `(scenario, phase)`. A window present in only one report is logged by name under "only in the baseline" / "only in the new report" and is never silently dropped. - [ ] A duplicate `(scenario, phase)` in either report is an error naming the key. - [ ] Differing `ReportMeta` fields are logged as a warning listing each field with both values; the diff still runs. - [ ] Per-pass fill is compared as an interval `[frag_min, frag_max]`: overlapping intervals give `NotEstablished` with both ranges in the note; disjoint intervals give `Established` with the conservative edge-to-edge delta. - [ ] `frag_samples < 2` on either side gives `NotMeasured`, not a delta. - [ ] The smoothed `fragment_invocations` point value is not diffed. - [ ] Timing and window-traffic columns are printed with their deltas but always carry `NotEvidence` and are excluded from the "established differences" count in the summary line. - [ ] Each window's `quiet` flag is reported for both sides; a `steady` pair where either side is not quiet logs the warning that its numbers describe a world being built. Deltas are still printed — `city` never goes quiet on this container, and suppressing it would delete the only ground measurement. - [ ] Unit tests in `perf_diff.rs`, over fixture `Report`s built in the test module (inline JSON, so the loader is exercised too — no new files, and no dependence on the repo root the way `the_shipped_manifest_parses` needs at `perf.rs:854`): - [ ] a real change — disjoint fill intervals, and a changed `draws` — is reported as a delta with the expected sign and magnitude; - [ ] a change inside the spread — overlapping fill intervals whose midpoints differ — is `NotEstablished`; - [ ] a window present in one report only is named in `only_in_baseline` / `only_in_current`; - [ ] a `frag_samples == 1` pair is `NotMeasured`; - [ ] a column `None` on one side is `NotMeasured`, not a delta from zero; - [ ] **a report diffed against itself yields no `Established` row.** - [ ] `docs/notes/performance-suite.md` gains a section saying which columns can carry a difference and which cannot, and why; `.cargo/config.toml`'s `cargo perf` comment mentions the flag. - [ ] The commit body names the value from `docs/direction.md` that settled the column classification (value 6, *ship the smallest thing a machine can judge* — counts and spreads are judgeable, timings on this host are not) — required anyway, since this is a `feat` touching more than five files. ## Verification ```bash cargo test -p cartopolis perf_diff # the unit tests above cargo fmt --check -p cartopolis # plus the other four owned crates cargo check -p cartopolis ``` End to end, on a host that can render (lavapipe here is enough — the measurements in `docs/notes/performance-suite.md` were taken on this container; the container this *refinement* pass runs in must not build): ```bash cargo perf --only orbit --perf-out /tmp/perf-a.json cargo perf --only orbit --perf-out /tmp/perf-b.json --perf-baseline /tmp/perf-a.json ``` `orbit` is the scenario that reliably goes quiet, so it is the one to smoke-test with. Two unchanged runs of it should produce no `Established` row; if they do, the diff is reporting run-to-run noise as a finding and the column that did it needs reclassifying before this lands. A `--features perf` run diffed against a non-`perf` one is the check that absent columns come back `NotMeasured`. Not checkable here or on this container at all: whether the classification holds on a real GPU. The Steam Deck runner (`deck-gpu`, see the `steamdeck-gpu-runner` note) is where a desktop-GPU pair of reports would come from. ## Out of scope - **Any exit code or gate.** A regression does not fail the run. Thresholds would have to be guessed, and `--expect` already exists for gating a scripted run. - **Diffing two saved reports without flying anything** (`--perf-diff a.json b.json`). The pure function makes it a few lines later; it is a second entry path into `lib.rs` and a separate ticket. - **Writing the diff to a file.** Log only. - **Recording a spread for any column other than fill** — e.g. per-frame min/max of frame time, which the frag sampler's shape would fit. Timings are not evidence here, so a spread on them buys nothing today; it becomes worth doing when a report comes off real hardware. - **Repeating a scenario N times within one run to get a cross-run spread.** That is a change to the measuring path, not the diff. - **Making `Window`/`GpuPass` fields `#[serde(default)]`** so an older baseline still parses. Deliberately not done: a missing column would silently read 0 and be reported as a large delta, which is worse than the parse error. A baseline from a build with different columns fails loudly, with the path in the message. - Any new scenario in `docs/perf.toml`. ## Open questions None. --- Branch: `feat/301-perf-baseline-diff` <details><summary>Original request</summary> From the maintainer's focus for this lane: > Improving performance on the steamdeck. keep it concrete and simple. use or extend harness tooling to find useful improvements. **cargo perf --baseline <report.json>: a diff that refuses a difference the spread cannot support** Same module as the ticket above and the half that makes it useful. Reads a previous report, matches windows by (scenario, phase), prints per-column deltas, and where a delta is smaller than the combined spread of the two runs reports it as not established rather than as a number. This is the tool every later ticket here is judged with, and it is what stops a fourth repetition of the sky-shell A/B that produced a clean-looking table saying turning the clouds off increased fill. Judged by: unit tests over two fixture reports — a real change reported as a delta, a change inside the spread reported as not established, and a window present in one report and not the other named rather than silently dropped. This is item 3 of 20 of the lane's queue. It is ordered by a person in the admin console, so build this one rather than the one you would have picked — and keep it the size it is. A ticket that turns out to be three tickets is one ticket: finish this part and say on it what the other two are. If the item is simply wrong — already done, impossible, or a bad idea against the values — say so here and close it. That is a result, not a failure. **Decide it yourself.** This ticket is not being watched, so a question asked here is a ticket that stops. The seven values at the top of `docs/direction.md` exist to settle exactly that kind of ambiguity: pick the reading they support, say in the commit body which one you applied, and build. Only a decision needing something nobody can derive from the repository — a credential, a licence somebody must accept, a choice about what the project is *for* — is a reason to stop. <sub>Filed by the autopilot. <!-- autopilot:focus-cargo-perf-baseline-report-json-a-diff-t --></sub> </details> <sub>🤖 Refined by the viberfox issue agent. Reply with **@agent refine** and what is wrong to have this rewritten.</sub>
Author
Collaborator

🤖 Refining this before anything gets built — reading the code, no branch, no commits. If it turns out I'd have to guess at the requirements I'll stop and ask instead.

🤖 Refining this before anything gets built — reading the code, no branch, no commits. If it turns out I'd have to guess at the requirements I'll stop and ask instead.
Author
Collaborator

🤖 Refined. The description above is now the spec; I'll pick it up for implementation on the next poll.

If the direction is wrong, say so now — @agent refine with a correction rewrites it, and agent:skip stops me.


🤖 viberfox issue agent on the VPS — 21 turns · $2.31 · session c7fd21a9-70e2-476e-bb32-4d466ef13b3f. Reply with @agent to continue this session.

🤖 **Refined.** The description above is now the spec; I'll pick it up for implementation on the next poll. If the direction is wrong, say so now — **@agent refine** with a correction rewrites it, and `agent:skip` stops me. --- <sub>🤖 viberfox issue agent on the VPS — 21 turns · $2.31 · session `c7fd21a9-70e2-476e-bb32-4d466ef13b3f`. Reply with **@agent** to continue this session.</sub>
Author
Collaborator

🤖 Picked this up — working on branch feat/301-perf-baseline-diff in a fresh worktree on the VPS. I'll report back here when I'm done.

Label an issue agent:skip to keep me off it, or driven to keep it in a session you are driving.

🤖 Picked this up — working on branch `feat/301-perf-baseline-diff` in a fresh worktree on the VPS. I'll report back here when I'm done. <sub>Label an issue `agent:skip` to keep me off it, or `driven` to keep it in a session you are driving.</sub>
Author
Collaborator

cargo perf can now compare a run against a previous one, and it refuses differences the two runs' own measured spread cannot support.

What changed

  • New file crates/cartopolis/src/systems/dev/perf_diff.rs — a pure function over two parsed reports (no Bevy, no filesystem), plus a logger. It matches windows by scenario and phase, and gives every column one of five verdicts.
  • New flag --perf-baseline <path> (--baseline is accepted as an alias). The suite runs and writes its report exactly as before, then logs the diff. The baseline file is read and parsed at startup, so a wrong path fails before the first scenario is flown. Passing it without --perf is refused by name.
  • docs/notes/performance-suite.md gained a "Comparing two runs" section, and the cargo perf comment in .cargo/config.toml mentions the flag.

The classification is the substance of it. Draw calls, triangle and entity counts, mesh slabs and the three residency numbers are compared exactly. Per-pass fill is compared as the interval the report already records — smallest and largest fragment count seen across the window — so two ranges that overlap come back as not established rather than as a number, and where they are disjoint the reported delta is the gap between the nearest edges, i.e. the smallest change both runs allow. Fewer than two samples on either side is not measured. Frame rate, frame time, per-pass GPU and CPU times, the process percentages and the per-window streaming traffic are printed with their deltas but marked not evidence and never counted. A column absent on one side is not measured, never a delta from zero. The reason for the split is value 6 of docs/direction.md — counts and spreads are judgeable on this hardware, timings are not — and the commit body says so.

How I verified it

  • cargo test -p cartopolis — 951 passed, 0 failed. That includes 12 new tests in perf_diff: a real change reported with the right sign and size, a change inside the spread reported as not established, a window present in only one report named rather than dropped, a single-sample pair not measured, an absent column not measured, and a report diffed against itself producing no established row.
  • cargo test -p cartopolis_geo — 255 passed, 0 failed.
  • cargo fmt --check clean on all five owned crates.

One test caught something worth knowing: the theme suite scans every short string literal in the tree for characters the bundled font cannot draw, and it rejected the → I had used in the "this meta field differs" message. It is -> now.

What is not verified

I did not run the suite end to end. Doing that means launching the renderer, and this session's standing instruction is not to attempt rendered runs here. So the flag has never actually flown a scenario and diffed a real report — only the pure function is exercised. The check to run on a machine that can render is the one in the ticket:

cargo perf --only orbit --perf-out /tmp/perf-a.json
cargo perf --only orbit --perf-out /tmp/perf-b.json --perf-baseline /tmp/perf-a.json

Two unchanged runs of orbit should produce no established row. If they do, a column is reporting run-to-run noise as a finding and needs reclassifying before this is trusted. Whether the classification holds on a real graphics processor is also open; that needs a pair of reports off the Steam Deck runner.

For you to decide

Nothing is blocked. Two things the ticket deliberately left out, if you want them as follow-ups: diffing two saved reports without flying anything (--perf-diff a.json b.json — the pure function makes it a few lines, but it is a second way into the command line), and recording a spread for columns other than fill, which only becomes worth doing once reports come off real hardware.

Branch feat/301-perf-baseline-diff (pushed)

795f02b feat(perf): diff a run against a baseline, and refuse what the spread cannot carry

🤖 viberfox issue agent on the VPS — 284 turns · $28.70 · session 54cceff7-840b-4846-b360-ad33faa3f323. Reply with @agent to continue this session.

`cargo perf` can now compare a run against a previous one, and it refuses differences the two runs' own measured spread cannot support. ## What changed - **New file `crates/cartopolis/src/systems/dev/perf_diff.rs`** — a pure function over two parsed reports (no Bevy, no filesystem), plus a logger. It matches windows by scenario and phase, and gives every column one of five verdicts. - **New flag `--perf-baseline <path>`** (`--baseline` is accepted as an alias). The suite runs and writes its report exactly as before, then logs the diff. The baseline file is read and parsed *at startup*, so a wrong path fails before the first scenario is flown. Passing it without `--perf` is refused by name. - `docs/notes/performance-suite.md` gained a "Comparing two runs" section, and the `cargo perf` comment in `.cargo/config.toml` mentions the flag. The classification is the substance of it. Draw calls, triangle and entity counts, mesh slabs and the three residency numbers are compared exactly. Per-pass fill is compared as the interval the report already records — smallest and largest fragment count seen across the window — so two ranges that overlap come back as **not established** rather than as a number, and where they are disjoint the reported delta is the gap between the nearest edges, i.e. the smallest change both runs allow. Fewer than two samples on either side is **not measured**. Frame rate, frame time, per-pass GPU and CPU times, the process percentages and the per-window streaming traffic are printed with their deltas but marked **not evidence** and never counted. A column absent on one side is **not measured**, never a delta from zero. The reason for the split is value 6 of `docs/direction.md` — counts and spreads are judgeable on this hardware, timings are not — and the commit body says so. ## How I verified it - `cargo test -p cartopolis` — 951 passed, 0 failed. That includes 12 new tests in `perf_diff`: a real change reported with the right sign and size, a change inside the spread reported as not established, a window present in only one report named rather than dropped, a single-sample pair not measured, an absent column not measured, and a report diffed against itself producing no established row. - `cargo test -p cartopolis_geo` — 255 passed, 0 failed. - `cargo fmt --check` clean on all five owned crates. One test caught something worth knowing: the theme suite scans every short string literal in the tree for characters the bundled font cannot draw, and it rejected the `→` I had used in the "this meta field differs" message. It is `->` now. ## What is not verified I did not run the suite end to end. Doing that means launching the renderer, and this session's standing instruction is not to attempt rendered runs here. So the flag has never actually flown a scenario and diffed a real report — only the pure function is exercised. The check to run on a machine that can render is the one in the ticket: ```bash cargo perf --only orbit --perf-out /tmp/perf-a.json cargo perf --only orbit --perf-out /tmp/perf-b.json --perf-baseline /tmp/perf-a.json ``` Two unchanged runs of `orbit` should produce no established row. If they do, a column is reporting run-to-run noise as a finding and needs reclassifying before this is trusted. Whether the classification holds on a real graphics processor is also open; that needs a pair of reports off the Steam Deck runner. ## For you to decide Nothing is blocked. Two things the ticket deliberately left out, if you want them as follow-ups: diffing two saved reports without flying anything (`--perf-diff a.json b.json` — the pure function makes it a few lines, but it is a second way into the command line), and recording a spread for columns other than fill, which only becomes worth doing once reports come off real hardware. **Branch** `feat/301-perf-baseline-diff` (pushed) ``` 795f02b feat(perf): diff a run against a baseline, and refuse what the spread cannot carry ``` --- <sub>🤖 viberfox issue agent on the VPS — 284 turns · $28.70 · session `54cceff7-840b-4846-b360-ad33faa3f323`. Reply with **@agent** to continue this session.</sub>
Author
Collaborator

🤖 Marked agent:done without a CI verdict on 795f02bd — no workflow reported on this commit. The branch is waiting for review; check the run yourself before landing it.

🤖 Marked `agent:done` **without a CI verdict** on `795f02bd` — no workflow reported on this commit. The branch is waiting for review; check the run yourself before landing it.
Author
Collaborator

🤖 Not shipping this one — the branch conflicts with main (another ticket landed first).

Rebase feat/301-perf-baseline-diff onto main, resolve, push, and put the ship label back to land it.

🤖 **Not shipping this one — the branch conflicts with `main`** (another ticket landed first). Rebase `feat/301-perf-baseline-diff` onto `main`, resolve, push, and put the `ship` label back to land it.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jeroen/cartopolis#301
No description provided.