From 6d4b6c50a86864a53be9e4f5d0199374a0a7b3f6 Mon Sep 17 00:00:00 2001 From: Christophe Pettus Date: Wed, 29 Jul 2026 14:19:40 -0700 Subject: [PATCH] bench: restore the rows a removed cache ceiling was still excluding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BENCH-5. micro_grid skipped cells and dropped two whole rows on a premise that no longer holds. TX_BUDGET_BYTES was justified as avoiding "Chisel's CacheFull ... cache hard ceiling is ~16 MB", and update-1000pertx / delete-1000pertx were never registered because "1000 random updates/deletes pin ~1000 distinct dirty data pages, exceeding Chisel's 2048-page cache ceiling. The cells are not measurable under default cache settings." Both halves are stale. CACHE_SIZE_PAGES is 256, not 2048, and the strict cache ceiling itself went away when the bench engines gained the spillway at production-default scale — runner.rs already says so ("there is no strict cache-page ceiling on transaction size") and chisel_engine.rs sets `spillway_max_bytes(cache_max_bytes * 1024)`. Measured every excluded cell against the current engine rather than assuming either way. All of them complete: allocate-1000pertx 16KB 0.71 s update-1000pertx 16KB 1.27 s allocate-1000pertx 128KB 2.70 s delete-1000pertx 16KB 0.80 s allocate-1000pertx 1MB 18.51 s (smaller sizes: ~0.5 s) So capacity is no longer the constraint — wall clock is, for exactly one cell. TX_BUDGET_BYTES is kept but reframed as a bench-runtime budget and raised to 128 MiB: that admits the 16KB and 128KB columns, which were losing real coverage for no reason, and still excludes 1MB x 1000, which at ~18.5 s per iteration across five engine modes would add hours to every run. Deleting the constant outright, as the issue suggests, is the one part of its direction not followed, and the comment now says why with the measurement behind it. update-1000pertx and delete-1000pertx are registered. BENCH-6. The module header claimed a "270-cell micro grid ... 9 row groups" while six rows registered 165 cells, so anyone checking that a run completed would conclude 105 cells had silently failed. The header now states 230 cells across 8 rows with the per-row breakdown, and runner.rs's "8 of 9 rows: rows 1, 2, 4-9" is replaced by what actually distinguishes them (every row except read-warm, which reuses one engine and has its own capture function). Verified 230 both by deriving it from the registration rules — the same derivation reproduces the old 165 against the old constants — and by counting `cargo bench --bench micro_grid -- --list`. The dead `seed_for` arms for the two rows are now live again rather than removed. BENCH-7. cross-engine.md's methodology footer told readers "Each engine takes a single fsync per commit". Chisel performs three (pre-drain, data pages, superblock), which the bench's own runner comments state. This renderer's output is described as suitable for the README and release notes, so the claim understated Chisel's per-commit cost by 3x in published material and implied the three commit paths were equivalent. The footer now states each engine's protocol, since that asymmetry is what the throughput table is read against. Closes #100. --- bench/benches/micro_grid.rs | 65 +++++++++++++++++++----- bench/src/runner.rs | 6 ++- bench/src/summary/discover.rs | 2 +- bench/src/summary/render_cross_engine.rs | 11 +++- 4 files changed, 65 insertions(+), 19 deletions(-) diff --git a/bench/benches/micro_grid.rs b/bench/benches/micro_grid.rs index 2f4ef70..ceb26d7 100644 --- a/bench/benches/micro_grid.rs +++ b/bench/benches/micro_grid.rs @@ -1,9 +1,21 @@ -// Bench binary: the 270-cell micro grid. Iterates EngineMode::ALL × SIZES -// × the 9 row groups, registering each cell as a Criterion benchmark +// Bench binary: the micro grid. Iterates EngineMode::ALL (5) × SIZES (6) +// × 8 row groups, registering each cell as a Criterion benchmark // inside a per-row BenchmarkGroup with Throughput::Elements(N) for // per-op normalization. Aux metrics (file-size delta + Chisel internal // counter deltas) are captured per cell into bench/results/aux_metrics.jsonl. // +// 230 cells, not the 5 × 6 × 8 = 240 the dimensions suggest: the two +// 1000-per-tx write rows skip their 1MB column on the bench-runtime budget +// (see TX_BUDGET_BYTES). Rows and their cell counts: +// +// allocate-1pertx 30 read-warm 30 update-1pertx 30 +// allocate-1000pertx 25 read-cold 30 update-1000pertx 25 +// delete-1pertx 30 delete-1000pertx 30 +// +// The count is worth keeping honest: a reader checking that a run completed +// compares against it, and it feeds the raw-archive size estimate in +// summary/discover.rs. +// // The three Criterion-shaped cell-runner helpers (run_*_cell) live here // rather than in src/runner.rs because Criterion is in [dev-dependencies] // and src/ code can't import dev-deps. The helpers are private. @@ -47,11 +59,30 @@ const SIZES: [(usize, &str, usize); 6] = [ (1_048_576, "1MB", 25), ]; -/// Per-transaction byte budget. Cells where `ops_per_tx * size_bytes` exceeds -/// this are skipped to avoid Chisel's CacheFull at large 1000-per-tx writes -/// (cache hard ceiling is ~16 MB; 8 MB leaves headroom for COW overhead). -/// Affects allocate-1000pertx and update-1000pertx at sizes ≥ 16KB. -const TX_BUDGET_BYTES: usize = 8 * 1024 * 1024; +/// Per-transaction byte budget for the 1000-per-tx rows. Cells where +/// `ops_per_tx * size_bytes` exceeds this are skipped. +/// +/// This is a BENCH-RUNTIME budget, not a Chisel capacity limit. It used to be +/// the latter — 8 MiB, justified as avoiding "Chisel's CacheFull ... cache +/// hard ceiling is ~16 MB" — but that ceiling was removed when the bench +/// engines gained the spillway at production-default scale (1024 × the cache +/// budget; see `ChiselEngine::open_file` and `POPULATE_TX_BUDGET_BYTES`). +/// Measured against the current engine at `CACHE_SIZE_PAGES`, every cell the +/// old budget excluded completes: +/// +/// allocate-1000pertx 16KB (16 MB/tx) 0.71 s +/// allocate-1000pertx 128KB (128 MB/tx) 2.70 s +/// allocate-1000pertx 1MB (1 GB/tx) 18.51 s +/// +/// So capacity is no longer the constraint; wall clock is. 128 MiB admits the +/// 16KB and 128KB columns, which were losing real coverage for no reason, and +/// still excludes 1MB × 1000 — at ~18.5 s per iteration that one cell would +/// add hours to every micro-grid run across the five engine modes. +/// +/// Affects allocate-1000pertx and update-1000pertx at 1MB only. +/// delete-1000pertx does not consult this: deleting frees rather than writes +/// `ops * size` bytes, and it clamps its op count to `prepop_count` anyway. +const TX_BUDGET_BYTES: usize = 128 * 1024 * 1024; /// Hardcoded per-row seeds for workload determinism. Hardcoded rather /// than derived from row names because Rust's DefaultHasher randomizes @@ -171,7 +202,7 @@ fn bench_row_allocate_n_per_tx( for (size_bytes, size_label, _) in SIZES { if ops_per_tx * size_bytes > TX_BUDGET_BYTES { - continue; // skip cells too large to fit in Chisel's cache + continue; // over the bench-runtime budget; see TX_BUDGET_BYTES } let workload = gen_allocate(ops_per_tx, size_bytes); for mode in EngineMode::ALL { @@ -298,7 +329,7 @@ fn bench_row_update_n_per_tx( for (size_bytes, size_label, prepop_count) in SIZES { if ops_per_tx * size_bytes > TX_BUDGET_BYTES { - continue; // skip cells too large to fit in Chisel's cache + continue; // over the bench-runtime budget; see TX_BUDGET_BYTES } let workload = gen_update_random(seed_for(group_name), prepop_count, ops_per_tx, size_bytes); @@ -409,12 +440,18 @@ fn micro_grid(c: &mut Criterion) { bench_row_read_warm(c, &mut aux); bench_row_read_cold(c, &mut aux); bench_row_update_n_per_tx(c, &mut aux, "update-1pertx", 1); - // update-1000pertx and delete-1000pertx skipped: 1000 random - // updates/deletes pin ~1000 distinct dirty data pages, exceeding - // Chisel's 2048-page cache ceiling. The cells are not measurable - // under default cache settings; revisit with a larger - // CACHE_SIZE_PAGES in a future PR. + // update-1000pertx and delete-1000pertx were previously not registered at + // all, on the rationale that "1000 random updates/deletes pin ~1000 + // distinct dirty data pages, exceeding Chisel's 2048-page cache ceiling" + // and so "are not measurable under default cache settings". Both halves of + // that are stale: CACHE_SIZE_PAGES is 256, not 2048, and the strict cache + // ceiling itself is gone now that the bench engines run with the spillway + // at production-default scale. Measured at 1000 ops per tx, largest size + // each row admits: update 1.27 s, delete 0.80 s. They are measurable, so + // they are registered. + bench_row_update_n_per_tx(c, &mut aux, "update-1000pertx", 1000); bench_row_delete_n_per_tx(c, &mut aux, "delete-1pertx", 1); + bench_row_delete_n_per_tx(c, &mut aux, "delete-1000pertx", 1000); } criterion_group!(benches, micro_grid); diff --git a/bench/src/runner.rs b/bench/src/runner.rs index 761dffc..3003544 100644 --- a/bench/src/runner.rs +++ b/bench/src/runner.rs @@ -402,8 +402,10 @@ pub fn populate_snapshot( Ok(PopulatedSnapshot { file, ids }) } -/// Capture per-cell aux metrics for the snapshot-restore-style rows -/// (8 of 9 rows: rows 1, 2, 4–9). One calibration iteration: copy +/// Capture per-cell aux metrics for the snapshot-restore-style rows — +/// every row except read-warm, which reuses one engine across iterations +/// and so has its own `capture_aux_metrics_warm_read`. One calibration +/// iteration: copy /// the snapshot, open the engine, snapshot counters + file size, /// drive the workload, snapshot again, return deltas. /// diff --git a/bench/src/summary/discover.rs b/bench/src/summary/discover.rs index 6db5284..1347ce9 100644 --- a/bench/src/summary/discover.rs +++ b/bench/src/summary/discover.rs @@ -273,7 +273,7 @@ fn parse_sample_json(path: &Path) -> Result, Box std::io::Result<()> { for entry in walkdir::WalkDir::new(criterion_dir) .into_iter() diff --git a/bench/src/summary/render_cross_engine.rs b/bench/src/summary/render_cross_engine.rs index 3c3752d..56e6157 100644 --- a/bench/src/summary/render_cross_engine.rs +++ b/bench/src/summary/render_cross_engine.rs @@ -142,8 +142,15 @@ fn render_methodology_footer() -> &'static str { [`summary.md`](summary.md) in the same directory for the full per-cell\n\ detail (p50, p95, total wall clock, file-size delta, and Chisel-internal\n\ counter snapshots) and [the architecture doc](../../../ARCHITECTURE.md#benchmark-infrastructure)\n\ - for the bench-suite layout and workload definitions. Each engine takes\n\ - a single fsync per commit through the disk write cache; numbers depend\n\ + for the bench-suite layout and workload definitions.\n\ + \n\ + The three engines do not perform equal work per commit, and the\n\ + throughput numbers should be read against that. Chisel's shadow-paging\n\ + protocol performs **three** fsyncs per commit (pre-drain flush, data\n\ + pages, then the superblock swap); redb commits at\n\ + `Durability::Immediate`; SQLite runs in WAL mode with\n\ + `synchronous=FULL`, plus `fullfsync=ON` so that on macOS it issues the\n\ + same `F_FULLFSYNC` Chisel does rather than a plain fsync. Numbers depend\n\ on the platform's storage stack and are not portable across machine\n\ classes.\n" }