diff --git a/bench/src/summary/discover.rs b/bench/src/summary/discover.rs index de4adbf..6db5284 100644 --- a/bench/src/summary/discover.rs +++ b/bench/src/summary/discover.rs @@ -1,8 +1,12 @@ // Filesystem discovery layer for the summary post-processor. Walks -// target/criterion////sample.json files, parses +// target/criterion////new/sample.json files, parses // aux_metrics.jsonl, joins them by (row, mode, size) key into Cell // values. Cells with missing-on-one-side data carry None for the // missing field — renderers handle the partial-state cases explicitly. +// +// The `new/` component is Criterion's own, not ours: it writes the +// just-finished run to `new/` and retains the previous one under `base/`. +// Only `new/` is read here; see the walk in `discover_cells`. use crate::runner::ChiselCountersDelta; use crate::summary::format::percentile_linear_interp; @@ -146,23 +150,37 @@ pub fn discover_cells( } // Step 2: walk criterion_dir to find sample.json leaves. - // Each leaf is at depth 4 (criterion_dir/row/mode/size/sample.json). + // + // Each leaf is at depth 5: criterion_dir/row/mode/size/{new,base}/sample.json. + // Criterion 0.5 interposes that last directory — `new/` is the run that just + // finished, `base/` is the previous run kept for its own change-detection. + // We want `new/` only: `base/` is a stale duplicate under the same + // (row, mode, size) key, and counting it would emit two Cells per cell. + // + // The `new` check is the load-bearing filter; the depth bounds only + // restate the grid's shape (a row/mode/size triple is exactly what the + // path walk-back below needs, and what a Cell is keyed by). let mut cells: Vec = Vec::new(); let mut sample_paths_seen = 0usize; for entry in walkdir::WalkDir::new(criterion_dir) - .min_depth(4) - .max_depth(4) + .min_depth(5) + .max_depth(5) .into_iter() .filter_map(|e| e.ok()) { if entry.file_name() != "sample.json" { continue; } + + // Walk the path back: file -> new dir -> size dir -> mode dir -> row dir. + let run_dir = entry.path().parent().unwrap(); + if run_dir.file_name().unwrap_or_default() != "new" { + continue; + } sample_paths_seen += 1; - // Walk the path back: file -> size dir -> mode dir -> row dir. - let size_dir = entry.path().parent().unwrap(); + let size_dir = run_dir.parent().unwrap(); let mode_dir = size_dir.parent().unwrap(); let row_dir = mode_dir.parent().unwrap(); @@ -373,6 +391,21 @@ mod tests { cells.len() ); + // The chisel-strict 32B fixture has BOTH a new/ and a base/ sample.json, + // mirroring a real Criterion tree after a second run. Exactly one cell + // must come out of it, and its numbers must be new/'s (base/ carries + // times 10x larger, so a wrong pick shows up as p50 30000). + assert_eq!( + cells + .iter() + .filter(|c| c.row == "allocate-1pertx" + && c.mode == "chisel-strict" + && c.size == "32B") + .count(), + 1, + "base/ must not produce a second cell under the same key" + ); + let chisel_cell = cells .iter() .find(|c| c.row == "allocate-1pertx" && c.mode == "chisel-strict" && c.size == "32B") diff --git a/bench/src/summary/mod.rs b/bench/src/summary/mod.rs index 7908315..46cda48 100644 --- a/bench/src/summary/mod.rs +++ b/bench/src/summary/mod.rs @@ -1,5 +1,5 @@ // Markdown summary post-processor module. Reads PR 4b's bench output -// (target/criterion////sample.json + bench/results/aux_metrics.jsonl) +// (target/criterion////new/sample.json + bench/results/aux_metrics.jsonl) // and produces three artifacts under bench/results//: // // summary.md — human-readable per-row tables diff --git a/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/base/sample.json b/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/base/sample.json new file mode 100644 index 0000000..9c5c074 --- /dev/null +++ b/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/base/sample.json @@ -0,0 +1,4 @@ +{ + "iters": [10.0, 10.0, 10.0, 10.0, 10.0], + "times": [100000.0, 200000.0, 300000.0, 400000.0, 500000.0] +} diff --git a/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/estimates.json b/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/new/estimates.json similarity index 100% rename from bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/estimates.json rename to bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/new/estimates.json diff --git a/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/sample.json b/bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/new/sample.json similarity index 100% rename from bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/sample.json rename to bench/tests/fixtures/criterion/allocate-1pertx/chisel-strict/32B/new/sample.json diff --git a/bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/estimates.json b/bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/new/estimates.json similarity index 100% rename from bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/estimates.json rename to bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/new/estimates.json diff --git a/bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/sample.json b/bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/new/sample.json similarity index 100% rename from bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/sample.json rename to bench/tests/fixtures/criterion/allocate-1pertx/redb-strict/32B/new/sample.json diff --git a/bench/tests/fixtures/criterion/corrupt/chisel-strict/32B/sample.json b/bench/tests/fixtures/criterion/corrupt/chisel-strict/32B/new/sample.json similarity index 100% rename from bench/tests/fixtures/criterion/corrupt/chisel-strict/32B/sample.json rename to bench/tests/fixtures/criterion/corrupt/chisel-strict/32B/new/sample.json diff --git a/bench/tests/summarize_smoke.rs b/bench/tests/summarize_smoke.rs index 61ddd9b..0e958b3 100644 --- a/bench/tests/summarize_smoke.rs +++ b/bench/tests/summarize_smoke.rs @@ -80,10 +80,13 @@ fn summarize_smoke_runs_against_fixtures() { "summary.md missing ycsb-a row" ); + // `new/` is Criterion's own directory level, and `copy_raw_archive` + // mirrors the source tree verbatim — so the archived path carries it too. let chisel_raw = raw_dir .join("allocate-1pertx") .join("chisel-strict") - .join("32B"); + .join("32B") + .join("new"); assert!( chisel_raw.join("sample.json").exists(), "raw chisel-strict sample.json missing"