From 4251888095bd9c4c49e98245b1ce8a8d242a42f5 Mon Sep 17 00:00:00 2001 From: Mohamed Mansour Date: Mon, 24 Aug 2026 16:56:52 -0700 Subject: [PATCH] fix(xtask): run Criterion benchmarks per target `cargo xtask bench all` mapped every Criterion target onto a single `cargo bench --workspace`. Cargo forwards everything after `--` to every benchable target in the workspace, including the libtest unit-test harnesses of libraries and binaries. Those harnesses reject Criterion's baseline flags, so the run aborted on the first one it reached and no baseline was ever recorded. Walk an explicit CRITERION_BENCHES table instead and invoke each target on its own, so baseline flags only ever reach a real Criterion binary. Keep the run fail-fast so a broken declared benchmark stays visible. Add `cargo xtask bench lazy-hydration` for the existing offscreen hydration matrix, mapping baseline flags onto the WEBUI_LAZY_HYDRATION_* env vars its spec already consumes, and tighten the shared lazy bench driver/fixtures to assert Toggle/Delete wiring per handler. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2605df0b-c217-4047-9413-49be787b1786 --- BENCHMARKS.md | 42 +++ .../tests/lib/lazy-driver.ts | 17 +- .../tests/lib/lazy-fixtures.ts | 7 +- xtask/src/main.rs | 273 +++++++++++++++++- 4 files changed, 332 insertions(+), 7 deletions(-) diff --git a/BENCHMARKS.md b/BENCHMARKS.md index d79c1b519..4f5a9fb9c 100644 --- a/BENCHMARKS.md +++ b/BENCHMARKS.md @@ -19,6 +19,7 @@ how to compare results. | `cargo xtask bench streaming-resource` | example | ~30 s | exact alloc count + bytes + getrusage CPU + RSS | proving zero-alloc claims; allocation regression hunting | | `cargo xtask bench streaming-e2e-ttfb` | example | ~10 s | HTTP-level TTFB / TTLB through actix | confirming wire-level streaming win | | `cargo xtask bench streaming-browser` | Playwright | ~30 s | real Chromium TTFB / FCP / LCP / DCL / load | proving user-perceived paint improvement | +| `cargo xtask bench lazy-hydration` | Playwright + CDP | ~1 min | hydration, heap, rendering, and trace metrics at 10/100/1000 rows | validating offscreen work reduction | | `cargo xtask bench full` (= `streaming-all`) | suite | ~3 min | runs all four streaming-related benches in sequence | full streaming evidence pack for a PR | ## The before/after workflow @@ -41,6 +42,7 @@ Baselines are stored at `target/bench-baselines/`: * `streaming-resource-.json` — alloc + RSS + CPU table * `e2e-ttfb-.json` — HTTP TTFB/TTLB table * `browser-.json` — browser metrics table +* `browser-lazy-hydration-.json` — offscreen rendering/hydration matrix * `node-addon-.json` — Node/V8/N-API latency table * `target/criterion//` — criterion's native baseline directory tree @@ -48,6 +50,27 @@ Baselines are stored at `target/bench-baselines/`: The compare phase prints a Δ%-table for every row. Negative Δ% = improvement; positive = regression. +### Criterion baselines are recorded per target + +`cargo xtask bench all` invokes each Criterion target separately — +`cargo bench -p --bench -- --save-baseline NAME` — rather +than a single `cargo bench --workspace`. A workspace-wide run forwards +Criterion's flags to *every* benchable target, including the libtest +unit-test harnesses of libraries and binaries, which abort with +`error: Unrecognized option: 'save-baseline'` before a single baseline is +written. + +Because each target runs on its own, criterion writes one baseline directory +per bench: + +```bash +cargo xtask bench all --save-baseline before +# target/criterion///before/ for every Criterion target +``` + +The target list lives in `CRITERION_BENCHES` in `xtask/src/main.rs`. Add new +`benches/*.rs` harnesses there so `bench all` and its baselines pick them up. + ### Threshold guidance | Source | Treat as noise | Treat as signal | @@ -155,6 +178,25 @@ the workload. The first-callback metric is in-process; it is not HTTP TTFB. The runner verifies JSON-string, object-state, and streamed output are byte-identical before collecting samples. +### `lazy-hydration` (offscreen hydration matrix) + +`cargo xtask bench lazy-hydration` drives the Playwright + CDP lazy-hydration +matrix in `examples/integration/streaming-browser-bench` across the `eager`, +`lazy-hydrate`, and `lazy-render` modes at 10/100/1000 rows. It reports bundle +init cost, hydration CPU, hydrated-root and listener counts, JS heap, rendering +trace metrics, and validated interaction counts. + +```bash +cargo xtask bench lazy-hydration --save-baseline before +# … change … +cargo xtask bench lazy-hydration --baseline before +``` + +The command maps its baseline flags onto the `WEBUI_LAZY_HYDRATION_SAVE` and +`WEBUI_LAZY_HYDRATION_COMPARE` env vars consumed by the spec, and writes +`target/bench-baselines/browser-lazy-hydration-.json`. See the bench +README for run-count, mode-subset, and framework-source overrides. + ## Recommended PR workflow For any change touching `crates/webui/src/streaming.rs` or its diff --git a/examples/integration/streaming-browser-bench/tests/lib/lazy-driver.ts b/examples/integration/streaming-browser-bench/tests/lib/lazy-driver.ts index 28a4d6180..f195ded0a 100644 --- a/examples/integration/streaming-browser-bench/tests/lib/lazy-driver.ts +++ b/examples/integration/streaming-browser-bench/tests/lib/lazy-driver.ts @@ -68,6 +68,8 @@ export async function runLazyHydration(mode: LazyMode): Promise __benchListenerCount?: number; __benchPeakHeap?: number; __benchInteractionCount?: number; + __benchRemoveCount?: number; + __benchToggleCount?: number; __defineBenchTodo(): void; }; @@ -99,6 +101,8 @@ export async function runLazyHydration(mode: LazyMode): Promise win.__benchHydratedCount = 0; win.__benchListenerCount = 0; win.__benchInteractionCount = 0; + win.__benchRemoveCount = 0; + win.__benchToggleCount = 0; const baseHeap = heapSize(); win.__benchPeakHeap = baseHeap ?? 0; const roots = document.getElementsByTagName('bench-todo-item'); @@ -150,9 +154,10 @@ export async function runLazyHydration(mode: LazyMode): Promise } const firstButton = roots[0]?.querySelector('button'); + const firstDeleteButton = roots[0]?.querySelectorAll('button')[1]; const lastRoot = roots[roots.length - 1] as HTMLElement | undefined; const lastButton = lastRoot?.querySelector('button'); - if (!firstButton || !lastRoot || !lastButton) { + if (!firstButton || !firstDeleteButton || !lastRoot || !lastButton) { throw new Error('lazy hydration benchmark roots are incomplete'); } const dormantContentSkipped = typeof lastButton.checkVisibility === 'function' @@ -168,6 +173,14 @@ export async function runLazyHydration(mode: LazyMode): Promise lastButton.click(); const dormantInteractionMs = performance.now() - dormantStarted; const afterDormant = win.__benchHydratedCount ?? 0; + const interactionCount = win.__benchInteractionCount ?? 0; + firstDeleteButton.click(); + if ( + win.__benchToggleCount !== 2 + || win.__benchRemoveCount !== 1 + ) { + throw new Error('lazy hydration benchmark wired Toggle/Delete incorrectly'); + } return { bundleInitMs: win.__benchBundleInitMs ?? 0, @@ -196,7 +209,7 @@ export async function runLazyHydration(mode: LazyMode): Promise dormantInteractionMs, dormantInteractionHydrated: afterDormant > beforeDormant, dormantContentSkipped, - interactionCount: win.__benchInteractionCount ?? 0, + interactionCount, liveRootCount: roots.length, }; } diff --git a/examples/integration/streaming-browser-bench/tests/lib/lazy-fixtures.ts b/examples/integration/streaming-browser-bench/tests/lib/lazy-fixtures.ts index 2ee42e790..deb7e432b 100644 --- a/examples/integration/streaming-browser-bench/tests/lib/lazy-fixtures.ts +++ b/examples/integration/streaming-browser-bench/tests/lib/lazy-fixtures.ts @@ -5,6 +5,7 @@ import { build } from 'esbuild'; import { gzipSync } from 'node:zlib'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import type { TemplateMeta } from '../../../../../packages/webui-framework/src/template-types.js'; const here = dirname(fileURLToPath(import.meta.url)); const FRAMEWORK_SRC = process.env.WEBUI_LAZY_HYDRATION_FRAMEWORK_SRC @@ -32,7 +33,7 @@ export const ITEM_COUNTS = [10, 100, 1_000] as const; */ export type LazyBenchMode = 'eager' | 'lazy-hydrate' | 'lazy-render'; -const TODO_TEMPLATE = { +const TODO_TEMPLATE: TemplateMeta = { h: '
', tr: ['title', 'description', 'priority', 'due'], tx: [ @@ -47,7 +48,7 @@ const TODO_TEMPLATE = { ['remove', [], 8], ]], ], -} as const; +}; function entrySource(mode: LazyBenchMode): string { const optionalImport = mode === 'eager' @@ -86,10 +87,12 @@ class BenchTodoItem extends WebUIElement { toggle() { window.__benchInteractionCount = (window.__benchInteractionCount || 0) + 1; + window.__benchToggleCount = (window.__benchToggleCount || 0) + 1; } remove() { window.__benchInteractionCount = (window.__benchInteractionCount || 0) + 1; + window.__benchRemoveCount = (window.__benchRemoveCount || 0) + 1; } } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index e1ba68c56..7037a4397 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -135,9 +135,10 @@ fn usage() -> ExitCode { docs Build the documentation site\n \ bench [-- ] [--save-baseline NAME | --baseline NAME]\n \ Criterion: parser, handler, protocol, expressions, state, contact-book, streaming, all\n \ - Integration: node-addon, streaming-resource, streaming-e2e-ttfb, streaming-browser\n \ + Integration: node-addon, streaming-resource, streaming-e2e-ttfb, streaming-browser, lazy-hydration\n \ Streaming suite: streaming-all/full\n \ Baselines: --save-baseline NAME records, --baseline NAME compares\n \ + 'all' runs each Criterion target separately so baselines are recorded per target\n \ dev Run example app in dev mode (server + client watch concurrently)\n \ e2e [--update-snapshots] Run Playwright E2E tests for all example apps\n \ e2e-approve [run-id] Download CI screenshot baselines and apply locally\n \ @@ -187,6 +188,7 @@ fn bench(target: Option<&str>, extra_args: &[&str]) -> ExitCode { Some("streaming-resource") => bench_resource(save_baseline, compare_baseline), Some("streaming-e2e-ttfb") => bench_e2e_ttfb(save_baseline, compare_baseline), Some("streaming-browser") => bench_browser(save_baseline, compare_baseline), + Some("lazy-hydration") => bench_lazy_hydration(save_baseline, compare_baseline), Some("node-addon") | Some("webui-node") | Some("microsoft-webui-node") => { bench_node_addon(save_baseline, compare_baseline) } @@ -218,6 +220,7 @@ fn bench(target: Option<&str>, extra_args: &[&str]) -> ExitCode { } ExitCode::SUCCESS } + Some("all") | None => bench_all_criterion(&criterion_args, save_baseline, compare_baseline), _ => { // Criterion path (existing behaviour). Pass baseline flags // through as criterion's native flags. @@ -258,7 +261,9 @@ fn bench(target: Option<&str>, extra_args: &[&str]) -> ExitCode { args.push("streaming_bench".into()); } Some("all") | None => { - args.push("--workspace".into()); + // Handled by `bench_all_criterion` before reaching this arm. + eprintln!("bench target was not resolved"); + return ExitCode::FAILURE; } Some(other) => { eprintln!( @@ -266,7 +271,8 @@ fn bench(target: Option<&str>, extra_args: &[&str]) -> ExitCode { Criterion targets: parser, handler, protocol, expressions, state, \ contact-book, streaming, all.\n\ Integration targets: node-addon, streaming-resource, \ - streaming-e2e-ttfb, streaming-browser, streaming-all (= full)." + streaming-e2e-ttfb, streaming-browser, lazy-hydration, \ + streaming-all (= full)." ); return ExitCode::FAILURE; } @@ -302,6 +308,100 @@ fn bench(target: Option<&str>, extra_args: &[&str]) -> ExitCode { } } +/// Every Criterion benchmark target in the workspace, as +/// `(cargo package, bench target)` pairs. +/// +/// `cargo xtask bench all` walks this table and invokes each target +/// individually. Running `cargo bench --workspace` instead would forward +/// Criterion flags such as `--save-baseline` to *every* benchable target, +/// including the libtest unit-test harnesses of libs and binaries, which +/// reject them with `Unrecognized option: 'save-baseline'` before any +/// baseline is recorded. +const CRITERION_BENCHES: &[(&str, &str)] = &[ + ("microsoft-webui-parser", "parser_bench"), + ("microsoft-webui-handler", "handler_bench"), + ("microsoft-webui-handler", "bootstrap_state_bench"), + ("microsoft-webui-handler", "streaming_hydration_bench"), + ("microsoft-webui-protocol", "protocol_bench"), + ("microsoft-webui-expressions", "expressions_bench"), + ("microsoft-webui-state", "state_bench"), + ("microsoft-webui-ffi", "protocol_bench"), + ("microsoft-webui", "contact_book_bench"), + ("microsoft-webui", "streaming_bench"), + ("microsoft-webui", "component_assets_bench"), +]; + +fn bench_all_criterion( + criterion_args: &[&str], + save: Option, + compare: Option, +) -> ExitCode { + for &(package, bench_name) in CRITERION_BENCHES { + eprintln!( + "\n{} {} / {}", + console::style("▸").cyan().bold(), + console::style(package).bold(), + console::style(bench_name).bold() + ); + if let Err(message) = run_criterion_bench( + package, + bench_name, + criterion_args, + save.as_deref(), + compare.as_deref(), + ) { + eprintln!("bench failed: {message}"); + return ExitCode::FAILURE; + } + } + ExitCode::SUCCESS +} + +fn run_criterion_bench( + package: &str, + bench_name: &str, + criterion_args: &[&str], + save: Option<&str>, + compare: Option<&str>, +) -> Result<(), String> { + let args = criterion_bench_args(package, bench_name, criterion_args, save, compare); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + run_command("cargo", &arg_refs, None) +} + +/// Build the `cargo bench` argument vector for one Criterion target. +/// +/// Kept separate from process spawning so the routing of baseline flags is +/// unit-testable. +fn criterion_bench_args( + package: &str, + bench_name: &str, + criterion_args: &[&str], + save: Option<&str>, + compare: Option<&str>, +) -> Vec { + let mut args = vec![ + "bench".to_string(), + "-p".to_string(), + package.to_string(), + "--bench".to_string(), + bench_name.to_string(), + ]; + if save.is_some() || compare.is_some() || !criterion_args.is_empty() { + args.push("--".to_string()); + } + args.extend(criterion_args.iter().map(|arg| (*arg).to_string())); + if let Some(name) = save { + args.push("--save-baseline".to_string()); + args.push(name.to_string()); + } + if let Some(name) = compare { + args.push("--baseline".to_string()); + args.push(name.to_string()); + } + args +} + fn bench_webui_criterion_phase(save: Option, compare: Option) -> ExitCode { let mut args: Vec = vec![ "bench".into(), @@ -421,6 +521,36 @@ fn bench_browser(save: Option, compare: Option) -> ExitCode { } } +fn bench_lazy_hydration(save: Option, compare: Option) -> ExitCode { + use std::process::Command; + let bench_dir = std::path::PathBuf::from("examples") + .join("integration") + .join("streaming-browser-bench"); + if !bench_dir.join("package.json").exists() { + eprintln!("lazy-hydration bench: {} not found", bench_dir.display()); + return ExitCode::FAILURE; + } + let mut cmd = Command::new("pnpm"); + cmd.arg("test:lazy-hydration").current_dir(&bench_dir); + if let Some(name) = save.as_ref() { + cmd.env("WEBUI_LAZY_HYDRATION_SAVE", name); + } + if let Some(name) = compare.as_ref() { + cmd.env("WEBUI_LAZY_HYDRATION_COMPARE", name); + } + match cmd.status() { + Ok(status) if status.success() => ExitCode::SUCCESS, + Ok(status) => { + eprintln!("lazy-hydration bench exited with {status}"); + ExitCode::FAILURE + } + Err(error) => { + eprintln!("lazy-hydration bench: failed to spawn pnpm: {error}"); + ExitCode::FAILURE + } + } +} + fn bench_node_addon(save: Option, compare: Option) -> ExitCode { if save.is_some() && compare.is_some() { eprintln!("node-addon bench: save and compare modes are mutually exclusive"); @@ -827,3 +957,140 @@ fn print_failure_output_with_name(name: &str, output: &str) { } eprintln!(" {separator}"); } + +#[cfg(test)] +mod tests { + use super::{criterion_bench_args, CRITERION_BENCHES}; + + #[test] + fn criterion_bench_args_target_one_bench_binary() { + let args = criterion_bench_args("microsoft-webui-parser", "parser_bench", &[], None, None); + assert_eq!( + args, + vec![ + "bench", + "-p", + "microsoft-webui-parser", + "--bench", + "parser_bench" + ] + ); + // `--workspace` would also sweep libtest harnesses, which reject + // Criterion's baseline flags. + assert!(!args.iter().any(|arg| arg == "--workspace")); + } + + #[test] + fn criterion_bench_args_forward_save_baseline_after_separator() { + let args = criterion_bench_args( + "microsoft-webui-state", + "state_bench", + &[], + Some("before"), + None, + ); + assert_eq!( + args, + vec![ + "bench", + "-p", + "microsoft-webui-state", + "--bench", + "state_bench", + "--", + "--save-baseline", + "before" + ] + ); + } + + #[test] + fn criterion_bench_args_forward_compare_baseline_after_separator() { + let args = criterion_bench_args( + "microsoft-webui-state", + "state_bench", + &[], + None, + Some("before"), + ); + assert_eq!( + args, + vec![ + "bench", + "-p", + "microsoft-webui-state", + "--bench", + "state_bench", + "--", + "--baseline", + "before" + ] + ); + } + + #[test] + fn criterion_bench_args_keep_extra_args_before_baseline_flags() { + let args = criterion_bench_args( + "microsoft-webui", + "streaming_bench", + &["--quick"], + Some("after"), + None, + ); + assert_eq!( + args, + vec![ + "bench", + "-p", + "microsoft-webui", + "--bench", + "streaming_bench", + "--", + "--quick", + "--save-baseline", + "after" + ] + ); + } + + #[test] + fn criterion_bench_args_omit_separator_without_passthrough_args() { + let args = criterion_bench_args("microsoft-webui-ffi", "protocol_bench", &[], None, None); + assert!(!args.iter().any(|arg| arg == "--")); + } + + #[test] + fn criterion_bench_table_is_non_empty_and_unique() { + assert!(!CRITERION_BENCHES.is_empty()); + let mut seen: Vec<(&str, &str)> = CRITERION_BENCHES.to_vec(); + seen.sort_unstable(); + let before = seen.len(); + seen.dedup(); + assert_eq!( + before, + seen.len(), + "CRITERION_BENCHES has duplicate entries" + ); + } + + #[test] + fn criterion_bench_table_targets_exist_on_disk() { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .map(std::path::Path::to_path_buf) + .unwrap_or_default(); + for &(package, bench_name) in CRITERION_BENCHES { + let crate_dir = package.replace("microsoft-", ""); + let path = root + .join("crates") + .join(&crate_dir) + .join("benches") + .join(format!("{bench_name}.rs")); + assert!( + path.is_file(), + "CRITERION_BENCHES entry ({package}, {bench_name}) has no harness at {}", + path.display() + ); + } + } +}