From 108fe79c79cbef0b3fc7b31decd0da51509e8f90 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 08:02:03 -0400 Subject: [PATCH 1/9] ^f C2 session-start latency bench harness, doc, and tests (low risk; additive CI-safe harness that skips cleanly without a runtime, no scripts/workcell or boundary touched; median/p90/gate/skip logic validated locally by go test + shellcheck + shfmt + markdownlint) Implement the locally-validatable core of roadmap item C2 (Session Start Latency Program), mirroring the C5 exec-guard benchmark structure. - scripts/bench/startup-bench.sh: measurement harness timing one mode (cold/warm/cache-hit) with an N-sample median + p90; stats conventions match the C5 harness exactly. Deterministic dry-run path via WORKCELL_STARTUP_SAMPLES_NS makes the math reproducible without a runtime. - scripts/bench/run-startup-bench.sh: driver that runs cold vs warm passes, repeats for RUNS, and enforces a cross-run median-stability gate. Skips cleanly (exit 0) when no container runtime is available, so it is CI-safe. - docs/session-startup-benchmarks.md: methodology + rerun guide mirroring the C5 doc, with results tables left as explicit "pending live capture" placeholders (no fabricated numbers). - internal/startupbench/bench_test.go: pins the median/p90/stddev math, the stability-gate threshold (pass/fail), and the skip-when-no-runtime behavior; runs under `go test ./...` with no runtime. Deferred (needs a live runtime, blocked by the Debian-mirror outage): live benchmark numbers, the kept-warm lane implementation, the cold/warm prep-hook wiring, and the scheduled bench.yml workflow lane. Co-Authored-By: Claude Opus 4.8 --- docs/session-startup-benchmarks.md | 164 ++++++++++++++++++++ internal/startupbench/bench_test.go | 207 +++++++++++++++++++++++++ scripts/bench/run-startup-bench.sh | 224 ++++++++++++++++++++++++++++ scripts/bench/startup-bench.sh | 162 ++++++++++++++++++++ 4 files changed, 757 insertions(+) create mode 100644 docs/session-startup-benchmarks.md create mode 100644 internal/startupbench/bench_test.go create mode 100755 scripts/bench/run-startup-bench.sh create mode 100755 scripts/bench/startup-bench.sh diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md new file mode 100644 index 00000000..b6afa1aa --- /dev/null +++ b/docs/session-startup-benchmarks.md @@ -0,0 +1,164 @@ +# Session-start latency baselines + +Workcell starts an isolated session by resolving the runtime image, booting the +container runtime (Colima today, Apple `container` under evaluation in C1), and +completing the supervisor handshake before an agent can run. **C2** is the +program that measures that start latency, drives it down with cached images and +an optional kept-warm lane, and publishes reproducible numbers. This page +records the methodology and the rerun steps; it is the C2 sibling of +[syscall-shim-benchmarks.md](syscall-shim-benchmarks.md) (C5). + +The numbers are produced on a host with a live container runtime, not in the +PR-blocking CI lanes: a real session start needs a booted VM, so the cost only +exists where that runtime is available. The results tables below are +**placeholders pending a live capture** — see +[Filling in the numbers](#filling-in-the-numbers). Do not treat the template +values as measured. + +## What is measured + +A **sample** is one full session start: the wall-clock time from invoking the +session-start command to the point the session is ready. The harness times three +modes that span the latency shapes C2 targets: + +| Mode | Runtime state before the sample | What it isolates | +|---|---|---| +| `cold` | image not cached, no kept-warm session | worst-case first start (image resolve + full boot) | +| `warm` | image cached and a kept-warm session available | best-case start off the kept-warm lane | +| `cache-hit` | image cached, no kept-warm session | the image-cache win alone, without the warm lane | + +The `cold` vs `warm` delta is the headline C2 number: how much the cache plus the +kept-warm lane save off a first start. The `cache-hit` mode separates the +image-cache contribution from the kept-warm-lane contribution so each optimization +can be credited independently. + +## Methodology + +The harness (`scripts/bench/startup-bench.sh`) times one mode: it runs the +session-start command `WORKCELL_STARTUP_WARMUP` times (discarded) to settle +first-touch page-cache and loader costs, then `WORKCELL_STARTUP_ITERATIONS` +measured times, and reports the sample distribution. The stats conventions match +the C5 exec-guard harness exactly, so the two pages' numbers are directly +comparable: + +- **median** — `sorted[floor(n/2)]`, robust to scheduler and I/O outliers; the + headline figure. +- **p90** — `sorted[floor(n*9/10)]` (clamped to the last sample), the tail a user + actually feels on a slow start. +- **mean / stddev** — population mean and standard deviation, reported alongside + the median so a skewed distribution is visible. +- **min / max** — the observed range. + +The driver (`scripts/bench/run-startup-bench.sh`) establishes each mode's runtime +state through a per-mode prep hook (`WORKCELL_STARTUP_COLD_PREP` / +`WORKCELL_STARTUP_WARM_PREP` — e.g. evicting the cached image and stopping the +kept-warm session for `cold`, or pre-pulling and priming the warm lane for +`warm`), then times the configured `WORKCELL_STARTUP_CMD` for that mode. The +whole measurement is repeated for `WORKCELL_STARTUP_RUNS` passes. + +### The cross-run stability gate + +Reproducibility is the C2 acceptance bar, so the driver does not just print the +per-run numbers — it enforces them. After all runs it computes, for each mode, +the spread of the run-to-run **median** as a percentage of the smallest run's +median, and it **fails** (non-zero exit) if any mode exceeds +`WORKCELL_STARTUP_STABILITY_PCT` (default 15%). A published number is only +trustworthy if it repeats; this gate is the evidence that it does. It mirrors the +C5 cross-run stability check, made enforcing. + +### Runner caveats + +- Numbers are **relative** to the measuring host's hardware and runtime backend; + treat the cold-vs-warm delta, not the absolute medians, as the portable signal, + and re-measure on the target host for absolute figures. +- The `cold` mode depends on the prep hook genuinely evicting cached state. If the + hook is a no-op the `cold` and `warm` numbers converge — that is a + misconfiguration, not a fast cold start. +- Session start includes VM boot, which is noisier than a userspace microbenchmark; + expect a wider stddev than the C5 exec-guard numbers and keep the stability + threshold accordingly. + +## Results + +**Status: numbers pending live capture.** The tables below are templates. There +is a sustained upstream mirror outage blocking the container image build at the +time of writing, so no live runtime was available to capture real figures. +Replace the `TODO` cells once a live run is captured (see +[Filling in the numbers](#filling-in-the-numbers)). Do not fabricate values. + +### Start latency by mode (median of N samples, R runs) + +| Mode | Median (ns) | p90 (ns) | Mean (ns) | Stddev (ns) | vs cold | +|---|---|---|---|---|---| +| `cold` | TODO | TODO | TODO | TODO | — | +| `cache-hit` | TODO | TODO | TODO | TODO | TODO | +| `warm` | TODO | TODO | TODO | TODO | TODO | + +### Cross-run stability (median) + +| Mode | Min median (ns) | Max median (ns) | Spread (ns) | Spread (%) | Verdict | +|---|---|---|---|---|---| +| `cold` | TODO | TODO | TODO | TODO | TODO | +| `cache-hit` | TODO | TODO | TODO | TODO | TODO | +| `warm` | TODO | TODO | TODO | TODO | TODO | + +## Filling in the numbers + +1. On a host with a live container runtime, wire the prep hooks and session + command, then run the driver (see [Rerunning](#rerunning) below) with + `WORKCELL_STARTUP_OUTPUT` set to capture the Markdown report. +2. Confirm the run exits `0` — that means the cross-run stability gate passed and + the numbers are reproducible. A non-zero exit means the spread was too wide to + publish; investigate before transcribing. +3. Transcribe the report's per-mode medians, p90s and the stability table into the + tables above, fill the `vs cold` column with the measured deltas, and record + the host, runtime backend, and `N`/`R` used. + +## Rerunning + +From the repository root on a host with a container runtime: + +```sh +# Wire the prep hooks and the session-start command to your runtime, then: +export WORKCELL_STARTUP_CMD='./scripts/workcell ' +export WORKCELL_STARTUP_COLD_PREP='' +export WORKCELL_STARTUP_WARM_PREP='' +export WORKCELL_STARTUP_OUTPUT=session-startup-results.md + +# Defaults: 5 iterations, 1 warmup, 2 runs, 15% stability threshold. +./scripts/bench/run-startup-bench.sh +``` + +Tunable via environment: `WORKCELL_STARTUP_ITERATIONS`, `WORKCELL_STARTUP_WARMUP`, +`WORKCELL_STARTUP_RUNS`, `WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, +`WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_WARM_PREP`, and +`WORKCELL_STARTUP_OUTPUT`. + +### Dry run without a runtime + +The driver is CI-safe: with no runtime available it prints a clear message and +exits `0` (skip, not fail). To rehearse the full report and stability gate on any +host — no runtime needed — feed canned samples: + +```sh +# One stable run set (gate passes, exit 0): +WORKCELL_STARTUP_SAMPLES_NS='10 20 30 40 50' ./scripts/bench/run-startup-bench.sh + +# Two ';'-separated per-run groups with divergent medians (gate fails, exit 2): +WORKCELL_STARTUP_SAMPLES_NS='10 20 30;100 200 300' ./scripts/bench/run-startup-bench.sh +``` + +This canned path is what the unit tests in `internal/startupbench` use to pin the +median/p90/stddev math, the stability gate, and the skip behavior without a +container build or a live VM. + +## Where this fits + +The harness and driver live in `scripts/bench/` alongside the C5 exec-guard +benchmark. A scheduled, non-PR-blocking workflow lane that captures the live +numbers on a runtime-capable runner is **deferred** until the image build is +unblocked; when added it will follow the `bench.yml` pattern and upload the +Markdown report as an artifact. See +[syscall-shim-benchmarks.md](syscall-shim-benchmarks.md) for the sibling C5 +baselines and [github-workflows.md](github-workflows.md) for the workflow +inventory. diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go new file mode 100644 index 00000000..393936b1 --- /dev/null +++ b/internal/startupbench/bench_test.go @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 Omkhar Arasaratnam + +// Package startupbench holds the tests that pin the pure logic of the C2 +// session-start latency benchmark scripts (scripts/bench/startup-bench.sh and +// scripts/bench/run-startup-bench.sh): the median/p90/stddev math, the +// cross-run stability gate, and the skip-when-no-runtime behavior. These run +// under `go test ./...` and need no container runtime. +package startupbench + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func repoRoot(tb testing.TB) string { + tb.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + tb.Fatal("unable to determine repo root") + } + return filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) +} + +// runScript runs a bench script with the given args and extra environment, +// returning its exit code and combined stdout+stderr. +func runScript(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string) { + tb.Helper() + root := repoRoot(tb) + cmd := exec.Command(filepath.Join(root, filepath.FromSlash(relScript)), args...) + cmd.Dir = root + cmd.Env = os.Environ() + for k, v := range env { + cmd.Env = append(cmd.Env, k+"="+v) + } + out, err := cmd.CombinedOutput() + if err == nil { + return 0, string(out) + } + if exitErr, ok := err.(*exec.ExitError); ok { + return exitErr.ExitCode(), string(out) + } + tb.Fatalf("run %s failed: %v\n%s", relScript, err, out) + return -1, "" +} + +const harness = "scripts/bench/startup-bench.sh" +const driver = "scripts/bench/run-startup-bench.sh" + +// parseFields turns a "k=v k=v" harness line into a map. +func parseFields(line string) map[string]string { + fields := map[string]string{} + for _, tok := range strings.Fields(strings.TrimSpace(line)) { + if k, v, ok := strings.Cut(tok, "="); ok { + fields[k] = v + } + } + return fields +} + +func TestHarnessStatsOddSampleSet(t *testing.T) { + // Deliberately unsorted; the harness sorts before computing. n=5 so median + // is the 3rd value and p90 (index floor(5*9/10)=4) is the max. + code, out := runScript(t, harness, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "50 10 40 20 30"}, + "cold", "0", "0") + if code != 0 { + t.Fatalf("exit %d, out=%q", code, out) + } + f := parseFields(out) + want := map[string]string{ + "mode": "cold", "n": "5", "mean_ns": "30", "median_ns": "30", + "p90_ns": "50", "stddev_ns": "14", "min_ns": "10", "max_ns": "50", + } + for k, v := range want { + if f[k] != v { + t.Errorf("field %s = %q, want %q (line: %s)", k, f[k], v, strings.TrimSpace(out)) + } + } +} + +func TestHarnessMedianEvenSampleSet(t *testing.T) { + // n=6: matches the C5 harness convention median=sorted[n/2] (upper-middle). + code, out := runScript(t, harness, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50 60"}, + "warm", "0", "0") + if code != 0 { + t.Fatalf("exit %d, out=%q", code, out) + } + f := parseFields(out) + if f["median_ns"] != "40" { + t.Errorf("median_ns = %q, want 40", f["median_ns"]) + } + if f["p90_ns"] != "60" { + t.Errorf("p90_ns = %q, want 60", f["p90_ns"]) + } + if f["n"] != "6" { + t.Errorf("n = %q, want 6", f["n"]) + } +} + +func TestHarnessRejectsUnknownMode(t *testing.T) { + code, out := runScript(t, harness, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30"}, + "bogus", "0", "0") + if code == 0 { + t.Fatalf("expected non-zero exit for unknown mode, got 0: %s", out) + } + if !strings.Contains(out, "unknown mode") { + t.Errorf("missing 'unknown mode' diagnostic: %s", out) + } +} + +func TestHarnessRejectsNonIntegerSample(t *testing.T) { + code, out := runScript(t, harness, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 x 30"}, + "cold", "0", "0") + if code == 0 { + t.Fatalf("expected non-zero exit for non-integer sample, got 0: %s", out) + } + if !strings.Contains(out, "non-integer sample") { + t.Errorf("missing 'non-integer sample' diagnostic: %s", out) + } +} + +func TestHarnessLivePathTimesTarget(t *testing.T) { + // No canned samples: the harness times a benign target. This exercises the + // real clock + timing pipeline and confirms n == iterations with a + // non-negative median. Values are host-dependent, so we assert structure. + code, out := runScript(t, harness, nil, "warm", "3", "1", "--", "true") + if code != 0 { + t.Fatalf("exit %d, out=%q", code, out) + } + f := parseFields(out) + if f["n"] != "3" { + t.Errorf("n = %q, want 3 (line: %s)", f["n"], strings.TrimSpace(out)) + } + for _, k := range []string{"median_ns", "p90_ns", "min_ns", "max_ns"} { + if f[k] == "" { + t.Errorf("missing field %s in live output: %s", k, strings.TrimSpace(out)) + } + } +} + +func TestDriverSkipsWithoutRuntime(t *testing.T) { + code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_RUNTIME": "none"}) + if code != 0 { + t.Fatalf("skip should exit 0, got %d: %s", code, out) + } + if !strings.Contains(out, "skipping") || !strings.Contains(out, "no container runtime") { + t.Errorf("missing clean-skip message: %s", out) + } +} + +func TestDriverDryRunStablePasses(t *testing.T) { + code, out := runScript(t, driver, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50"}) + if code != 0 { + t.Fatalf("stable dry run should exit 0, got %d: %s", code, out) + } + for _, want := range []string{ + "# session-start latency benchmark results", + "| cold |", "| warm |", + "Cross-run stability (median)", + "Stability gate: STABLE", + } { + if !strings.Contains(out, want) { + t.Errorf("report missing %q\n---\n%s", want, out) + } + } +} + +func TestDriverDryRunUnstableFailsGate(t *testing.T) { + // Two ';'-separated per-run groups with very different medians (20 vs 200) + // blow past the default 15%% stability threshold, so the gate must fail. + code, out := runScript(t, driver, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30;100 200 300"}) + if code != 2 { + t.Fatalf("unstable dry run should exit 2, got %d: %s", code, out) + } + if !strings.Contains(out, "Stability gate: UNSTABLE") { + t.Errorf("missing UNSTABLE gate verdict: %s", out) + } + if !strings.Contains(out, "stability gate FAILED") { + t.Errorf("missing gate-failure diagnostic: %s", out) + } +} + +func TestDriverStabilityThresholdIsConfigurable(t *testing.T) { + // The same 20->21 spread (5%%) passes at the default threshold but a 1%% + // threshold rejects it, proving the gate reads the configured bound. + env := map[string]string{ + "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30;11 21 31", + "WORKCELL_STARTUP_STABILITY_PCT": "1", + } + code, out := runScript(t, driver, env) + if code != 2 { + t.Fatalf("5%% spread under a 1%% threshold should fail (exit 2), got %d: %s", code, out) + } + if !strings.Contains(out, "UNSTABLE") { + t.Errorf("missing UNSTABLE verdict at tight threshold: %s", out) + } +} diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh new file mode 100755 index 00000000..fd0a0791 --- /dev/null +++ b/scripts/bench/run-startup-bench.sh @@ -0,0 +1,224 @@ +#!/usr/bin/env -S BASH_ENV= ENV= bash +# +# run-startup-bench.sh -- drive the session-start latency benchmark (C2). +# +# For each mode (cold, warm) it runs a per-mode prep hook to establish the +# runtime state, then times WORKCELL_STARTUP_ITERATIONS full session starts of +# the target command via scripts/bench/startup-bench.sh, reporting the median, +# p90 and spread. The whole measurement is repeated for WORKCELL_STARTUP_RUNS +# passes so a reviewer can confirm the numbers are stable across runs, and the +# driver FAILS if the run-to-run spread of any mode's median exceeds the +# stability threshold -- the C2 sibling of the C5 cross-run validation gate. +# +# CI/offline safety: session starts need a live container runtime (Colima or an +# Apple `container` VM). When none is available the driver prints a clear +# message and exits 0 (skip, not fail), so it is safe to invoke as a dry run on +# any host. Supplying WORKCELL_STARTUP_SAMPLES_NS switches the driver into a +# canned dry-run that exercises the full report + stability gate without a +# runtime (used by the unit tests and for local rehearsal). +# See docs/session-startup-benchmarks.md. +# +# Configuration (all optional, via environment): +# WORKCELL_STARTUP_ITERATIONS measured samples per mode (default 5) +# WORKCELL_STARTUP_WARMUP discarded warmup samples (default 1) +# WORKCELL_STARTUP_RUNS full measurement passes (default 2) +# WORKCELL_STARTUP_STABILITY_PCT max allowed cross-run median spread (default 15) +# WORKCELL_STARTUP_CMD session-start command to time (required live) +# WORKCELL_STARTUP_COLD_PREP shell run before the cold pass (evict cache/warm lane) +# WORKCELL_STARTUP_WARM_PREP shell run before the warm pass (prime cache/warm lane) +# WORKCELL_STARTUP_RUNTIME override runtime detection (a name, or "none") +# WORKCELL_STARTUP_SAMPLES_NS canned samples -> dry-run, no runtime needed. +# A ';' splits per-run groups (each ';'-segment is +# one run's samples), which drives RUNS and lets a +# dry run rehearse an unstable cross-run spread. +# WORKCELL_STARTUP_OUTPUT also write the Markdown report to this file +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/.." +ROOT_DIR="$(cd "${ROOT_DIR}" && pwd)" +HARNESS="${ROOT_DIR}/scripts/bench/startup-bench.sh" + +MODES="cold warm" +ITERATIONS="${WORKCELL_STARTUP_ITERATIONS:-5}" +WARMUP="${WORKCELL_STARTUP_WARMUP:-1}" +RUNS="${WORKCELL_STARTUP_RUNS:-2}" +STABILITY_PCT="${WORKCELL_STARTUP_STABILITY_PCT:-15}" +OUTPUT_PATH="${WORKCELL_STARTUP_OUTPUT:-}" +SAMPLES="${WORKCELL_STARTUP_SAMPLES_NS:-}" + +[ -x "${HARNESS}" ] || { + echo "run-startup-bench: harness not found or not executable: ${HARNESS}" >&2 + exit 1 +} + +# Detect an available container runtime. A canned-sample dry run needs none. +DRY_RUN=0 +RUNTIME="" +SAMPLE_GROUPS=() +if [ -n "${SAMPLES}" ]; then + DRY_RUN=1 + RUNTIME="dry-run (canned samples)" + # A ';' separates per-run sample groups; multiple groups define RUNS. + local_ifs="${IFS}" + IFS=';' read -ra SAMPLE_GROUPS <<<"${SAMPLES}" + IFS="${local_ifs}" + if [ "${#SAMPLE_GROUPS[@]}" -gt 1 ]; then + RUNS="${#SAMPLE_GROUPS[@]}" + fi +else + detected="${WORKCELL_STARTUP_RUNTIME:-}" + if [ -z "${detected}" ]; then + for candidate in colima container docker; do + if command -v "${candidate}" >/dev/null 2>&1; then + detected="${candidate}" + break + fi + done + fi + if [ -z "${detected}" ] || [ "${detected}" = "none" ]; then + echo "run-startup-bench: no container runtime (Colima / Apple container) is" \ + "available on this host; session-start latency needs a live runtime." >&2 + echo "run-startup-bench: skipping (clean exit). Set WORKCELL_STARTUP_SAMPLES_NS" \ + "for a canned dry run, or run on a host with a runtime. See" \ + "docs/session-startup-benchmarks.md." >&2 + exit 0 + fi + RUNTIME="${detected}" + if [ -z "${WORKCELL_STARTUP_CMD:-}" ]; then + echo "run-startup-bench: WORKCELL_STARTUP_CMD (the session-start command to" \ + "time) is required for a live run." >&2 + exit 1 + fi +fi + +WORKDIR="$(mktemp -d "${TMPDIR:-/tmp}/startup-bench.XXXXXX")" +trap 'rm -rf "${WORKDIR}"' EXIT +REPORT="${WORKDIR}/report.md" + +# Extract an integer key=value field from a harness output line. +field() { + printf '%s\n' "$1" | sed -n "s/.*[[:space:]]$2=\([0-9]*\).*/\1/p" +} + +# Run the per-mode prep hook that establishes cold vs warm runtime state. +prep_mode() { + case "$1" in + cold) eval "${WORKCELL_STARTUP_COLD_PREP:-:}" ;; + warm) eval "${WORKCELL_STARTUP_WARM_PREP:-:}" ;; + esac +} + +# Invoke the harness for one mode. In dry-run the canned samples for this run +# are passed straight through; live, the configured session command is timed. +run_harness() { + # $1 mode, $2 run index (1-based) + if [ "${DRY_RUN}" -eq 1 ]; then + local gi=$(($2 - 1)) + [ "${gi}" -lt "${#SAMPLE_GROUPS[@]}" ] || gi=$((${#SAMPLE_GROUPS[@]} - 1)) + WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" "$1" "${ITERATIONS}" "${WARMUP}" + else + # shellcheck disable=SC2086 # WORKCELL_STARTUP_CMD is an intentional word-split command + "${HARNESS}" "$1" "${ITERATIONS}" "${WARMUP}" -- ${WORKCELL_STARTUP_CMD} + fi +} + +{ + echo "# session-start latency benchmark results" + echo + echo "- date (UTC): $(date -u '+%Y-%m-%dT%H:%M:%SZ')" + echo "- host: $(uname -srm)" + echo "- online CPUs: $(getconf _NPROCESSORS_ONLN 2>/dev/null || echo unknown)" + echo "- runtime: ${RUNTIME}" + echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}) x ${RUNS} run(s)" + echo "- stability threshold: ${STABILITY_PCT}% cross-run median spread" + echo +} >"${REPORT}" + +run_index=1 +while [ "${run_index}" -le "${RUNS}" ]; do + run_file="${WORKDIR}/run-${run_index}" + : >"${run_file}" + + { + echo "## Run ${run_index}" + echo + echo "| Mode | Median (ns) | p90 (ns) | Mean (ns) | Stddev (ns) | Min (ns) | Max (ns) | n |" + echo "|---|---|---|---|---|---|---|---|" + } >>"${REPORT}" + + for mode in ${MODES}; do + prep_mode "${mode}" + line="$(run_harness "${mode}" "${run_index}")" + med="$(field "${line}" median_ns)" + p90="$(field "${line}" p90_ns)" + mean="$(field "${line}" mean_ns)" + std="$(field "${line}" stddev_ns)" + lo="$(field "${line}" min_ns)" + hi="$(field "${line}" max_ns)" + n="$(field "${line}" n)" + + printf '| %s | %s | %s | %s | %s | %s | %s | %s |\n' \ + "${mode}" "${med}" "${p90}" "${mean}" "${std}" "${lo}" "${hi}" "${n}" >>"${REPORT}" + printf '%s %s\n' "${mode}" "${med}" >>"${run_file}" + done + + echo >>"${REPORT}" + run_index=$((run_index + 1)) +done + +# Cross-run stability gate: for each mode, the spread of the median across all +# runs as a percentage of the smallest run's median. If any mode exceeds the +# threshold the run is not reproducible and the driver fails. +GATE_STATUS="STABLE" +if [ "${RUNS}" -ge 2 ]; then + { + echo "## Cross-run stability (median)" + echo + echo "| Mode | Min median (ns) | Max median (ns) | Spread (ns) | Spread (%) | Verdict |" + echo "|---|---|---|---|---|---|" + } >>"${REPORT}" + + all_runs="${WORKDIR}/all-runs" + cat "${WORKDIR}"/run-* >"${all_runs}" + worst="$(awk -v thr="${STABILITY_PCT}" ' + { m = $1; v = $2 + if (!(m in seen)) { order[++k] = m; seen[m] = 1; min[m] = v; max[m] = v } + if (v < min[m]) min[m] = v + if (v > max[m]) max[m] = v } + END { + worst = 0 + for (i = 1; i <= k; i++) { + m = order[i]; s = max[m] - min[m] + p = (min[m] > 0) ? s * 100.0 / min[m] : 0 + verdict = (p > thr) ? "UNSTABLE" : "STABLE" + if (p > worst) worst = p + printf "| %s | %d | %d | %d | %.1f | %s |\n", m, min[m], max[m], s, p, verdict > "/dev/stderr" + } + printf "%.1f", worst + } + ' "${all_runs}" 2>>"${REPORT}")" + echo >>"${REPORT}" + + if awk -v w="${worst}" -v thr="${STABILITY_PCT}" 'BEGIN { exit (w > thr) ? 0 : 1 }'; then + GATE_STATUS="UNSTABLE" + fi + { + if [ "${GATE_STATUS}" = "STABLE" ]; then + echo "Stability gate: STABLE (max cross-run median spread ${worst}% <= ${STABILITY_PCT}%)." + else + echo "Stability gate: UNSTABLE (max cross-run median spread ${worst}% > ${STABILITY_PCT}%)." + fi + echo + } >>"${REPORT}" +fi + +cat "${REPORT}" +if [ -n "${OUTPUT_PATH}" ]; then + cp "${REPORT}" "${OUTPUT_PATH}" + echo "run-startup-bench: report written to ${OUTPUT_PATH}" >&2 +fi + +if [ "${GATE_STATUS}" = "UNSTABLE" ]; then + echo "run-startup-bench: cross-run stability gate FAILED (spread ${worst}% > ${STABILITY_PCT}%)" >&2 + exit 2 +fi diff --git a/scripts/bench/startup-bench.sh b/scripts/bench/startup-bench.sh new file mode 100755 index 00000000..05f8880e --- /dev/null +++ b/scripts/bench/startup-bench.sh @@ -0,0 +1,162 @@ +#!/usr/bin/env -S BASH_ENV= ENV= bash +# +# startup-bench.sh -- session-start latency microbenchmark harness (C2). +# +# Times one "session start" latency sample -- the wall-clock cost of launching +# a workcell session -- for a single mode (cold / warm / cache-hit), repeated +# for a reproducible N-sample median + p90. It is the C2 sibling of the C5 +# exec-guard harness (scripts/bench/exec-guard-bench.c): a small, dependency- +# free measurement core whose numbers a reviewer can reproduce. The driver +# (scripts/bench/run-startup-bench.sh) orchestrates the cold vs warm passes and +# the cross-run stability gate. See docs/session-startup-benchmarks.md. +# +# A "session start" is expensive (image resolve + VM/runtime boot + supervisor +# handshake), so the sample unit is one full launch of the target command, not +# a tight micro-loop. The stats conventions match the C5 harness exactly: +# median = sorted[floor(n/2)] (0-based) +# p90 = sorted[floor(n*9/10)] (0-based, clamped to n-1) +# mean = sum/n +# stddev = sqrt(sumsq/n - mean^2) (population) +# so cold/warm numbers are directly comparable to the exec-guard baselines. +# +# Usage: +# startup-bench.sh [--] [target-cmd ...] +# mode one of: cold warm cache-hit +# iterations measured samples (each = one full target launch) +# warmup discarded warmup launches run before measurement +# target-cmd the session-start command to time (e.g. ./scripts/workcell ...) +# +# Deterministic / dry-run path: if WORKCELL_STARTUP_SAMPLES_NS is set to a +# whitespace-separated list of positive integers, those are used as the measured +# samples verbatim and NO command is launched. This makes the stats core +# reproducible in CI and unit tests without a live runtime, and is how the +# driver feeds canned data for a dry run. +# +# Output (one line, key=value pairs, all times in nanoseconds): +# mode= n= mean_ns= median_ns= p90_ns=

\ +# stddev_ns= min_ns= max_ns= +# +# A failed target launch (non-zero exit) aborts with a non-zero status and no +# numbers, so a broken session start can never masquerade as a measurement. +set -euo pipefail + +die() { + echo "startup-bench: $*" >&2 + exit 1 +} + +[ "$#" -ge 3 ] || die "usage: startup-bench.sh [--] [target-cmd ...]" + +MODE="$1" +ITERATIONS="$2" +WARMUP="$3" +shift 3 +if [ "${1:-}" = "--" ]; then + shift +fi + +case "${MODE}" in + cold | warm | cache-hit) ;; + *) die "unknown mode '${MODE}' (want: cold warm cache-hit)" ;; +esac + +case "${ITERATIONS}" in + '' | *[!0-9]*) die "iterations must be a non-negative integer, got '${ITERATIONS}'" ;; +esac +case "${WARMUP}" in + '' | *[!0-9]*) die "warmup must be a non-negative integer, got '${WARMUP}'" ;; +esac + +# Reduce sorted newline-separated integer samples on stdin to the key=value +# stats line (n .. max), using the exact C5 harness conventions. Kept pure so a +# unit test can pin the median/p90/stddev math against a known sample set. +stats() { + sort -n | awk ' + { s[NR] = $1; sum += $1; sumsq += $1 * $1 } + END { + n = NR + if (n == 0) { print "startup-bench: no samples" > "/dev/stderr"; exit 1 } + mean = sum / n + var = sumsq / n - mean * mean + if (var < 0) { var = 0 } + stddev = sqrt(var) + median = s[int(n / 2) + 1] + p90i = int(n * 9 / 10) + if (p90i >= n) { p90i = n - 1 } + p90 = s[p90i + 1] + printf "n=%d mean_ns=%.0f median_ns=%d p90_ns=%d stddev_ns=%.0f min_ns=%d max_ns=%d\n", + n, mean, median, p90, stddev, s[1], s[n] + }' +} + +emit() { + # $@ = integer samples (one per argument) + local line + line="$(printf '%s\n' "$@" | stats)" + printf 'mode=%s %s\n' "${MODE}" "${line}" +} + +# --- Deterministic path: canned samples, no launch ------------------------- +if [ -n "${WORKCELL_STARTUP_SAMPLES_NS:-}" ]; then + read -ra samples <<<"${WORKCELL_STARTUP_SAMPLES_NS}" + [ "${#samples[@]}" -ge 1 ] || die "WORKCELL_STARTUP_SAMPLES_NS is empty" + for sample in "${samples[@]}"; do + case "${sample}" in + '' | *[!0-9]*) die "WORKCELL_STARTUP_SAMPLES_NS holds a non-integer sample '${sample}'" ;; + esac + done + emit "${samples[@]}" + exit 0 +fi + +# --- Live path: launch the target and time each start ---------------------- +[ "$#" -ge 1 ] || die "no target command given (and WORKCELL_STARTUP_SAMPLES_NS unset)" +[ "${ITERATIONS}" -ge 1 ] || die "iterations must be >= 1 for a live measurement" + +# Pick a monotonic-enough nanosecond clock. GNU date has %N; macOS date does +# not, so fall back to perl/python for sub-second resolution. +CLOCK="" +probe="$(date +%s%N 2>/dev/null || true)" +case "${probe}" in + '' | *[!0-9]*) ;; + *) CLOCK="date" ;; +esac +if [ -z "${CLOCK}" ]; then + if command -v perl >/dev/null 2>&1; then + CLOCK="perl" + elif command -v python3 >/dev/null 2>&1; then + CLOCK="python3" + else + die "no nanosecond clock available (need GNU date, perl, or python3)" + fi +fi + +now_ns() { + case "${CLOCK}" in + date) date +%s%N ;; + perl) perl -MTime::HiRes=time -e 'printf "%.0f", Time::HiRes::time() * 1e9' ;; + python3) python3 -c 'import time; print(int(time.time() * 1e9))' ;; + esac +} + +launch() { + "$@" >/dev/null 2>&1 || die "target launch failed (exit $?): $*" +} + +warm_index=0 +while [ "${warm_index}" -lt "${WARMUP}" ]; do + launch "$@" + warm_index=$((warm_index + 1)) +done + +samples=() +sample_index=0 +while [ "${sample_index}" -lt "${ITERATIONS}" ]; do + start="$(now_ns)" + launch "$@" + end="$(now_ns)" + samples+=("$((end - start))") + sample_index=$((sample_index + 1)) +done + +emit "${samples[@]}" From 1d8a31f3d4a4d8895be16bb8ee6141c11d07a0f5 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 10:37:17 -0400 Subject: [PATCH 2/9] ^B Fix C2 startup-bench driver cold-warmup and cache-hit modes (local go test + shellcheck/shfmt/markdownlint green, live runtime unverifiable; user-visible benchmark output) P1: with the default warmup, the driver ran the cold-prep hook once and then spent that freshly-evicted state on the discarded warmup launch, so every measured cold sample was actually a warmed start. Force warmup to 0 for cold so the first measured sample is a true first start; warm and cache-hit keep the configured warmup. P2: the documented three-mode methodology (cold, cache-hit, warm) was only driven as cold+warm. Add cache-hit to the driven modes with its own WORKCELL_STARTUP_CACHE_HIT_PREP hook, mirroring the cold/warm prep wiring, and document it. Results tables stay pending-live-capture placeholders. Tests: add a stub-harness (WORKCELL_STARTUP_HARNESS test seam) assertion that the driver passes warmup 0 for cold, the configured warmup for warm/cache-hit, and drives all three modes including cache-hit. Co-Authored-By: Claude Opus 4.8 --- docs/session-startup-benchmarks.md | 30 +++++++++---- internal/startupbench/bench_test.go | 65 +++++++++++++++++++++++++++++ scripts/bench/run-startup-bench.sh | 44 ++++++++++++------- 3 files changed, 117 insertions(+), 22 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index b6afa1aa..89d34140 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -51,10 +51,17 @@ comparable: The driver (`scripts/bench/run-startup-bench.sh`) establishes each mode's runtime state through a per-mode prep hook (`WORKCELL_STARTUP_COLD_PREP` / -`WORKCELL_STARTUP_WARM_PREP` — e.g. evicting the cached image and stopping the -kept-warm session for `cold`, or pre-pulling and priming the warm lane for -`warm`), then times the configured `WORKCELL_STARTUP_CMD` for that mode. The -whole measurement is repeated for `WORKCELL_STARTUP_RUNS` passes. +`WORKCELL_STARTUP_CACHE_HIT_PREP` / `WORKCELL_STARTUP_WARM_PREP` — e.g. evicting +the cached image and stopping the kept-warm session for `cold`, pre-pulling the +image but leaving the warm lane down for `cache-hit`, or pre-pulling and priming +the warm lane for `warm`), then times the configured `WORKCELL_STARTUP_CMD` for +that mode. The whole measurement is repeated for `WORKCELL_STARTUP_RUNS` passes. + +For `cold` the driver forces the warmup count to `0`: a discarded warmup launch +would spend the freshly-evicted state, so with any warmup every measured `cold` +sample would actually be a warmed start. The `warm` and `cache-hit` modes keep +the configured `WORKCELL_STARTUP_WARMUP` to settle first-touch page-cache and +loader costs before measuring. ### The cross-run stability gate @@ -74,6 +81,11 @@ C5 cross-run stability check, made enforcing. - The `cold` mode depends on the prep hook genuinely evicting cached state. If the hook is a no-op the `cold` and `warm` numbers converge — that is a misconfiguration, not a fast cold start. +- Because a session start warms the cache it touches, only the first `cold` sample + runs against a fully evicted state; the driver runs `cold` with warmup `0` so + that first measured sample is a true first start. For a strict per-sample cold + median, make `WORKCELL_STARTUP_COLD_PREP` re-evict on every invocation and drive + `cold` with `WORKCELL_STARTUP_ITERATIONS=1` across more `WORKCELL_STARTUP_RUNS`. - Session start includes VM boot, which is noisier than a userspace microbenchmark; expect a wider stddev than the C5 exec-guard numbers and keep the stability threshold accordingly. @@ -122,6 +134,7 @@ From the repository root on a host with a container runtime: # Wire the prep hooks and the session-start command to your runtime, then: export WORKCELL_STARTUP_CMD='./scripts/workcell ' export WORKCELL_STARTUP_COLD_PREP='' +export WORKCELL_STARTUP_CACHE_HIT_PREP='' export WORKCELL_STARTUP_WARM_PREP='' export WORKCELL_STARTUP_OUTPUT=session-startup-results.md @@ -129,10 +142,11 @@ export WORKCELL_STARTUP_OUTPUT=session-startup-results.md ./scripts/bench/run-startup-bench.sh ``` -Tunable via environment: `WORKCELL_STARTUP_ITERATIONS`, `WORKCELL_STARTUP_WARMUP`, -`WORKCELL_STARTUP_RUNS`, `WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, -`WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_WARM_PREP`, and -`WORKCELL_STARTUP_OUTPUT`. +Tunable via environment: `WORKCELL_STARTUP_ITERATIONS`, `WORKCELL_STARTUP_WARMUP` +(forced to `0` for `cold`), `WORKCELL_STARTUP_RUNS`, +`WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, +`WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_CACHE_HIT_PREP`, +`WORKCELL_STARTUP_WARM_PREP`, and `WORKCELL_STARTUP_OUTPUT`. ### Dry run without a runtime diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 393936b1..35752fa5 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -190,6 +190,71 @@ func TestDriverDryRunUnstableFailsGate(t *testing.T) { } } +func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { + // Regression for the C2 driver findings: + // P1 -- cold must not spend its freshly-evicted state on a discarded + // warmup launch, so the driver forces warmup=0 for cold while other + // modes keep the configured warmup. + // P2 -- the driver must drive the documented three-mode set, including + // cache-hit, not just cold+warm. + // A stub harness (wired via the WORKCELL_STARTUP_HARNESS test seam) records + // the mode + warmup it was invoked with so we can assert the argv the driver + // actually passes on the live path. + dir := t.TempDir() + logPath := filepath.Join(dir, "harness.log") + stub := filepath.Join(dir, "stub-harness.sh") + script := "#!/usr/bin/env bash\n" + + "set -euo pipefail\n" + + "mode=\"$1\"; iters=\"$2\"; warmup=\"$3\"\n" + + "printf '%s %s\\n' \"$mode\" \"$warmup\" >> \"${HARNESS_LOG}\"\n" + + "printf 'mode=%s n=%s mean_ns=1 median_ns=1 p90_ns=1 stddev_ns=0 min_ns=1 max_ns=1\\n' \"$mode\" \"$iters\"\n" + if err := os.WriteFile(stub, []byte(script), 0o755); err != nil { + t.Fatalf("write stub harness: %v", err) + } + env := map[string]string{ + "HARNESS_LOG": logPath, + "WORKCELL_STARTUP_HARNESS": stub, + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": "true", + "WORKCELL_STARTUP_ITERATIONS": "2", + "WORKCELL_STARTUP_WARMUP": "1", + "WORKCELL_STARTUP_RUNS": "1", + } + code, out := runScript(t, driver, env) + if code != 0 { + t.Fatalf("driver exit %d: %s", code, out) + } + if !strings.Contains(out, "| cache-hit |") { + t.Errorf("report missing cache-hit row (P2): %s", out) + } + + data, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("read harness log: %v", err) + } + warmupByMode := map[string]string{} + for _, ln := range strings.Split(strings.TrimSpace(string(data)), "\n") { + parts := strings.Fields(ln) + if len(parts) == 2 { + warmupByMode[parts[0]] = parts[1] + } + } + for _, mode := range []string{"cold", "cache-hit", "warm"} { + if _, ok := warmupByMode[mode]; !ok { + t.Errorf("driver never invoked harness for mode %q; saw %v", mode, warmupByMode) + } + } + if got := warmupByMode["cold"]; got != "0" { + t.Errorf("cold warmup = %q, want 0 (P1: cold must not warm before measuring)", got) + } + if got := warmupByMode["warm"]; got != "1" { + t.Errorf("warm warmup = %q, want 1 (configured warmup preserved)", got) + } + if got := warmupByMode["cache-hit"]; got != "1" { + t.Errorf("cache-hit warmup = %q, want 1 (configured warmup preserved)", got) + } +} + func TestDriverStabilityThresholdIsConfigurable(t *testing.T) { // The same 20->21 spread (5%%) passes at the default threshold but a 1%% // threshold rejects it, proving the gate reads the configured bound. diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index fd0a0791..5d12ea19 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -2,10 +2,13 @@ # # run-startup-bench.sh -- drive the session-start latency benchmark (C2). # -# For each mode (cold, warm) it runs a per-mode prep hook to establish the -# runtime state, then times WORKCELL_STARTUP_ITERATIONS full session starts of -# the target command via scripts/bench/startup-bench.sh, reporting the median, -# p90 and spread. The whole measurement is repeated for WORKCELL_STARTUP_RUNS +# For each mode (cold, cache-hit, warm) it runs a per-mode prep hook to +# establish the runtime state, then times WORKCELL_STARTUP_ITERATIONS full +# session starts of the target command via scripts/bench/startup-bench.sh, +# reporting the median, p90 and spread. The `cold` mode forces warmup to 0 so a +# discarded warmup launch cannot spend the freshly-evicted state -- every cold +# sample is measured against cold-prepped state, not a warmed start. +# The whole measurement is repeated for WORKCELL_STARTUP_RUNS # passes so a reviewer can confirm the numbers are stable across runs, and the # driver FAILS if the run-to-run spread of any mode's median exceeds the # stability threshold -- the C2 sibling of the C5 cross-run validation gate. @@ -20,12 +23,13 @@ # # Configuration (all optional, via environment): # WORKCELL_STARTUP_ITERATIONS measured samples per mode (default 5) -# WORKCELL_STARTUP_WARMUP discarded warmup samples (default 1) +# WORKCELL_STARTUP_WARMUP discarded warmup samples (default 1; forced to 0 for cold) # WORKCELL_STARTUP_RUNS full measurement passes (default 2) # WORKCELL_STARTUP_STABILITY_PCT max allowed cross-run median spread (default 15) # WORKCELL_STARTUP_CMD session-start command to time (required live) -# WORKCELL_STARTUP_COLD_PREP shell run before the cold pass (evict cache/warm lane) -# WORKCELL_STARTUP_WARM_PREP shell run before the warm pass (prime cache/warm lane) +# WORKCELL_STARTUP_COLD_PREP shell run before the cold pass (evict cache + stop warm lane) +# WORKCELL_STARTUP_CACHE_HIT_PREP shell run before the cache-hit pass (prime cache, no warm lane) +# WORKCELL_STARTUP_WARM_PREP shell run before the warm pass (prime cache + warm lane) # WORKCELL_STARTUP_RUNTIME override runtime detection (a name, or "none") # WORKCELL_STARTUP_SAMPLES_NS canned samples -> dry-run, no runtime needed. # A ';' splits per-run groups (each ';'-segment is @@ -36,9 +40,10 @@ set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/.." ROOT_DIR="$(cd "${ROOT_DIR}" && pwd)" -HARNESS="${ROOT_DIR}/scripts/bench/startup-bench.sh" +# WORKCELL_STARTUP_HARNESS overrides the harness path (test seam only). +HARNESS="${WORKCELL_STARTUP_HARNESS:-${ROOT_DIR}/scripts/bench/startup-bench.sh}" -MODES="cold warm" +MODES="cold cache-hit warm" ITERATIONS="${WORKCELL_STARTUP_ITERATIONS:-5}" WARMUP="${WORKCELL_STARTUP_WARMUP:-1}" RUNS="${WORKCELL_STARTUP_RUNS:-2}" @@ -104,6 +109,7 @@ field() { prep_mode() { case "$1" in cold) eval "${WORKCELL_STARTUP_COLD_PREP:-:}" ;; + cache-hit) eval "${WORKCELL_STARTUP_CACHE_HIT_PREP:-:}" ;; warm) eval "${WORKCELL_STARTUP_WARM_PREP:-:}" ;; esac } @@ -111,17 +117,27 @@ prep_mode() { # Invoke the harness for one mode. In dry-run the canned samples for this run # are passed straight through; live, the configured session command is timed. run_harness() { - # $1 mode, $2 run index (1-based) + # $1 mode, $2 run index (1-based), $3 warmup for this mode if [ "${DRY_RUN}" -eq 1 ]; then local gi=$(($2 - 1)) [ "${gi}" -lt "${#SAMPLE_GROUPS[@]}" ] || gi=$((${#SAMPLE_GROUPS[@]} - 1)) - WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" "$1" "${ITERATIONS}" "${WARMUP}" + WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" "$1" "${ITERATIONS}" "$3" else # shellcheck disable=SC2086 # WORKCELL_STARTUP_CMD is an intentional word-split command - "${HARNESS}" "$1" "${ITERATIONS}" "${WARMUP}" -- ${WORKCELL_STARTUP_CMD} + "${HARNESS}" "$1" "${ITERATIONS}" "$3" -- ${WORKCELL_STARTUP_CMD} fi } +# Effective warmup for a mode. Cold must reflect a true first start, so its +# freshly-evicted state cannot be spent on a discarded warmup launch; warm and +# cache-hit keep the configured warmup to settle first-touch page-cache costs. +mode_warmup() { + case "$1" in + cold) echo 0 ;; + *) echo "${WARMUP}" ;; + esac +} + { echo "# session-start latency benchmark results" echo @@ -129,7 +145,7 @@ run_harness() { echo "- host: $(uname -srm)" echo "- online CPUs: $(getconf _NPROCESSORS_ONLN 2>/dev/null || echo unknown)" echo "- runtime: ${RUNTIME}" - echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}) x ${RUNS} run(s)" + echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}; cold forces warmup 0) x ${RUNS} run(s)" echo "- stability threshold: ${STABILITY_PCT}% cross-run median spread" echo } >"${REPORT}" @@ -148,7 +164,7 @@ while [ "${run_index}" -le "${RUNS}" ]; do for mode in ${MODES}; do prep_mode "${mode}" - line="$(run_harness "${mode}" "${run_index}")" + line="$(run_harness "${mode}" "${run_index}" "$(mode_warmup "${mode}")")" med="$(field "${line}" median_ns)" p90="$(field "${line}" p90_ns)" mean="$(field "${line}" mean_ns)" From 20f9f9d530eeb3f1593c42472b9b77fdc02e92f2 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 10:54:53 -0400 Subject: [PATCH 3/9] ^B Re-prep cold per sample and preserve WORKCELL_STARTUP_CMD argv (local go test + shellcheck/shfmt/markdownlint green, live runtime unverifiable; user-visible benchmark correctness) Follow-up to the C2 driver review (752a727): P1: warmup=0 was not sufficient. prep_mode/the cold-prep hook ran once before the harness measured all WORKCELL_STARTUP_ITERATIONS launches, so a session start warmed the cache and only the FIRST cold sample was a true first start; the rest were cache-hits. Add measure_cold: re-run WORKCELL_STARTUP_COLD_PREP before EACH measured cold sample, time each start on its own (warmup 0), and aggregate the per-sample timings through the harness stats core so the cold row still reads like a normal distribution. warm/cache-hit keep one shared prep. P2: the driver expanded $WORKCELL_STARTUP_CMD unquoted, so an argument with spaces (e.g. --workspace '/path/with space') was word-split/globbed and the wrong argv was timed. Parse the command once via a shell-quoting-aware `eval "CMD_ARGV=( ... )"` and launch "${CMD_ARGV[@]}", preserving argv boundaries. Documented as the WORKCELL_STARTUP_CMD contract. Tests: dry-run test asserts cold prep runs once per measured sample (3 canned samples -> 3 preps) while warm/cache-hit prep once; a recorder-target test on the live harness path asserts the spaced argument reaches the target as one argv element. Doc updated for both contracts. Co-Authored-By: Claude Opus 4.8 --- docs/session-startup-benchmarks.md | 33 +++++++---- internal/startupbench/bench_test.go | 86 +++++++++++++++++++++++++++++ scripts/bench/run-startup-bench.sh | 78 ++++++++++++++++++-------- 3 files changed, 165 insertions(+), 32 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index 89d34140..0378a618 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -57,11 +57,22 @@ image but leaving the warm lane down for `cache-hit`, or pre-pulling and priming the warm lane for `warm`), then times the configured `WORKCELL_STARTUP_CMD` for that mode. The whole measurement is repeated for `WORKCELL_STARTUP_RUNS` passes. -For `cold` the driver forces the warmup count to `0`: a discarded warmup launch -would spend the freshly-evicted state, so with any warmup every measured `cold` -sample would actually be a warmed start. The `warm` and `cache-hit` modes keep -the configured `WORKCELL_STARTUP_WARMUP` to settle first-touch page-cache and -loader costs before measuring. +For `cold` the driver re-runs `WORKCELL_STARTUP_COLD_PREP` before **every** +measured sample and times each start on its own with warmup `0`, then aggregates +the per-sample timings through the same stats core. A single session start warms +the cache the next start would otherwise hit, so evicting only once before the +pass would leave just the first sample genuinely cold; the per-sample re-prep +keeps every `cold` sample a true first start. The cold-prep hook must therefore +be **repeatable** — it runs once per measured sample, not once per pass. The +`warm` and `cache-hit` modes legitimately share one prep for the whole pass and +keep the configured `WORKCELL_STARTUP_WARMUP` to settle first-touch page-cache +and loader costs before measuring. + +`WORKCELL_STARTUP_CMD` is parsed with shell quoting into the target argv, so an +argument containing spaces keeps its boundary. Quote such arguments exactly as +you would on a shell command line — e.g. +`WORKCELL_STARTUP_CMD="./scripts/workcell --workspace '/path/with space'"` reaches +the target as three argv elements, not four word-split tokens. ### The cross-run stability gate @@ -81,11 +92,10 @@ C5 cross-run stability check, made enforcing. - The `cold` mode depends on the prep hook genuinely evicting cached state. If the hook is a no-op the `cold` and `warm` numbers converge — that is a misconfiguration, not a fast cold start. -- Because a session start warms the cache it touches, only the first `cold` sample - runs against a fully evicted state; the driver runs `cold` with warmup `0` so - that first measured sample is a true first start. For a strict per-sample cold - median, make `WORKCELL_STARTUP_COLD_PREP` re-evict on every invocation and drive - `cold` with `WORKCELL_STARTUP_ITERATIONS=1` across more `WORKCELL_STARTUP_RUNS`. +- The cold-prep hook runs once per measured `cold` sample (not once per pass), so + it must be **repeatable and idempotent** — every invocation has to leave the + same fully-evicted, no-warm-lane state. A hook that only evicts on its first + call will silently measure cache-hits for the remaining samples. - Session start includes VM boot, which is noisier than a userspace microbenchmark; expect a wider stddev than the C5 exec-guard numbers and keep the stability threshold accordingly. @@ -132,6 +142,9 @@ From the repository root on a host with a container runtime: ```sh # Wire the prep hooks and the session-start command to your runtime, then: +# WORKCELL_STARTUP_CMD is parsed with shell quoting; quote args with spaces, +# e.g. ...--workspace '/path/with space'. WORKCELL_STARTUP_COLD_PREP is re-run +# before every cold sample, so make it repeatable (idempotent eviction). export WORKCELL_STARTUP_CMD='./scripts/workcell ' export WORKCELL_STARTUP_COLD_PREP='' export WORKCELL_STARTUP_CACHE_HIT_PREP='' diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 35752fa5..35eba7de 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -255,6 +255,92 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { } } +func TestDriverColdRepsPerMeasuredSample(t *testing.T) { + // Regression for the C2 cold-prep finding: a single session start warms the + // cache the next sample would otherwise hit, so evicting once before the + // whole pass leaves only the first sample genuinely cold. The driver must + // re-run WORKCELL_STARTUP_COLD_PREP before EVERY measured cold sample, while + // warm/cache-hit legitimately share one prep for their whole pass. The prep + // hooks append a byte per invocation so we can count them; the dry-run path + // exercises the same re-prep loop without a runtime. + dir := t.TempDir() + coldF := filepath.Join(dir, "cold") + warmF := filepath.Join(dir, "warm") + chF := filepath.Join(dir, "cachehit") + env := map[string]string{ + "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30", + "WORKCELL_STARTUP_RUNS": "1", + "WORKCELL_STARTUP_COLD_PREP": "printf c >> " + coldF, + "WORKCELL_STARTUP_WARM_PREP": "printf w >> " + warmF, + "WORKCELL_STARTUP_CACHE_HIT_PREP": "printf h >> " + chF, + } + code, out := runScript(t, driver, env) + if code != 0 { + t.Fatalf("driver exit %d: %s", code, out) + } + countPreps := func(path string) int { + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read prep counter %s: %v", path, err) + } + return len(data) + } + // Three canned cold samples -> three cold preps (one per measured sample). + if got := countPreps(coldF); got != 3 { + t.Errorf("cold prep ran %d time(s), want 3 (once per measured sample)", got) + } + if got := countPreps(warmF); got != 1 { + t.Errorf("warm prep ran %d time(s), want 1 (one prep for the whole pass)", got) + } + if got := countPreps(chF); got != 1 { + t.Errorf("cache-hit prep ran %d time(s), want 1 (one prep for the whole pass)", got) + } + // The per-sample samples are aggregated through the harness stats core, so + // the cold row must still read like a normal n=3 distribution. + if !strings.Contains(out, "| cold | 20 | 30 | 20 |") { + t.Errorf("aggregated cold row missing/incorrect: %s", out) + } +} + +func TestDriverPreservesCommandArgv(t *testing.T) { + // Regression for the WORKCELL_STARTUP_CMD word-splitting finding: an argument + // with spaces (e.g. --workspace '/path/with space') must reach the target as + // a single argv element, not be split/globbed. A recorder script writes the + // argv it was launched with; the last launch wins (it truncates each time), + // so the constant command leaves a deterministic argv regardless of mode. + dir := t.TempDir() + argvF := filepath.Join(dir, "argv") + rec := filepath.Join(dir, "record.sh") + script := "#!/usr/bin/env bash\n" + + "set -euo pipefail\n" + + ": > \"${ARGV_FILE}\"\n" + + "for a in \"$@\"; do printf '%s\\n' \"$a\" >> \"${ARGV_FILE}\"; done\n" + if err := os.WriteFile(rec, []byte(script), 0o755); err != nil { + t.Fatalf("write recorder: %v", err) + } + env := map[string]string{ + "ARGV_FILE": argvF, + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": rec + " alpha 'beta gamma'", + "WORKCELL_STARTUP_ITERATIONS": "1", + "WORKCELL_STARTUP_WARMUP": "0", + "WORKCELL_STARTUP_RUNS": "1", + } + code, out := runScript(t, driver, env) + if code != 0 { + t.Fatalf("driver exit %d: %s", code, out) + } + data, err := os.ReadFile(argvF) + if err != nil { + t.Fatalf("read argv file: %v", err) + } + got := strings.Split(strings.TrimRight(string(data), "\n"), "\n") + want := []string{"alpha", "beta gamma"} + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Errorf("target argv = %q, want %q (word-splitting would break the spaced arg)\n%s", got, want, out) + } +} + func TestDriverStabilityThresholdIsConfigurable(t *testing.T) { // The same 20->21 spread (5%%) passes at the default threshold but a 1%% // threshold rejects it, proving the gate reads the configured bound. diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index 5d12ea19..520705f0 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -5,9 +5,10 @@ # For each mode (cold, cache-hit, warm) it runs a per-mode prep hook to # establish the runtime state, then times WORKCELL_STARTUP_ITERATIONS full # session starts of the target command via scripts/bench/startup-bench.sh, -# reporting the median, p90 and spread. The `cold` mode forces warmup to 0 so a -# discarded warmup launch cannot spend the freshly-evicted state -- every cold -# sample is measured against cold-prepped state, not a warmed start. +# reporting the median, p90 and spread. The `cold` mode re-runs its prep hook +# before every measured sample and times each start on its own (warmup 0), so a +# session start warming the cache cannot turn later samples into cache-hits -- +# every cold sample reflects a genuine first start, not a warmed one. # The whole measurement is repeated for WORKCELL_STARTUP_RUNS # passes so a reviewer can confirm the numbers are stable across runs, and the # driver FAILS if the run-to-run spread of any mode's median exceeds the @@ -26,8 +27,11 @@ # WORKCELL_STARTUP_WARMUP discarded warmup samples (default 1; forced to 0 for cold) # WORKCELL_STARTUP_RUNS full measurement passes (default 2) # WORKCELL_STARTUP_STABILITY_PCT max allowed cross-run median spread (default 15) -# WORKCELL_STARTUP_CMD session-start command to time (required live) -# WORKCELL_STARTUP_COLD_PREP shell run before the cold pass (evict cache + stop warm lane) +# WORKCELL_STARTUP_CMD session-start command to time (required live); +# parsed with shell quoting, so quote args with +# spaces (e.g. --workspace '/path/with space') +# WORKCELL_STARTUP_COLD_PREP shell re-run before EACH cold sample (evict cache +# + stop warm lane); must be repeatable # WORKCELL_STARTUP_CACHE_HIT_PREP shell run before the cache-hit pass (prime cache, no warm lane) # WORKCELL_STARTUP_WARM_PREP shell run before the warm pass (prime cache + warm lane) # WORKCELL_STARTUP_RUNTIME override runtime detection (a name, or "none") @@ -94,6 +98,11 @@ else "time) is required for a live run." >&2 exit 1 fi + # Parse WORKCELL_STARTUP_CMD into an argv array honoring shell quoting, so an + # argument with spaces (e.g. --workspace '/path/with space') keeps its boundary + # instead of being word-split or glob-expanded. This is the documented contract: + # quote such arguments in WORKCELL_STARTUP_CMD as you would on a shell line. + eval "CMD_ARGV=( ${WORKCELL_STARTUP_CMD} )" fi WORKDIR="$(mktemp -d "${TMPDIR:-/tmp}/startup-bench.XXXXXX")" @@ -114,28 +123,49 @@ prep_mode() { esac } -# Invoke the harness for one mode. In dry-run the canned samples for this run -# are passed straight through; live, the configured session command is timed. +# Invoke the harness for one warm/cache-hit mode: those modes legitimately share +# a single prep for the whole pass. In dry-run the canned samples for this run +# are passed straight through; live, the parsed session command is timed. run_harness() { - # $1 mode, $2 run index (1-based), $3 warmup for this mode + # $1 mode, $2 run index (1-based) if [ "${DRY_RUN}" -eq 1 ]; then local gi=$(($2 - 1)) [ "${gi}" -lt "${#SAMPLE_GROUPS[@]}" ] || gi=$((${#SAMPLE_GROUPS[@]} - 1)) - WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" "$1" "${ITERATIONS}" "$3" + WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" "$1" "${ITERATIONS}" "${WARMUP}" else - # shellcheck disable=SC2086 # WORKCELL_STARTUP_CMD is an intentional word-split command - "${HARNESS}" "$1" "${ITERATIONS}" "$3" -- ${WORKCELL_STARTUP_CMD} + "${HARNESS}" "$1" "${ITERATIONS}" "${WARMUP}" -- "${CMD_ARGV[@]}" fi } -# Effective warmup for a mode. Cold must reflect a true first start, so its -# freshly-evicted state cannot be spent on a discarded warmup launch; warm and -# cache-hit keep the configured warmup to settle first-touch page-cache costs. -mode_warmup() { - case "$1" in - cold) echo 0 ;; - *) echo "${WARMUP}" ;; - esac +# Measure the cold mode with a genuine first start per sample. A single session +# start warms the cache the next start would otherwise hit, so evicting once +# before the pass would leave only the first sample cold. Re-run the cold-prep +# hook before EACH measured sample, time each start on its own (warmup 0), then +# aggregate the samples through the harness stats core so the reported cold row +# matches the other modes exactly. Emits the same key=value stats line. +measure_cold() { + # $1 run index (1-based) + local samples=() one + if [ "${DRY_RUN}" -eq 1 ]; then + local gi=$(($1 - 1)) + [ "${gi}" -lt "${#SAMPLE_GROUPS[@]}" ] || gi=$((${#SAMPLE_GROUPS[@]} - 1)) + local canned_arr=() canned + read -ra canned_arr <<<"${SAMPLE_GROUPS[gi]}" + for canned in "${canned_arr[@]}"; do + prep_mode cold + one="$(WORKCELL_STARTUP_SAMPLES_NS="${canned}" "${HARNESS}" cold 1 0)" + samples+=("$(field "${one}" min_ns)") + done + else + local i=0 + while [ "${i}" -lt "${ITERATIONS}" ]; do + prep_mode cold + one="$("${HARNESS}" cold 1 0 -- "${CMD_ARGV[@]}")" + samples+=("$(field "${one}" min_ns)") + i=$((i + 1)) + done + fi + WORKCELL_STARTUP_SAMPLES_NS="${samples[*]}" "${HARNESS}" cold "${#samples[@]}" 0 } { @@ -145,7 +175,7 @@ mode_warmup() { echo "- host: $(uname -srm)" echo "- online CPUs: $(getconf _NPROCESSORS_ONLN 2>/dev/null || echo unknown)" echo "- runtime: ${RUNTIME}" - echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}; cold forces warmup 0) x ${RUNS} run(s)" + echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}; cold re-preps + warmup 0 per sample) x ${RUNS} run(s)" echo "- stability threshold: ${STABILITY_PCT}% cross-run median spread" echo } >"${REPORT}" @@ -163,8 +193,12 @@ while [ "${run_index}" -le "${RUNS}" ]; do } >>"${REPORT}" for mode in ${MODES}; do - prep_mode "${mode}" - line="$(run_harness "${mode}" "${run_index}" "$(mode_warmup "${mode}")")" + if [ "${mode}" = "cold" ]; then + line="$(measure_cold "${run_index}")" + else + prep_mode "${mode}" + line="$(run_harness "${mode}" "${run_index}")" + fi med="$(field "${line}" median_ns)" p90="$(field "${line}" p90_ns)" mean="$(field "${line}" mean_ns)" From 0294b2a5ff28998134750dd60145f62d565da16d Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 11:04:53 -0400 Subject: [PATCH 4/9] ^B Require each driven mode's prep hook on live startup-bench runs (local go test + shellcheck/shfmt/markdownlint green, live runtime unverifiable; user-visible fail-fast guardrail) Follow-up to the C2 driver review (271b4c5): On a live run (WORKCELL_STARTUP_CMD set), a missing mode prep hook left prep_mode a no-op, so the harness measured whatever runtime state happened to be present -- no real cold eviction or warm-lane priming -- yet still exited 0/STABLE and produced publishable-looking numbers. Fail fast on a live run if the prep hook for any driven mode (WORKCELL_STARTUP_COLD_PREP / WORKCELL_STARTUP_CACHE_HIT_PREP / WORKCELL_STARTUP_WARM_PREP) is unset, naming the mode and the env var. The dry-run path (WORKCELL_STARTUP_SAMPLES_NS / stub harness) needs no prep hooks and is unaffected. Tests: add a test asserting a live run with a missing cold prep hook fails with the documented error while a dry run with no hooks still passes STABLE; the two existing live-path tests now set no-op prep hooks. Doc states live runs require each driven mode's prep hook or the numbers are not publishable. Co-Authored-By: Claude Opus 4.8 --- docs/session-startup-benchmarks.md | 9 +++++++ internal/startupbench/bench_test.go | 39 +++++++++++++++++++++++++++++ scripts/bench/run-startup-bench.sh | 17 +++++++++++++ 3 files changed, 65 insertions(+) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index 0378a618..e1cb8263 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -57,6 +57,15 @@ image but leaving the warm lane down for `cache-hit`, or pre-pulling and priming the warm lane for `warm`), then times the configured `WORKCELL_STARTUP_CMD` for that mode. The whole measurement is repeated for `WORKCELL_STARTUP_RUNS` passes. +A live run **requires** the prep hook for every driven mode: if any of +`WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_CACHE_HIT_PREP`, or +`WORKCELL_STARTUP_WARM_PREP` is unset the driver fails fast (naming the missing +mode and variable) rather than running. Without a mode's hook the harness would +measure whatever runtime state happened to be present — no real cold eviction or +warm-lane priming — and the numbers, though they may look stable, are not +publishable. (The canned dry run below needs no prep hooks; this requirement +applies only to live runs.) + For `cold` the driver re-runs `WORKCELL_STARTUP_COLD_PREP` before **every** measured sample and times each start on its own with warmup `0`, then aggregates the per-sample timings through the same stats core. A single session start warms diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 35eba7de..e34cd44f 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -219,6 +219,10 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { "WORKCELL_STARTUP_ITERATIONS": "2", "WORKCELL_STARTUP_WARMUP": "1", "WORKCELL_STARTUP_RUNS": "1", + // Live runs require each driven mode's prep hook; no-ops suffice here. + "WORKCELL_STARTUP_COLD_PREP": ":", + "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", + "WORKCELL_STARTUP_WARM_PREP": ":", } code, out := runScript(t, driver, env) if code != 0 { @@ -325,6 +329,10 @@ func TestDriverPreservesCommandArgv(t *testing.T) { "WORKCELL_STARTUP_ITERATIONS": "1", "WORKCELL_STARTUP_WARMUP": "0", "WORKCELL_STARTUP_RUNS": "1", + // Live runs require each driven mode's prep hook; no-ops suffice here. + "WORKCELL_STARTUP_COLD_PREP": ":", + "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", + "WORKCELL_STARTUP_WARM_PREP": ":", } code, out := runScript(t, driver, env) if code != 0 { @@ -341,6 +349,37 @@ func TestDriverPreservesCommandArgv(t *testing.T) { } } +func TestDriverLiveRequiresPrepHooks(t *testing.T) { + // Regression for the prep-hook finding: on a LIVE run, a missing mode prep + // hook must fail fast (naming the mode + env var) instead of silently + // measuring whatever runtime state is present and emitting publishable-looking + // numbers. The dry-run path needs no prep hooks and must keep passing. + live := map[string]string{ + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": "true", + // WORKCELL_STARTUP_COLD_PREP deliberately omitted. + "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", + "WORKCELL_STARTUP_WARM_PREP": ":", + } + code, out := runScript(t, driver, live) + if code == 0 { + t.Fatalf("live run with a missing cold prep hook should fail, got exit 0: %s", out) + } + if !strings.Contains(out, "WORKCELL_STARTUP_COLD_PREP") || !strings.Contains(out, "cold") { + t.Errorf("missing-prep error must name the mode and the env var: %s", out) + } + + // Dry-run still works with no prep hooks set. + code, out = runScript(t, driver, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50"}) + if code != 0 { + t.Fatalf("dry-run with no prep hooks should pass, got %d: %s", code, out) + } + if !strings.Contains(out, "Stability gate: STABLE") { + t.Errorf("dry-run should still produce a stable report: %s", out) + } +} + func TestDriverStabilityThresholdIsConfigurable(t *testing.T) { // The same 20->21 spread (5%%) passes at the default threshold but a 1%% // threshold rejects it, proving the gate reads the configured bound. diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index 520705f0..de2d7c22 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -98,6 +98,23 @@ else "time) is required for a live run." >&2 exit 1 fi + # Live runs must establish each driven mode's documented runtime state. A + # missing prep hook would leave prep_mode a no-op, so the harness would measure + # whatever state happened to be present -- no real cold eviction or warm-lane + # priming -- yet still exit STABLE with publishable-looking numbers. Fail fast. + for mode in ${MODES}; do + case "${mode}" in + cold) prep_var="WORKCELL_STARTUP_COLD_PREP" ;; + cache-hit) prep_var="WORKCELL_STARTUP_CACHE_HIT_PREP" ;; + warm) prep_var="WORKCELL_STARTUP_WARM_PREP" ;; + esac + if [ -z "${!prep_var:-}" ]; then + echo "run-startup-bench: live run requires a prep hook for mode '${mode}':" \ + "set ${prep_var} to establish the ${mode} runtime state. Without it the" \ + "harness measures arbitrary state and the numbers are not publishable." >&2 + exit 1 + fi + done # Parse WORKCELL_STARTUP_CMD into an argv array honoring shell quoting, so an # argument with spaces (e.g. --workspace '/path/with space') keeps its boundary # instead of being word-split or glob-expanded. This is the documented contract: From 7ff9b30429059230ab36e7173dc1f64e2e60bd37 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 11:24:37 -0400 Subject: [PATCH 5/9] ^B Harden startup-bench driver: hermetic tests, dry-run skips prep, validate numeric controls (local go test + shellcheck/shfmt/markdownlint green, live runtime unverifiable; user-visible driver behavior + test hardening) Follow-up to the C2 driver review (658ff40); three findings: 1) Hermetic script-driver tests. runScript inherited os.Environ(), so an exported WORKCELL_STARTUP_* (stray prep hook / SAMPLES_NS) could leak into tests that assume a clean process. Build the child env explicitly from PATH (+ HOME, TMPDIR when present) plus only what the test sets. Add a test that leaks SAMPLES_NS + a prep hook via t.Setenv and asserts a no-runtime run still cleanly skips and never runs the hook. 2) Dry runs must not run prep hooks. The canned dry-run path still called prep_mode, so live hooks exported from a previous run would execute. Suppress prep on the dry-run path (main loop guarded by DRY_RUN; measure_cold dry-run now emits the group's stats directly with no prep). Add a test asserting a dry run with all three hooks exported creates no marker file. The per-sample cold re-prep test moves to the live path (prep only runs live now). 3) Validate numeric controls. WORKCELL_STARTUP_RUNS/ITERATIONS/WARMUP/ STABILITY_PCT were unvalidated; RUNS=0 or a non-integer exited 0 with no benchmarking or a misleading STABLE. Add validate_int and fail fast: ITERATIONS/RUNS >= 1, WARMUP/STABILITY_PCT >= 0, naming the offending var. Add a table-driven test. Doc updated for the dry-run/no-prep and numeric-control rules. Co-Authored-By: Claude Opus 4.8 --- docs/session-startup-benchmarks.md | 10 ++- internal/startupbench/bench_test.go | 101 +++++++++++++++++++++++++--- scripts/bench/run-startup-bench.sh | 53 ++++++++++----- 3 files changed, 137 insertions(+), 27 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index e1cb8263..7c540cc8 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -168,13 +168,19 @@ Tunable via environment: `WORKCELL_STARTUP_ITERATIONS`, `WORKCELL_STARTUP_WARMUP (forced to `0` for `cold`), `WORKCELL_STARTUP_RUNS`, `WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, `WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_CACHE_HIT_PREP`, -`WORKCELL_STARTUP_WARM_PREP`, and `WORKCELL_STARTUP_OUTPUT`. +`WORKCELL_STARTUP_WARM_PREP`, and `WORKCELL_STARTUP_OUTPUT`. The numeric controls +are validated up front — `WORKCELL_STARTUP_ITERATIONS` and `WORKCELL_STARTUP_RUNS` +must be integers `>= 1`, and `WORKCELL_STARTUP_WARMUP` / +`WORKCELL_STARTUP_STABILITY_PCT` integers `>= 0`; anything else fails fast rather +than silently producing no measurements or a misleading `STABLE`. ### Dry run without a runtime The driver is CI-safe: with no runtime available it prints a clear message and exits `0` (skip, not fail). To rehearse the full report and stability gate on any -host — no runtime needed — feed canned samples: +host — no runtime needed — feed canned samples. The canned dry run needs no prep +hooks and never executes them, so any `WORKCELL_STARTUP_*_PREP` still exported +from a previous live run is ignored: ```sh # One stable run set (gate passes, exit 0): diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index e34cd44f..814ffc8a 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -33,7 +33,16 @@ func runScript(tb testing.TB, relScript string, env map[string]string, args ...s root := repoRoot(tb) cmd := exec.Command(filepath.Join(root, filepath.FromSlash(relScript)), args...) cmd.Dir = root - cmd.Env = os.Environ() + // Hermetic environment: build the child env explicitly instead of inheriting + // os.Environ(), so a developer's exported WORKCELL_STARTUP_* (or any other + // stray var) cannot leak in and change behavior. Carry only the few vars the + // bench scripts need to locate tools and temp space, plus what the test sets. + cmd.Env = []string{"PATH=" + os.Getenv("PATH")} + for _, k := range []string{"HOME", "TMPDIR"} { + if v, ok := os.LookupEnv(k); ok { + cmd.Env = append(cmd.Env, k+"="+v) + } + } for k, v := range env { cmd.Env = append(cmd.Env, k+"="+v) } @@ -259,20 +268,42 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { } } +func TestRunScriptEnvIsHermetic(t *testing.T) { + // A developer's exported WORKCELL_STARTUP_* must not leak into the script + // driver tests. Export a stray SAMPLES_NS + prep hook in the parent process; + // a no-runtime run must still cleanly SKIP (not turn into a canned dry run) + // and must not execute the leaked prep hook. + t.Setenv("WORKCELL_STARTUP_SAMPLES_NS", "999") + t.Setenv("WORKCELL_STARTUP_COLD_PREP", "echo LEAKED_PREP_RAN") + code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_RUNTIME": "none"}) + if code != 0 { + t.Fatalf("hermetic skip run should exit 0, got %d: %s", code, out) + } + if !strings.Contains(out, "skipping") || !strings.Contains(out, "no container runtime") { + t.Errorf("stray WORKCELL_STARTUP_SAMPLES_NS leaked (expected a clean skip): %s", out) + } + if strings.Contains(out, "LEAKED_PREP_RAN") { + t.Errorf("stray prep hook leaked into the run: %s", out) + } +} + func TestDriverColdRepsPerMeasuredSample(t *testing.T) { // Regression for the C2 cold-prep finding: a single session start warms the // cache the next sample would otherwise hit, so evicting once before the // whole pass leaves only the first sample genuinely cold. The driver must // re-run WORKCELL_STARTUP_COLD_PREP before EVERY measured cold sample, while - // warm/cache-hit legitimately share one prep for their whole pass. The prep - // hooks append a byte per invocation so we can count them; the dry-run path - // exercises the same re-prep loop without a runtime. + // warm/cache-hit legitimately share one prep for their whole pass. Verified on + // the live path (prep is dry-run-suppressed), timing a benign command; the + // prep hooks append a byte per invocation so we can count them. dir := t.TempDir() coldF := filepath.Join(dir, "cold") warmF := filepath.Join(dir, "warm") chF := filepath.Join(dir, "cachehit") env := map[string]string{ - "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30", + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": "true", + "WORKCELL_STARTUP_ITERATIONS": "3", + "WORKCELL_STARTUP_WARMUP": "0", "WORKCELL_STARTUP_RUNS": "1", "WORKCELL_STARTUP_COLD_PREP": "printf c >> " + coldF, "WORKCELL_STARTUP_WARM_PREP": "printf w >> " + warmF, @@ -289,7 +320,7 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { } return len(data) } - // Three canned cold samples -> three cold preps (one per measured sample). + // Three measured cold samples -> three cold preps (one per sample). if got := countPreps(coldF); got != 3 { t.Errorf("cold prep ran %d time(s), want 3 (once per measured sample)", got) } @@ -299,13 +330,67 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { if got := countPreps(chF); got != 1 { t.Errorf("cache-hit prep ran %d time(s), want 1 (one prep for the whole pass)", got) } - // The per-sample samples are aggregated through the harness stats core, so + // The per-sample timings are aggregated through the harness stats core, so // the cold row must still read like a normal n=3 distribution. - if !strings.Contains(out, "| cold | 20 | 30 | 20 |") { + if !strings.Contains(out, "| cold |") || !strings.Contains(out, " 3 |") { t.Errorf("aggregated cold row missing/incorrect: %s", out) } } +func TestDriverDryRunSkipsPrep(t *testing.T) { + // A canned dry run must NEVER execute prep hooks, even if an operator has live + // hooks exported from a previous run. The hooks would append to a marker file; + // after a dry run that file must not exist. + dir := t.TempDir() + marker := filepath.Join(dir, "prep-ran") + env := map[string]string{ + "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50", + "WORKCELL_STARTUP_COLD_PREP": "printf c >> " + marker, + "WORKCELL_STARTUP_WARM_PREP": "printf w >> " + marker, + "WORKCELL_STARTUP_CACHE_HIT_PREP": "printf h >> " + marker, + } + code, out := runScript(t, driver, env) + if code != 0 { + t.Fatalf("dry run should exit 0, got %d: %s", code, out) + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + data, _ := os.ReadFile(marker) + t.Errorf("dry run executed prep hook(s) (marker = %q); dry runs must skip prep", data) + } + if !strings.Contains(out, "Stability gate: STABLE") { + t.Errorf("dry run should still produce a stable report: %s", out) + } +} + +func TestDriverRejectsInvalidNumericControls(t *testing.T) { + // RUNS=0 / non-numeric controls could make the driver exit 0 with no + // benchmarking or a misleading STABLE, so each numeric control is validated as + // an integer at/above its floor. Uses the canned dry-run path so no runtime is + // needed; validation runs regardless of live vs dry-run. + cases := []struct{ name, key, val string }{ + {"RUNS_zero", "WORKCELL_STARTUP_RUNS", "0"}, + {"RUNS_nonnumeric", "WORKCELL_STARTUP_RUNS", "abc"}, + {"ITERATIONS_zero", "WORKCELL_STARTUP_ITERATIONS", "0"}, + {"WARMUP_negative", "WORKCELL_STARTUP_WARMUP", "-1"}, + {"STABILITY_PCT_nonnumeric", "WORKCELL_STARTUP_STABILITY_PCT", "5x"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + env := map[string]string{ + "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30", + c.key: c.val, + } + code, out := runScript(t, driver, env) + if code == 0 { + t.Fatalf("expected non-zero exit for %s=%s, got 0: %s", c.key, c.val, out) + } + if !strings.Contains(out, c.key) { + t.Errorf("error should name the offending control %s: %s", c.key, out) + } + }) + } +} + func TestDriverPreservesCommandArgv(t *testing.T) { // Regression for the WORKCELL_STARTUP_CMD word-splitting finding: an argument // with spaces (e.g. --workspace '/path/with space') must reach the target as diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index de2d7c22..17409bb3 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -55,6 +55,21 @@ STABILITY_PCT="${WORKCELL_STARTUP_STABILITY_PCT:-15}" OUTPUT_PATH="${WORKCELL_STARTUP_OUTPUT:-}" SAMPLES="${WORKCELL_STARTUP_SAMPLES_NS:-}" +# Validate a numeric driver control is an integer at/above a floor; fail fast. +validate_int() { + # $1 env var name, $2 value, $3 floor + case "$2" in + '' | *[!0-9]*) + echo "run-startup-bench: $1 must be an integer, got '$2'." >&2 + exit 1 + ;; + esac + if [ "$2" -lt "$3" ]; then + echo "run-startup-bench: $1 must be >= $3, got '$2'." >&2 + exit 1 + fi +} + [ -x "${HARNESS}" ] || { echo "run-startup-bench: harness not found or not executable: ${HARNESS}" >&2 exit 1 @@ -122,6 +137,14 @@ else eval "CMD_ARGV=( ${WORKCELL_STARTUP_CMD} )" fi +# Numeric controls must be sane before benchmarking: RUNS=0 or a non-integer +# would otherwise silently produce no measurements or a misleading STABLE. RUNS +# is validated after any dry-run per-group override above. +validate_int "WORKCELL_STARTUP_ITERATIONS" "${ITERATIONS}" 1 +validate_int "WORKCELL_STARTUP_WARMUP" "${WARMUP}" 0 +validate_int "WORKCELL_STARTUP_RUNS" "${RUNS}" 1 +validate_int "WORKCELL_STARTUP_STABILITY_PCT" "${STABILITY_PCT}" 0 + WORKDIR="$(mktemp -d "${TMPDIR:-/tmp}/startup-bench.XXXXXX")" trap 'rm -rf "${WORKDIR}"' EXIT REPORT="${WORKDIR}/report.md" @@ -162,26 +185,20 @@ run_harness() { # matches the other modes exactly. Emits the same key=value stats line. measure_cold() { # $1 run index (1-based) - local samples=() one if [ "${DRY_RUN}" -eq 1 ]; then + # Dry-run: canned samples, no prep, no launch -- emit the group's stats. local gi=$(($1 - 1)) [ "${gi}" -lt "${#SAMPLE_GROUPS[@]}" ] || gi=$((${#SAMPLE_GROUPS[@]} - 1)) - local canned_arr=() canned - read -ra canned_arr <<<"${SAMPLE_GROUPS[gi]}" - for canned in "${canned_arr[@]}"; do - prep_mode cold - one="$(WORKCELL_STARTUP_SAMPLES_NS="${canned}" "${HARNESS}" cold 1 0)" - samples+=("$(field "${one}" min_ns)") - done - else - local i=0 - while [ "${i}" -lt "${ITERATIONS}" ]; do - prep_mode cold - one="$("${HARNESS}" cold 1 0 -- "${CMD_ARGV[@]}")" - samples+=("$(field "${one}" min_ns)") - i=$((i + 1)) - done + WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" cold "${ITERATIONS}" 0 + return fi + local samples=() one i=0 + while [ "${i}" -lt "${ITERATIONS}" ]; do + prep_mode cold + one="$("${HARNESS}" cold 1 0 -- "${CMD_ARGV[@]}")" + samples+=("$(field "${one}" min_ns)") + i=$((i + 1)) + done WORKCELL_STARTUP_SAMPLES_NS="${samples[*]}" "${HARNESS}" cold "${#samples[@]}" 0 } @@ -213,7 +230,9 @@ while [ "${run_index}" -le "${RUNS}" ]; do if [ "${mode}" = "cold" ]; then line="$(measure_cold "${run_index}")" else - prep_mode "${mode}" + # Dry-run uses canned samples, so it must not execute any prep hook (an + # operator may have live hooks exported from a previous run). + [ "${DRY_RUN}" -eq 1 ] || prep_mode "${mode}" line="$(run_harness "${mode}" "${run_index}")" fi med="$(field "${line}" median_ns)" From eb92f1f8a8191dfb303d2d4b8d9b20149c488cf7 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 11:44:23 -0400 Subject: [PATCH 6/9] ^B Close startup-bench false-STABLE gaps: probe runtime health, require >=2 live runs, fail zero medians (local go test + shellcheck/shfmt/markdownlint green, live runtime unverifiable; user-visible gate/integrity behavior) Follow-up to the C2 driver review (cc74187); three false-publishable integrity bugs: 1) Probe runtime health, not just the client. Auto-detect used `command -v`, so a host with the docker/colima/container CLIENT but no usable daemon selected live mode and then hard-failed on the missing command/prep hooks instead of the documented clean CI-safe skip. Add runtime_usable() (docker info / colima status / container system status) and require it in the detect loop; an explicit WORKCELL_STARTUP_RUNTIME override is respected as-is and skips the probe. 2) Require RUNS >= 2 for a live/reportable capture. With RUNS=1 the cross-run stability section is skipped yet the script exits 0, which the docs say means the gate passed -- a single run has no repeatability evidence. Fail fast on a live run with RUNS<2. Dry-run canned mode keeps whatever RUNS the data implies. 3) Zero median must not read as STABLE. When one run's median was 0 and another nonzero the spread% was forced to 0 (STABLE, exit 0) despite a degenerate result. Treat min<=0 as a degenerate measurement: mark the mode UNSTABLE and fail the gate (exit 2) with a clear message; a 0 ns session start is impossible. Tests: client-only host -> clean skip (fake clients whose health probe fails, shadowing real ones on PATH; runScript now lets a test override PATH cleanly); RUNS=1 live -> fail fast while dry-run RUNS=1 still passes; zero-vs-nonzero medians -> exit 2 UNSTABLE. Existing live tests bumped to RUNS=2 (stub/high threshold to keep the gate deterministic). Doc updated for all three rules. Co-Authored-By: Claude Opus 4.8 --- docs/session-startup-benchmarks.md | 16 ++++ internal/startupbench/bench_test.go | 123 ++++++++++++++++++++++++---- scripts/bench/run-startup-bench.sh | 67 ++++++++++++--- 3 files changed, 179 insertions(+), 27 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index 7c540cc8..d7e9ce6f 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -66,6 +66,18 @@ warm-lane priming — and the numbers, though they may look stable, are not publishable. (The canned dry run below needs no prep hooks; this requirement applies only to live runs.) +Auto-detection only selects a runtime whose daemon is actually **usable** (a +cheap read-only status probe — `docker info` / `colima status` / +`container system status`), not merely that the client binary is installed. A +host with the client but no working runtime falls through to the clean CI-safe +skip (exit `0`) rather than selecting live mode and then failing. An explicit +`WORKCELL_STARTUP_RUNTIME` override is respected as-is and skips the probe. + +A live run also **requires** `WORKCELL_STARTUP_RUNS >= 2`: the stability gate +needs cross-run evidence, so a single-run capture — which would skip the gate yet +still exit `0` — is rejected up front and is not publishable. (The canned dry run +may use whatever `RUNS` the data implies; it is a rehearsal, never publishable.) + For `cold` the driver re-runs `WORKCELL_STARTUP_COLD_PREP` before **every** measured sample and times each start on its own with warmup `0`, then aggregates the per-sample timings through the same stats core. A single session start warms @@ -93,6 +105,10 @@ median, and it **fails** (non-zero exit) if any mode exceeds trustworthy if it repeats; this gate is the evidence that it does. It mirrors the C5 cross-run stability check, made enforcing. +A zero median in any run is treated as a **degenerate** measurement and fails the +gate outright — a 0 ns session start is impossible, so it signals a broken clock +or harness rather than a 0% spread that would otherwise read as `STABLE`. + ### Runner caveats - Numbers are **relative** to the measuring host's hardware and runtime backend; diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 814ffc8a..772c925a 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -36,14 +36,19 @@ func runScript(tb testing.TB, relScript string, env map[string]string, args ...s // Hermetic environment: build the child env explicitly instead of inheriting // os.Environ(), so a developer's exported WORKCELL_STARTUP_* (or any other // stray var) cannot leak in and change behavior. Carry only the few vars the - // bench scripts need to locate tools and temp space, plus what the test sets. - cmd.Env = []string{"PATH=" + os.Getenv("PATH")} + // bench scripts need to locate tools and temp space, plus what the test sets + // (a test-supplied key -- e.g. PATH -- overrides the base, with no duplicate). + base := map[string]string{"PATH": os.Getenv("PATH")} for _, k := range []string{"HOME", "TMPDIR"} { if v, ok := os.LookupEnv(k); ok { - cmd.Env = append(cmd.Env, k+"="+v) + base[k] = v } } for k, v := range env { + base[k] = v + } + cmd.Env = make([]string, 0, len(base)) + for k, v := range base { cmd.Env = append(cmd.Env, k+"="+v) } out, err := cmd.CombinedOutput() @@ -227,7 +232,7 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { "WORKCELL_STARTUP_CMD": "true", "WORKCELL_STARTUP_ITERATIONS": "2", "WORKCELL_STARTUP_WARMUP": "1", - "WORKCELL_STARTUP_RUNS": "1", + "WORKCELL_STARTUP_RUNS": "2", // live runs require >= 2; stub output is constant so the gate stays STABLE // Live runs require each driven mode's prep hook; no-ops suffice here. "WORKCELL_STARTUP_COLD_PREP": ":", "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", @@ -300,11 +305,14 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { warmF := filepath.Join(dir, "warm") chF := filepath.Join(dir, "cachehit") env := map[string]string{ - "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip - "WORKCELL_STARTUP_CMD": "true", - "WORKCELL_STARTUP_ITERATIONS": "3", - "WORKCELL_STARTUP_WARMUP": "0", - "WORKCELL_STARTUP_RUNS": "1", + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": "true", + "WORKCELL_STARTUP_ITERATIONS": "3", + "WORKCELL_STARTUP_WARMUP": "0", + "WORKCELL_STARTUP_RUNS": "2", // live runs require >= 2 runs + // The gate would be noisy timing `true`; this test is about prep counts, + // so widen the threshold so the (real) stability gate never flakes. + "WORKCELL_STARTUP_STABILITY_PCT": "100000000", "WORKCELL_STARTUP_COLD_PREP": "printf c >> " + coldF, "WORKCELL_STARTUP_WARM_PREP": "printf w >> " + warmF, "WORKCELL_STARTUP_CACHE_HIT_PREP": "printf h >> " + chF, @@ -320,15 +328,16 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { } return len(data) } - // Three measured cold samples -> three cold preps (one per sample). - if got := countPreps(coldF); got != 3 { - t.Errorf("cold prep ran %d time(s), want 3 (once per measured sample)", got) + // 3 measured cold samples x 2 runs -> 6 cold preps (one per sample), while + // warm/cache-hit share one prep per pass -> 2 each across the two runs. + if got := countPreps(coldF); got != 6 { + t.Errorf("cold prep ran %d time(s), want 6 (once per measured sample x 2 runs)", got) } - if got := countPreps(warmF); got != 1 { - t.Errorf("warm prep ran %d time(s), want 1 (one prep for the whole pass)", got) + if got := countPreps(warmF); got != 2 { + t.Errorf("warm prep ran %d time(s), want 2 (one prep per pass x 2 runs)", got) } - if got := countPreps(chF); got != 1 { - t.Errorf("cache-hit prep ran %d time(s), want 1 (one prep for the whole pass)", got) + if got := countPreps(chF); got != 2 { + t.Errorf("cache-hit prep ran %d time(s), want 2 (one prep per pass x 2 runs)", got) } // The per-sample timings are aggregated through the harness stats core, so // the cold row must still read like a normal n=3 distribution. @@ -413,7 +422,10 @@ func TestDriverPreservesCommandArgv(t *testing.T) { "WORKCELL_STARTUP_CMD": rec + " alpha 'beta gamma'", "WORKCELL_STARTUP_ITERATIONS": "1", "WORKCELL_STARTUP_WARMUP": "0", - "WORKCELL_STARTUP_RUNS": "1", + "WORKCELL_STARTUP_RUNS": "2", // live runs require >= 2 runs + // This test is about argv boundaries, not stability; widen the threshold + // so the (real) gate timing the recorder never flakes. + "WORKCELL_STARTUP_STABILITY_PCT": "100000000", // Live runs require each driven mode's prep hook; no-ops suffice here. "WORKCELL_STARTUP_COLD_PREP": ":", "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", @@ -465,6 +477,83 @@ func TestDriverLiveRequiresPrepHooks(t *testing.T) { } } +func TestDriverSkipsWhenRuntimeClientButNoDaemon(t *testing.T) { + // A host with the runtime CLIENT installed but no usable daemon must fall + // through to the clean CI-safe skip (exit 0), not select live mode and then + // error on the missing command/prep hooks. Fake colima/container/docker + // clients that exist (so `command -v` finds them) but whose health probe + // fails (exit 1); shadow any real ones by putting the fakes first on PATH. + dir := t.TempDir() + for _, name := range []string{"colima", "container", "docker"} { + // Every invocation (incl. the health probe) fails; existence still lets + // `command -v` succeed. + if err := os.WriteFile(filepath.Join(dir, name), + []byte("#!/usr/bin/env bash\nexit 1\n"), 0o755); err != nil { + t.Fatalf("write fake %s: %v", name, err) + } + } + env := map[string]string{ + // Fakes first, then the real system bins the driver needs (bash/date/...). + "PATH": dir + string(os.PathListSeparator) + os.Getenv("PATH"), + // No WORKCELL_STARTUP_RUNTIME (force auto-detect), no SAMPLES_NS. + } + code, out := runScript(t, driver, env) + if code != 0 { + t.Fatalf("client-only host should cleanly skip (exit 0), got %d: %s", code, out) + } + if !strings.Contains(out, "skipping") || !strings.Contains(out, "no container runtime") { + t.Errorf("expected clean skip when only the client (no daemon) is present: %s", out) + } +} + +func TestDriverLiveRequiresTwoRuns(t *testing.T) { + // A single-run live capture has no repeatability evidence; the docs say exit 0 + // means the stability gate passed, so a live run must require RUNS >= 2 and + // fail fast. The canned dry-run path may use whatever RUNS the data implies. + live := map[string]string{ + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": "true", + "WORKCELL_STARTUP_RUNS": "1", + "WORKCELL_STARTUP_COLD_PREP": ":", + "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", + "WORKCELL_STARTUP_WARM_PREP": ":", + } + code, out := runScript(t, driver, live) + if code == 0 { + t.Fatalf("live run with RUNS=1 should fail fast, got exit 0: %s", out) + } + if !strings.Contains(out, "WORKCELL_STARTUP_RUNS") || !strings.Contains(out, ">= 2") { + t.Errorf("error should require RUNS >= 2 for a live run: %s", out) + } + + // Dry-run with a single canned group and RUNS=1 is a rehearsal, not gated or + // publishable, and must keep working. + code, out = runScript(t, driver, map[string]string{ + "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50", + "WORKCELL_STARTUP_RUNS": "1", + }) + if code != 0 { + t.Fatalf("dry-run with RUNS=1 should still pass, got %d: %s", code, out) + } +} + +func TestDriverZeroMedianIsUnstable(t *testing.T) { + // A median of 0 in one run and nonzero in another is a degenerate/broken + // measurement (a 0 ns session start is impossible), not a 0%% spread that + // reads as STABLE. The gate must fail (exit 2). + code, out := runScript(t, driver, + map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "0 0 0;10 20 30"}) + if code != 2 { + t.Fatalf("zero-vs-nonzero medians should fail the gate (exit 2), got %d: %s", code, out) + } + if !strings.Contains(out, "Stability gate: UNSTABLE") { + t.Errorf("expected UNSTABLE verdict for a zero median: %s", out) + } + if strings.Contains(out, "Stability gate: STABLE") { + t.Errorf("a zero median must not read as STABLE: %s", out) + } +} + func TestDriverStabilityThresholdIsConfigurable(t *testing.T) { // The same 20->21 spread (5%%) passes at the default threshold but a 1%% // threshold rejects it, proving the gate reads the configured bound. diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index 17409bb3..2c3dacd5 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -70,6 +70,18 @@ validate_int() { fi } +# Probe that an auto-detected runtime's daemon is actually usable (a cheap, +# read-only status call), not just that the client binary exists. Returns +# non-zero when the runtime is unusable so detection falls through to the skip. +runtime_usable() { + case "$1" in + docker) docker info ;; + colima) colima status ;; + container) container system status ;; + *) return 1 ;; + esac >/dev/null 2>&1 +} + [ -x "${HARNESS}" ] || { echo "run-startup-bench: harness not found or not executable: ${HARNESS}" >&2 exit 1 @@ -92,8 +104,13 @@ if [ -n "${SAMPLES}" ]; then else detected="${WORKCELL_STARTUP_RUNTIME:-}" if [ -z "${detected}" ]; then + # Auto-detect only selects a runtime whose daemon is actually usable: a host + # can have the client binary installed with no working runtime, which would + # otherwise pick live mode and then hard-fail instead of skipping cleanly. An + # explicit WORKCELL_STARTUP_RUNTIME override skips this probe (respected as-is). for candidate in colima container docker; do - if command -v "${candidate}" >/dev/null 2>&1; then + command -v "${candidate}" >/dev/null 2>&1 || continue + if runtime_usable "${candidate}"; then detected="${candidate}" break fi @@ -145,6 +162,16 @@ validate_int "WORKCELL_STARTUP_WARMUP" "${WARMUP}" 0 validate_int "WORKCELL_STARTUP_RUNS" "${RUNS}" 1 validate_int "WORKCELL_STARTUP_STABILITY_PCT" "${STABILITY_PCT}" 0 +# A gated, publishable live capture needs cross-run repeatability evidence, so +# require at least two runs: with one run the stability section is skipped and a +# 0 exit would misleadingly read as "gate passed". Dry-run canned mode may use +# whatever RUNS the canned data implies -- it is a rehearsal, never publishable. +if [ "${DRY_RUN}" -eq 0 ] && [ "${RUNS}" -lt 2 ]; then + echo "run-startup-bench: a live run requires WORKCELL_STARTUP_RUNS >= 2 for" \ + "cross-run stability evidence (got ${RUNS}); a single run is not publishable." >&2 + exit 1 +fi + WORKDIR="$(mktemp -d "${TMPDIR:-/tmp}/startup-bench.XXXXXX")" trap 'rm -rf "${WORKDIR}"' EXIT REPORT="${WORKDIR}/report.md" @@ -266,31 +293,47 @@ if [ "${RUNS}" -ge 2 ]; then all_runs="${WORKDIR}/all-runs" cat "${WORKDIR}"/run-* >"${all_runs}" - worst="$(awk -v thr="${STABILITY_PCT}" ' + # Emit " " on stdout; the per-mode rows go to + # the report via stderr. A zero median in any run is a degenerate/broken + # measurement (a 0 ns session start is impossible), not a 0% spread -- flag it + # so the gate fails instead of reading STABLE. + gate_line="$(awk -v thr="${STABILITY_PCT}" ' { m = $1; v = $2 if (!(m in seen)) { order[++k] = m; seen[m] = 1; min[m] = v; max[m] = v } if (v < min[m]) min[m] = v if (v > max[m]) max[m] = v } END { - worst = 0 + worst = 0; degenerate = 0 for (i = 1; i <= k; i++) { m = order[i]; s = max[m] - min[m] - p = (min[m] > 0) ? s * 100.0 / min[m] : 0 - verdict = (p > thr) ? "UNSTABLE" : "STABLE" - if (p > worst) worst = p - printf "| %s | %d | %d | %d | %.1f | %s |\n", m, min[m], max[m], s, p, verdict > "/dev/stderr" + if (min[m] <= 0) { + degenerate = 1 + printf "| %s | %d | %d | %d | n/a | UNSTABLE |\n", m, min[m], max[m], s > "/dev/stderr" + } else { + p = s * 100.0 / min[m] + verdict = (p > thr) ? "UNSTABLE" : "STABLE" + if (p > worst) worst = p + printf "| %s | %d | %d | %d | %.1f | %s |\n", m, min[m], max[m], s, p, verdict > "/dev/stderr" + } } - printf "%.1f", worst + printf "%.1f %d", worst, degenerate } ' "${all_runs}" 2>>"${REPORT}")" echo >>"${REPORT}" + worst="${gate_line%% *}" + degenerate="${gate_line##* }" - if awk -v w="${worst}" -v thr="${STABILITY_PCT}" 'BEGIN { exit (w > thr) ? 0 : 1 }'; then + if [ "${degenerate}" -eq 1 ]; then + GATE_STATUS="UNSTABLE" + elif awk -v w="${worst}" -v thr="${STABILITY_PCT}" 'BEGIN { exit (w > thr) ? 0 : 1 }'; then GATE_STATUS="UNSTABLE" fi { if [ "${GATE_STATUS}" = "STABLE" ]; then echo "Stability gate: STABLE (max cross-run median spread ${worst}% <= ${STABILITY_PCT}%)." + elif [ "${degenerate}" -eq 1 ]; then + echo "Stability gate: UNSTABLE (a mode reported a zero median across runs" \ + "-- degenerate measurement, not a fast start)." else echo "Stability gate: UNSTABLE (max cross-run median spread ${worst}% > ${STABILITY_PCT}%)." fi @@ -305,6 +348,10 @@ if [ -n "${OUTPUT_PATH}" ]; then fi if [ "${GATE_STATUS}" = "UNSTABLE" ]; then - echo "run-startup-bench: cross-run stability gate FAILED (spread ${worst}% > ${STABILITY_PCT}%)" >&2 + if [ "${degenerate:-0}" -eq 1 ]; then + echo "run-startup-bench: cross-run stability gate FAILED (degenerate zero median)" >&2 + else + echo "run-startup-bench: cross-run stability gate FAILED (spread ${worst}% > ${STABILITY_PCT}%)" >&2 + fi exit 2 fi From 1f1a0fcbc6d5de999e95182fd049c3ddd8e58344 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 12:45:15 -0400 Subject: [PATCH 7/9] ^d Trim startup-bench doc and test comments to fit the PR-shape budget (the 5 rounds of integrity hardening pushed the PR to 1312 lines, over the 1200 PR-shape limit; tighten verbose regression comments and redundant methodology prose without changing any behavior or dropping a test assertion) (go test/shellcheck/shfmt/markdownlint green, no assertion removed; docs/test-comment-only trim) --- docs/session-startup-benchmarks.md | 156 +++++++++-------------- internal/startupbench/bench_test.go | 185 ++++++++++------------------ scripts/bench/run-startup-bench.sh | 48 +++----- scripts/bench/startup-bench.sh | 26 ++-- 4 files changed, 151 insertions(+), 264 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index d7e9ce6f..f8f9bd9d 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -9,9 +9,8 @@ records the methodology and the rerun steps; it is the C2 sibling of [syscall-shim-benchmarks.md](syscall-shim-benchmarks.md) (C5). The numbers are produced on a host with a live container runtime, not in the -PR-blocking CI lanes: a real session start needs a booted VM, so the cost only -exists where that runtime is available. The results tables below are -**placeholders pending a live capture** — see +PR-blocking CI lanes (a real session start needs a booted VM). The results tables +below are **placeholders pending a live capture** — see [Filling in the numbers](#filling-in-the-numbers). Do not treat the template values as measured. @@ -27,10 +26,8 @@ modes that span the latency shapes C2 targets: | `warm` | image cached and a kept-warm session available | best-case start off the kept-warm lane | | `cache-hit` | image cached, no kept-warm session | the image-cache win alone, without the warm lane | -The `cold` vs `warm` delta is the headline C2 number: how much the cache plus the -kept-warm lane save off a first start. The `cache-hit` mode separates the -image-cache contribution from the kept-warm-lane contribution so each optimization -can be credited independently. +The `cold` vs `warm` delta is the headline C2 number; `cache-hit` separates the +image-cache win from the kept-warm-lane win so each is credited independently. ## Methodology @@ -41,13 +38,9 @@ measured times, and reports the sample distribution. The stats conventions match the C5 exec-guard harness exactly, so the two pages' numbers are directly comparable: -- **median** — `sorted[floor(n/2)]`, robust to scheduler and I/O outliers; the - headline figure. -- **p90** — `sorted[floor(n*9/10)]` (clamped to the last sample), the tail a user - actually feels on a slow start. -- **mean / stddev** — population mean and standard deviation, reported alongside - the median so a skewed distribution is visible. -- **min / max** — the observed range. +- **median** (`sorted[floor(n/2)]`, the outlier-robust headline), **p90** + (`sorted[floor(n*9/10)]` clamped, the tail a slow start shows), **mean/stddev** + (population, to expose a skewed distribution), and **min/max** (observed range). The driver (`scripts/bench/run-startup-bench.sh`) establishes each mode's runtime state through a per-mode prep hook (`WORKCELL_STARTUP_COLD_PREP` / @@ -57,80 +50,50 @@ image but leaving the warm lane down for `cache-hit`, or pre-pulling and priming the warm lane for `warm`), then times the configured `WORKCELL_STARTUP_CMD` for that mode. The whole measurement is repeated for `WORKCELL_STARTUP_RUNS` passes. -A live run **requires** the prep hook for every driven mode: if any of -`WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_CACHE_HIT_PREP`, or -`WORKCELL_STARTUP_WARM_PREP` is unset the driver fails fast (naming the missing -mode and variable) rather than running. Without a mode's hook the harness would -measure whatever runtime state happened to be present — no real cold eviction or -warm-lane priming — and the numbers, though they may look stable, are not -publishable. (The canned dry run below needs no prep hooks; this requirement -applies only to live runs.) - -Auto-detection only selects a runtime whose daemon is actually **usable** (a -cheap read-only status probe — `docker info` / `colima status` / -`container system status`), not merely that the client binary is installed. A -host with the client but no working runtime falls through to the clean CI-safe -skip (exit `0`) rather than selecting live mode and then failing. An explicit -`WORKCELL_STARTUP_RUNTIME` override is respected as-is and skips the probe. - -A live run also **requires** `WORKCELL_STARTUP_RUNS >= 2`: the stability gate -needs cross-run evidence, so a single-run capture — which would skip the gate yet -still exit `0` — is rejected up front and is not publishable. (The canned dry run -may use whatever `RUNS` the data implies; it is a rehearsal, never publishable.) +Live runs are guarded so a misconfigured capture cannot look publishable: + +- **Every driven mode's prep hook is required** (`*_COLD_PREP` / + `*_CACHE_HIT_PREP` / `*_WARM_PREP`); an unset hook fails fast rather than + measuring whatever state happened to be present. +- **The runtime must be usable, not just installed** — a cheap read-only probe + (`docker info` / `colima status` / `container system status`) sends a + client-only host to the clean CI-safe skip. `WORKCELL_STARTUP_RUNTIME` overrides + and skips the probe. +- **`WORKCELL_STARTUP_RUNS >= 2`** — the stability gate needs cross-run evidence, + so a single-run capture is rejected. For `cold` the driver re-runs `WORKCELL_STARTUP_COLD_PREP` before **every** -measured sample and times each start on its own with warmup `0`, then aggregates -the per-sample timings through the same stats core. A single session start warms -the cache the next start would otherwise hit, so evicting only once before the -pass would leave just the first sample genuinely cold; the per-sample re-prep -keeps every `cold` sample a true first start. The cold-prep hook must therefore -be **repeatable** — it runs once per measured sample, not once per pass. The -`warm` and `cache-hit` modes legitimately share one prep for the whole pass and -keep the configured `WORKCELL_STARTUP_WARMUP` to settle first-touch page-cache -and loader costs before measuring. - -`WORKCELL_STARTUP_CMD` is parsed with shell quoting into the target argv, so an -argument containing spaces keeps its boundary. Quote such arguments exactly as -you would on a shell command line — e.g. -`WORKCELL_STARTUP_CMD="./scripts/workcell --workspace '/path/with space'"` reaches -the target as three argv elements, not four word-split tokens. +measured sample (warmup `0`) and aggregates the per-sample timings — a start warms +the cache the next start would hit, so evicting once per pass would leave only the +first sample truly cold, and the cold-prep hook must be **repeatable**. `warm` and +`cache-hit` share one prep per pass and keep `WORKCELL_STARTUP_WARMUP`. -### The cross-run stability gate +`WORKCELL_STARTUP_CMD` is parsed with shell quoting, so a spaced argument keeps its +boundary (`--workspace '/path/with space'` stays one argv element, not word-split +tokens). The canned dry run needs no prep hooks or runtime and never executes +hooks — these guards are live-only. -Reproducibility is the C2 acceptance bar, so the driver does not just print the -per-run numbers — it enforces them. After all runs it computes, for each mode, -the spread of the run-to-run **median** as a percentage of the smallest run's -median, and it **fails** (non-zero exit) if any mode exceeds -`WORKCELL_STARTUP_STABILITY_PCT` (default 15%). A published number is only -trustworthy if it repeats; this gate is the evidence that it does. It mirrors the -C5 cross-run stability check, made enforcing. +### The cross-run stability gate -A zero median in any run is treated as a **degenerate** measurement and fails the -gate outright — a 0 ns session start is impossible, so it signals a broken clock -or harness rather than a 0% spread that would otherwise read as `STABLE`. +Reproducibility is the C2 acceptance bar, so the driver enforces it: after all +runs it computes, per mode, the run-to-run **median** spread as a percentage of +the smallest run's median, and **fails** (non-zero exit) if any mode exceeds +`WORKCELL_STARTUP_STABILITY_PCT` (default 15%) — the evidence a published number +repeats. A zero median fails the gate outright: a 0 ns start is impossible, so it +signals a broken clock rather than a 0% spread that would read as `STABLE`. ### Runner caveats - Numbers are **relative** to the measuring host's hardware and runtime backend; - treat the cold-vs-warm delta, not the absolute medians, as the portable signal, - and re-measure on the target host for absolute figures. -- The `cold` mode depends on the prep hook genuinely evicting cached state. If the - hook is a no-op the `cold` and `warm` numbers converge — that is a - misconfiguration, not a fast cold start. -- The cold-prep hook runs once per measured `cold` sample (not once per pass), so - it must be **repeatable and idempotent** — every invocation has to leave the - same fully-evicted, no-warm-lane state. A hook that only evicts on its first - call will silently measure cache-hits for the remaining samples. + treat the cold-vs-warm delta, not the absolute medians, as the portable signal. - Session start includes VM boot, which is noisier than a userspace microbenchmark; - expect a wider stddev than the C5 exec-guard numbers and keep the stability - threshold accordingly. + expect a wider stddev than the C5 exec-guard numbers. ## Results -**Status: numbers pending live capture.** The tables below are templates. There -is a sustained upstream mirror outage blocking the container image build at the -time of writing, so no live runtime was available to capture real figures. -Replace the `TODO` cells once a live run is captured (see +**Status: numbers pending live capture.** The tables below are templates — a +sustained upstream mirror outage blocking the image build left no live runtime to +capture real figures. Replace the `TODO` cells once a live run is captured (see [Filling in the numbers](#filling-in-the-numbers)). Do not fabricate values. ### Start latency by mode (median of N samples, R runs) @@ -152,24 +115,22 @@ Replace the `TODO` cells once a live run is captured (see ## Filling in the numbers 1. On a host with a live container runtime, wire the prep hooks and session - command, then run the driver (see [Rerunning](#rerunning) below) with - `WORKCELL_STARTUP_OUTPUT` set to capture the Markdown report. -2. Confirm the run exits `0` — that means the cross-run stability gate passed and - the numbers are reproducible. A non-zero exit means the spread was too wide to - publish; investigate before transcribing. -3. Transcribe the report's per-mode medians, p90s and the stability table into the - tables above, fill the `vs cold` column with the measured deltas, and record - the host, runtime backend, and `N`/`R` used. + command, then run the driver (see [Rerunning](#rerunning)) with + `WORKCELL_STARTUP_OUTPUT` set to capture the report. +2. Confirm the run exits `0` (stability gate passed, numbers reproducible). A + non-zero exit means the spread was too wide to publish — investigate first. +3. Transcribe the report's per-mode medians, p90s and stability table into the + tables above, fill `vs cold` with the deltas, and record the host, runtime + backend, and `N`/`R`. ## Rerunning From the repository root on a host with a container runtime: ```sh -# Wire the prep hooks and the session-start command to your runtime, then: -# WORKCELL_STARTUP_CMD is parsed with shell quoting; quote args with spaces, -# e.g. ...--workspace '/path/with space'. WORKCELL_STARTUP_COLD_PREP is re-run -# before every cold sample, so make it repeatable (idempotent eviction). +# Wire the prep hooks + session-start command to your runtime. WORKCELL_STARTUP_CMD +# is shell-quoted (quote args with spaces); COLD_PREP is re-run per sample (make +# it idempotent); a live run needs all three prep hooks and RUNS >= 2. export WORKCELL_STARTUP_CMD='./scripts/workcell ' export WORKCELL_STARTUP_COLD_PREP='' export WORKCELL_STARTUP_CACHE_HIT_PREP='' @@ -182,13 +143,10 @@ export WORKCELL_STARTUP_OUTPUT=session-startup-results.md Tunable via environment: `WORKCELL_STARTUP_ITERATIONS`, `WORKCELL_STARTUP_WARMUP` (forced to `0` for `cold`), `WORKCELL_STARTUP_RUNS`, -`WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, -`WORKCELL_STARTUP_COLD_PREP`, `WORKCELL_STARTUP_CACHE_HIT_PREP`, -`WORKCELL_STARTUP_WARM_PREP`, and `WORKCELL_STARTUP_OUTPUT`. The numeric controls -are validated up front — `WORKCELL_STARTUP_ITERATIONS` and `WORKCELL_STARTUP_RUNS` -must be integers `>= 1`, and `WORKCELL_STARTUP_WARMUP` / -`WORKCELL_STARTUP_STABILITY_PCT` integers `>= 0`; anything else fails fast rather -than silently producing no measurements or a misleading `STABLE`. +`WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, the three `*_PREP` hooks, +and `WORKCELL_STARTUP_OUTPUT`. The numeric controls are validated up front +(`ITERATIONS`/`RUNS` integers `>= 1`, `WARMUP`/`STABILITY_PCT` integers `>= 0`); +anything else fails fast rather than silently misreporting. ### Dry run without a runtime @@ -213,10 +171,6 @@ container build or a live VM. ## Where this fits The harness and driver live in `scripts/bench/` alongside the C5 exec-guard -benchmark. A scheduled, non-PR-blocking workflow lane that captures the live -numbers on a runtime-capable runner is **deferred** until the image build is -unblocked; when added it will follow the `bench.yml` pattern and upload the -Markdown report as an artifact. See -[syscall-shim-benchmarks.md](syscall-shim-benchmarks.md) for the sibling C5 -baselines and [github-workflows.md](github-workflows.md) for the workflow -inventory. +benchmark ([syscall-shim-benchmarks.md](syscall-shim-benchmarks.md)). A scheduled, +non-PR-blocking lane that captures the live numbers on a runtime-capable runner is +**deferred** until the image build is unblocked. diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 772c925a..5d423b27 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -1,11 +1,9 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright 2026 Omkhar Arasaratnam -// Package startupbench holds the tests that pin the pure logic of the C2 -// session-start latency benchmark scripts (scripts/bench/startup-bench.sh and -// scripts/bench/run-startup-bench.sh): the median/p90/stddev math, the -// cross-run stability gate, and the skip-when-no-runtime behavior. These run -// under `go test ./...` and need no container runtime. +// Package startupbench pins the pure logic of the C2 session-start latency bench +// scripts (median/p90/stddev math, stability gate, driver skip/validation guards) +// so it runs under `go test ./...` with no runtime. package startupbench import ( @@ -26,18 +24,15 @@ func repoRoot(tb testing.TB) string { return filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) } -// runScript runs a bench script with the given args and extra environment, -// returning its exit code and combined stdout+stderr. +// runScript runs a bench script with args + extra env, returning exit code and +// combined output. The child env is built explicitly (not inherited) so a stray +// exported WORKCELL_STARTUP_* can't leak in: only PATH/HOME/TMPDIR + test-set keys +// are carried (a test key overrides the base). func runScript(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string) { tb.Helper() root := repoRoot(tb) cmd := exec.Command(filepath.Join(root, filepath.FromSlash(relScript)), args...) cmd.Dir = root - // Hermetic environment: build the child env explicitly instead of inheriting - // os.Environ(), so a developer's exported WORKCELL_STARTUP_* (or any other - // stray var) cannot leak in and change behavior. Carry only the few vars the - // bench scripts need to locate tools and temp space, plus what the test sets - // (a test-supplied key -- e.g. PATH -- overrides the base, with no duplicate). base := map[string]string{"PATH": os.Getenv("PATH")} for _, k := range []string{"HOME", "TMPDIR"} { if v, ok := os.LookupEnv(k); ok { @@ -62,6 +57,14 @@ func runScript(tb testing.TB, relScript string, env map[string]string, args ...s return -1, "" } +// writeExec writes an executable helper script for a test. +func writeExec(tb testing.TB, path, script string) { + tb.Helper() + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + tb.Fatalf("write %s: %v", path, err) + } +} + const harness = "scripts/bench/startup-bench.sh" const driver = "scripts/bench/run-startup-bench.sh" @@ -77,8 +80,8 @@ func parseFields(line string) map[string]string { } func TestHarnessStatsOddSampleSet(t *testing.T) { - // Deliberately unsorted; the harness sorts before computing. n=5 so median - // is the 3rd value and p90 (index floor(5*9/10)=4) is the max. + // Deliberately unsorted; the harness sorts first. n=5 so median is the 3rd + // value and p90 (index floor(5*9/10)=4) is the max. code, out := runScript(t, harness, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "50 10 40 20 30"}, "cold", "0", "0") @@ -142,9 +145,8 @@ func TestHarnessRejectsNonIntegerSample(t *testing.T) { } func TestHarnessLivePathTimesTarget(t *testing.T) { - // No canned samples: the harness times a benign target. This exercises the - // real clock + timing pipeline and confirms n == iterations with a - // non-negative median. Values are host-dependent, so we assert structure. + // No canned samples: times a benign target, exercising the real clock. Values + // are host-dependent, so we assert structure (n and the fields present). code, out := runScript(t, harness, nil, "warm", "3", "1", "--", "true") if code != 0 { t.Fatalf("exit %d, out=%q", code, out) @@ -189,8 +191,8 @@ func TestDriverDryRunStablePasses(t *testing.T) { } func TestDriverDryRunUnstableFailsGate(t *testing.T) { - // Two ';'-separated per-run groups with very different medians (20 vs 200) - // blow past the default 15%% stability threshold, so the gate must fail. + // Two per-run groups with very different medians (20 vs 200) blow past the + // default 15% threshold, so the gate must fail. code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30;100 200 300"}) if code != 2 { @@ -205,35 +207,24 @@ func TestDriverDryRunUnstableFailsGate(t *testing.T) { } func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { - // Regression for the C2 driver findings: - // P1 -- cold must not spend its freshly-evicted state on a discarded - // warmup launch, so the driver forces warmup=0 for cold while other - // modes keep the configured warmup. - // P2 -- the driver must drive the documented three-mode set, including - // cache-hit, not just cold+warm. - // A stub harness (wired via the WORKCELL_STARTUP_HARNESS test seam) records - // the mode + warmup it was invoked with so we can assert the argv the driver - // actually passes on the live path. + // cold forces warmup=0; the driver drives all three modes incl. cache-hit. A + // stub harness (WORKCELL_STARTUP_HARNESS seam) records the mode + warmup it got. dir := t.TempDir() logPath := filepath.Join(dir, "harness.log") stub := filepath.Join(dir, "stub-harness.sh") - script := "#!/usr/bin/env bash\n" + - "set -euo pipefail\n" + - "mode=\"$1\"; iters=\"$2\"; warmup=\"$3\"\n" + - "printf '%s %s\\n' \"$mode\" \"$warmup\" >> \"${HARNESS_LOG}\"\n" + - "printf 'mode=%s n=%s mean_ns=1 median_ns=1 p90_ns=1 stddev_ns=0 min_ns=1 max_ns=1\\n' \"$mode\" \"$iters\"\n" - if err := os.WriteFile(stub, []byte(script), 0o755); err != nil { - t.Fatalf("write stub harness: %v", err) - } + writeExec(t, stub, "#!/usr/bin/env bash\n"+ + "set -euo pipefail\n"+ + "mode=\"$1\"; iters=\"$2\"; warmup=\"$3\"\n"+ + "printf '%s %s\\n' \"$mode\" \"$warmup\" >> \"${HARNESS_LOG}\"\n"+ + "printf 'mode=%s n=%s mean_ns=1 median_ns=1 p90_ns=1 stddev_ns=0 min_ns=1 max_ns=1\\n' \"$mode\" \"$iters\"\n") env := map[string]string{ - "HARNESS_LOG": logPath, - "WORKCELL_STARTUP_HARNESS": stub, - "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip - "WORKCELL_STARTUP_CMD": "true", - "WORKCELL_STARTUP_ITERATIONS": "2", - "WORKCELL_STARTUP_WARMUP": "1", - "WORKCELL_STARTUP_RUNS": "2", // live runs require >= 2; stub output is constant so the gate stays STABLE - // Live runs require each driven mode's prep hook; no-ops suffice here. + "HARNESS_LOG": logPath, + "WORKCELL_STARTUP_HARNESS": stub, + "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip + "WORKCELL_STARTUP_CMD": "true", + "WORKCELL_STARTUP_ITERATIONS": "2", + "WORKCELL_STARTUP_WARMUP": "1", + "WORKCELL_STARTUP_RUNS": "2", // live requires >=2; stub output is constant so the gate stays STABLE "WORKCELL_STARTUP_COLD_PREP": ":", "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", "WORKCELL_STARTUP_WARM_PREP": ":", @@ -245,7 +236,6 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { if !strings.Contains(out, "| cache-hit |") { t.Errorf("report missing cache-hit row (P2): %s", out) } - data, err := os.ReadFile(logPath) if err != nil { t.Fatalf("read harness log: %v", err) @@ -274,10 +264,8 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { } func TestRunScriptEnvIsHermetic(t *testing.T) { - // A developer's exported WORKCELL_STARTUP_* must not leak into the script - // driver tests. Export a stray SAMPLES_NS + prep hook in the parent process; - // a no-runtime run must still cleanly SKIP (not turn into a canned dry run) - // and must not execute the leaked prep hook. + // A stray exported WORKCELL_STARTUP_* must not leak in: a no-runtime run must + // still cleanly SKIP (not become a canned dry run) or run the leaked hook. t.Setenv("WORKCELL_STARTUP_SAMPLES_NS", "999") t.Setenv("WORKCELL_STARTUP_COLD_PREP", "echo LEAKED_PREP_RAN") code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_RUNTIME": "none"}) @@ -293,13 +281,8 @@ func TestRunScriptEnvIsHermetic(t *testing.T) { } func TestDriverColdRepsPerMeasuredSample(t *testing.T) { - // Regression for the C2 cold-prep finding: a single session start warms the - // cache the next sample would otherwise hit, so evicting once before the - // whole pass leaves only the first sample genuinely cold. The driver must - // re-run WORKCELL_STARTUP_COLD_PREP before EVERY measured cold sample, while - // warm/cache-hit legitimately share one prep for their whole pass. Verified on - // the live path (prep is dry-run-suppressed), timing a benign command; the - // prep hooks append a byte per invocation so we can count them. + // cold re-runs COLD_PREP before every measured sample (a start warms the cache); + // warm/cache-hit share one prep. Hooks append a byte per call so we count them. dir := t.TempDir() coldF := filepath.Join(dir, "cold") warmF := filepath.Join(dir, "warm") @@ -309,9 +292,9 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { "WORKCELL_STARTUP_CMD": "true", "WORKCELL_STARTUP_ITERATIONS": "3", "WORKCELL_STARTUP_WARMUP": "0", - "WORKCELL_STARTUP_RUNS": "2", // live runs require >= 2 runs - // The gate would be noisy timing `true`; this test is about prep counts, - // so widen the threshold so the (real) stability gate never flakes. + "WORKCELL_STARTUP_RUNS": "2", + // This test is about prep counts, not stability; widen the threshold so + // the (real) gate timing `true` never flakes. "WORKCELL_STARTUP_STABILITY_PCT": "100000000", "WORKCELL_STARTUP_COLD_PREP": "printf c >> " + coldF, "WORKCELL_STARTUP_WARM_PREP": "printf w >> " + warmF, @@ -328,8 +311,8 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { } return len(data) } - // 3 measured cold samples x 2 runs -> 6 cold preps (one per sample), while - // warm/cache-hit share one prep per pass -> 2 each across the two runs. + // 3 cold samples x 2 runs -> 6 cold preps; warm/cache-hit share one prep per + // pass -> 2 each. if got := countPreps(coldF); got != 6 { t.Errorf("cold prep ran %d time(s), want 6 (once per measured sample x 2 runs)", got) } @@ -339,17 +322,14 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { if got := countPreps(chF); got != 2 { t.Errorf("cache-hit prep ran %d time(s), want 2 (one prep per pass x 2 runs)", got) } - // The per-sample timings are aggregated through the harness stats core, so - // the cold row must still read like a normal n=3 distribution. if !strings.Contains(out, "| cold |") || !strings.Contains(out, " 3 |") { t.Errorf("aggregated cold row missing/incorrect: %s", out) } } func TestDriverDryRunSkipsPrep(t *testing.T) { - // A canned dry run must NEVER execute prep hooks, even if an operator has live - // hooks exported from a previous run. The hooks would append to a marker file; - // after a dry run that file must not exist. + // A canned dry run must NEVER execute prep hooks, even with live hooks + // exported; the marker file must not exist afterward. dir := t.TempDir() marker := filepath.Join(dir, "prep-ran") env := map[string]string{ @@ -372,10 +352,8 @@ func TestDriverDryRunSkipsPrep(t *testing.T) { } func TestDriverRejectsInvalidNumericControls(t *testing.T) { - // RUNS=0 / non-numeric controls could make the driver exit 0 with no - // benchmarking or a misleading STABLE, so each numeric control is validated as - // an integer at/above its floor. Uses the canned dry-run path so no runtime is - // needed; validation runs regardless of live vs dry-run. + // RUNS=0 / non-numeric controls could exit 0 with no benchmarking or a + // misleading STABLE, so each control is validated as an integer at its floor. cases := []struct{ name, key, val string }{ {"RUNS_zero", "WORKCELL_STARTUP_RUNS", "0"}, {"RUNS_nonnumeric", "WORKCELL_STARTUP_RUNS", "abc"}, @@ -401,32 +379,24 @@ func TestDriverRejectsInvalidNumericControls(t *testing.T) { } func TestDriverPreservesCommandArgv(t *testing.T) { - // Regression for the WORKCELL_STARTUP_CMD word-splitting finding: an argument - // with spaces (e.g. --workspace '/path/with space') must reach the target as - // a single argv element, not be split/globbed. A recorder script writes the - // argv it was launched with; the last launch wins (it truncates each time), - // so the constant command leaves a deterministic argv regardless of mode. + // WORKCELL_STARTUP_CMD is shell-quoted: a spaced arg (--workspace '/a b') must + // reach the target as one argv element. The recorder leaves the last launch. dir := t.TempDir() argvF := filepath.Join(dir, "argv") rec := filepath.Join(dir, "record.sh") - script := "#!/usr/bin/env bash\n" + - "set -euo pipefail\n" + - ": > \"${ARGV_FILE}\"\n" + - "for a in \"$@\"; do printf '%s\\n' \"$a\" >> \"${ARGV_FILE}\"; done\n" - if err := os.WriteFile(rec, []byte(script), 0o755); err != nil { - t.Fatalf("write recorder: %v", err) - } + writeExec(t, rec, "#!/usr/bin/env bash\n"+ + "set -euo pipefail\n"+ + ": > \"${ARGV_FILE}\"\n"+ + "for a in \"$@\"; do printf '%s\\n' \"$a\" >> \"${ARGV_FILE}\"; done\n") env := map[string]string{ "ARGV_FILE": argvF, "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip "WORKCELL_STARTUP_CMD": rec + " alpha 'beta gamma'", "WORKCELL_STARTUP_ITERATIONS": "1", "WORKCELL_STARTUP_WARMUP": "0", - "WORKCELL_STARTUP_RUNS": "2", // live runs require >= 2 runs - // This test is about argv boundaries, not stability; widen the threshold - // so the (real) gate timing the recorder never flakes. - "WORKCELL_STARTUP_STABILITY_PCT": "100000000", - // Live runs require each driven mode's prep hook; no-ops suffice here. + "WORKCELL_STARTUP_RUNS": "2", + // About argv boundaries, not stability; widen the threshold. + "WORKCELL_STARTUP_STABILITY_PCT": "100000000", "WORKCELL_STARTUP_COLD_PREP": ":", "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", "WORKCELL_STARTUP_WARM_PREP": ":", @@ -447,10 +417,8 @@ func TestDriverPreservesCommandArgv(t *testing.T) { } func TestDriverLiveRequiresPrepHooks(t *testing.T) { - // Regression for the prep-hook finding: on a LIVE run, a missing mode prep - // hook must fail fast (naming the mode + env var) instead of silently - // measuring whatever runtime state is present and emitting publishable-looking - // numbers. The dry-run path needs no prep hooks and must keep passing. + // On a LIVE run a missing mode prep hook must fail fast (naming the mode + + // env var); the dry-run path needs no prep hooks and must keep passing. live := map[string]string{ "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip "WORKCELL_STARTUP_CMD": "true", @@ -465,8 +433,6 @@ func TestDriverLiveRequiresPrepHooks(t *testing.T) { if !strings.Contains(out, "WORKCELL_STARTUP_COLD_PREP") || !strings.Contains(out, "cold") { t.Errorf("missing-prep error must name the mode and the env var: %s", out) } - - // Dry-run still works with no prep hooks set. code, out = runScript(t, driver, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50"}) if code != 0 { @@ -478,24 +444,15 @@ func TestDriverLiveRequiresPrepHooks(t *testing.T) { } func TestDriverSkipsWhenRuntimeClientButNoDaemon(t *testing.T) { - // A host with the runtime CLIENT installed but no usable daemon must fall - // through to the clean CI-safe skip (exit 0), not select live mode and then - // error on the missing command/prep hooks. Fake colima/container/docker - // clients that exist (so `command -v` finds them) but whose health probe - // fails (exit 1); shadow any real ones by putting the fakes first on PATH. + // A host with the runtime client but no usable daemon must cleanly skip (exit + // 0). Fake clients that exist but whose health probe fails, first on PATH. dir := t.TempDir() for _, name := range []string{"colima", "container", "docker"} { - // Every invocation (incl. the health probe) fails; existence still lets - // `command -v` succeed. - if err := os.WriteFile(filepath.Join(dir, name), - []byte("#!/usr/bin/env bash\nexit 1\n"), 0o755); err != nil { - t.Fatalf("write fake %s: %v", name, err) - } + writeExec(t, filepath.Join(dir, name), "#!/usr/bin/env bash\nexit 1\n") } env := map[string]string{ - // Fakes first, then the real system bins the driver needs (bash/date/...). + // Fakes first, then the real system bins the driver needs. "PATH": dir + string(os.PathListSeparator) + os.Getenv("PATH"), - // No WORKCELL_STARTUP_RUNTIME (force auto-detect), no SAMPLES_NS. } code, out := runScript(t, driver, env) if code != 0 { @@ -507,9 +464,7 @@ func TestDriverSkipsWhenRuntimeClientButNoDaemon(t *testing.T) { } func TestDriverLiveRequiresTwoRuns(t *testing.T) { - // A single-run live capture has no repeatability evidence; the docs say exit 0 - // means the stability gate passed, so a live run must require RUNS >= 2 and - // fail fast. The canned dry-run path may use whatever RUNS the data implies. + // A single-run live capture has no repeatability evidence, so RUNS >= 2 is required. live := map[string]string{ "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip "WORKCELL_STARTUP_CMD": "true", @@ -525,9 +480,7 @@ func TestDriverLiveRequiresTwoRuns(t *testing.T) { if !strings.Contains(out, "WORKCELL_STARTUP_RUNS") || !strings.Contains(out, ">= 2") { t.Errorf("error should require RUNS >= 2 for a live run: %s", out) } - - // Dry-run with a single canned group and RUNS=1 is a rehearsal, not gated or - // publishable, and must keep working. + // Dry-run with RUNS=1 is a rehearsal, not gated, and must keep working. code, out = runScript(t, driver, map[string]string{ "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30 40 50", "WORKCELL_STARTUP_RUNS": "1", @@ -538,9 +491,8 @@ func TestDriverLiveRequiresTwoRuns(t *testing.T) { } func TestDriverZeroMedianIsUnstable(t *testing.T) { - // A median of 0 in one run and nonzero in another is a degenerate/broken - // measurement (a 0 ns session start is impossible), not a 0%% spread that - // reads as STABLE. The gate must fail (exit 2). + // A median of 0 in one run and nonzero in another is degenerate (a 0 ns start + // is impossible), not a 0% spread that reads STABLE; the gate must fail. code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "0 0 0;10 20 30"}) if code != 2 { @@ -555,8 +507,7 @@ func TestDriverZeroMedianIsUnstable(t *testing.T) { } func TestDriverStabilityThresholdIsConfigurable(t *testing.T) { - // The same 20->21 spread (5%%) passes at the default threshold but a 1%% - // threshold rejects it, proving the gate reads the configured bound. + // The same 5% spread passes the default threshold but a 1% threshold rejects it. env := map[string]string{ "WORKCELL_STARTUP_SAMPLES_NS": "10 20 30;11 21 31", "WORKCELL_STARTUP_STABILITY_PCT": "1", diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index 2c3dacd5..a5702c8a 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -2,25 +2,18 @@ # # run-startup-bench.sh -- drive the session-start latency benchmark (C2). # -# For each mode (cold, cache-hit, warm) it runs a per-mode prep hook to -# establish the runtime state, then times WORKCELL_STARTUP_ITERATIONS full -# session starts of the target command via scripts/bench/startup-bench.sh, -# reporting the median, p90 and spread. The `cold` mode re-runs its prep hook -# before every measured sample and times each start on its own (warmup 0), so a -# session start warming the cache cannot turn later samples into cache-hits -- -# every cold sample reflects a genuine first start, not a warmed one. -# The whole measurement is repeated for WORKCELL_STARTUP_RUNS -# passes so a reviewer can confirm the numbers are stable across runs, and the -# driver FAILS if the run-to-run spread of any mode's median exceeds the -# stability threshold -- the C2 sibling of the C5 cross-run validation gate. +# For each mode (cold, cache-hit, warm) it runs a per-mode prep hook to establish +# the runtime state, then times WORKCELL_STARTUP_ITERATIONS full session starts of +# the target command via scripts/bench/startup-bench.sh. `cold` re-preps before +# every measured sample (warmup 0) so no sample is a cache-hit. The measurement is +# repeated for WORKCELL_STARTUP_RUNS passes and the driver FAILS if any mode's +# run-to-run median spread exceeds the stability threshold -- the C2 sibling of +# the C5 cross-run gate. # -# CI/offline safety: session starts need a live container runtime (Colima or an -# Apple `container` VM). When none is available the driver prints a clear -# message and exits 0 (skip, not fail), so it is safe to invoke as a dry run on -# any host. Supplying WORKCELL_STARTUP_SAMPLES_NS switches the driver into a -# canned dry-run that exercises the full report + stability gate without a -# runtime (used by the unit tests and for local rehearsal). -# See docs/session-startup-benchmarks.md. +# CI/offline safety: session starts need a live runtime; when none is available +# the driver exits 0 with a clear skip message. WORKCELL_STARTUP_SAMPLES_NS +# switches to a canned dry-run that exercises the report + gate with no runtime +# (used by the unit tests). See docs/session-startup-benchmarks.md. # # Configuration (all optional, via environment): # WORKCELL_STARTUP_ITERATIONS measured samples per mode (default 5) @@ -70,9 +63,8 @@ validate_int() { fi } -# Probe that an auto-detected runtime's daemon is actually usable (a cheap, -# read-only status call), not just that the client binary exists. Returns -# non-zero when the runtime is unusable so detection falls through to the skip. +# Probe that an auto-detected runtime's daemon is usable (cheap read-only status), +# not just that the client binary exists, so detection can fall through to skip. runtime_usable() { case "$1" in docker) docker info ;; @@ -104,10 +96,9 @@ if [ -n "${SAMPLES}" ]; then else detected="${WORKCELL_STARTUP_RUNTIME:-}" if [ -z "${detected}" ]; then - # Auto-detect only selects a runtime whose daemon is actually usable: a host - # can have the client binary installed with no working runtime, which would - # otherwise pick live mode and then hard-fail instead of skipping cleanly. An - # explicit WORKCELL_STARTUP_RUNTIME override skips this probe (respected as-is). + # Only select a runtime whose daemon is usable (an installed client with no + # working daemon would else pick live mode then hard-fail). An explicit + # WORKCELL_STARTUP_RUNTIME override skips this probe. for candidate in colima container docker; do command -v "${candidate}" >/dev/null 2>&1 || continue if runtime_usable "${candidate}"; then @@ -162,10 +153,9 @@ validate_int "WORKCELL_STARTUP_WARMUP" "${WARMUP}" 0 validate_int "WORKCELL_STARTUP_RUNS" "${RUNS}" 1 validate_int "WORKCELL_STARTUP_STABILITY_PCT" "${STABILITY_PCT}" 0 -# A gated, publishable live capture needs cross-run repeatability evidence, so -# require at least two runs: with one run the stability section is skipped and a -# 0 exit would misleadingly read as "gate passed". Dry-run canned mode may use -# whatever RUNS the canned data implies -- it is a rehearsal, never publishable. +# A publishable live capture needs cross-run evidence: with one run the stability +# section is skipped and a 0 exit would misleadingly read as "gate passed". Dry-run +# canned mode may use whatever RUNS the data implies (a rehearsal, not publishable). if [ "${DRY_RUN}" -eq 0 ] && [ "${RUNS}" -lt 2 ]; then echo "run-startup-bench: a live run requires WORKCELL_STARTUP_RUNS >= 2 for" \ "cross-run stability evidence (got ${RUNS}); a single run is not publishable." >&2 diff --git a/scripts/bench/startup-bench.sh b/scripts/bench/startup-bench.sh index 05f8880e..cf1a35c9 100755 --- a/scripts/bench/startup-bench.sh +++ b/scripts/bench/startup-bench.sh @@ -2,22 +2,16 @@ # # startup-bench.sh -- session-start latency microbenchmark harness (C2). # -# Times one "session start" latency sample -- the wall-clock cost of launching -# a workcell session -- for a single mode (cold / warm / cache-hit), repeated -# for a reproducible N-sample median + p90. It is the C2 sibling of the C5 -# exec-guard harness (scripts/bench/exec-guard-bench.c): a small, dependency- -# free measurement core whose numbers a reviewer can reproduce. The driver -# (scripts/bench/run-startup-bench.sh) orchestrates the cold vs warm passes and -# the cross-run stability gate. See docs/session-startup-benchmarks.md. -# -# A "session start" is expensive (image resolve + VM/runtime boot + supervisor -# handshake), so the sample unit is one full launch of the target command, not -# a tight micro-loop. The stats conventions match the C5 harness exactly: +# Times one "session start" latency sample -- the wall-clock cost of launching a +# workcell session -- for a single mode (cold / warm / cache-hit), repeated for a +# reproducible N-sample median + p90. A small, dependency-free measurement core; +# the driver (run-startup-bench.sh) orchestrates the passes and stability gate. +# See docs/session-startup-benchmarks.md. The sample unit is one full launch (not +# a micro-loop). Stats conventions match the C5 exec-guard harness exactly: # median = sorted[floor(n/2)] (0-based) # p90 = sorted[floor(n*9/10)] (0-based, clamped to n-1) # mean = sum/n # stddev = sqrt(sumsq/n - mean^2) (population) -# so cold/warm numbers are directly comparable to the exec-guard baselines. # # Usage: # startup-bench.sh [--] [target-cmd ...] @@ -26,11 +20,9 @@ # warmup discarded warmup launches run before measurement # target-cmd the session-start command to time (e.g. ./scripts/workcell ...) # -# Deterministic / dry-run path: if WORKCELL_STARTUP_SAMPLES_NS is set to a -# whitespace-separated list of positive integers, those are used as the measured -# samples verbatim and NO command is launched. This makes the stats core -# reproducible in CI and unit tests without a live runtime, and is how the -# driver feeds canned data for a dry run. +# Dry-run path: if WORKCELL_STARTUP_SAMPLES_NS is a whitespace-separated list of +# non-negative integers, those are the measured samples verbatim and NO command +# is launched -- how the driver/unit tests exercise the stats core with no runtime. # # Output (one line, key=value pairs, all times in nanoseconds): # mode= n= mean_ns= median_ns= p90_ns=

\ From f4874428c36b5609db6f4988c4d4328f698d54f4 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 13:09:21 -0400 Subject: [PATCH 8/9] ^B Register bench scripts in shell gates, stream prep output to stderr, use a monotonic clock (P2) (three integrity/hygiene fixes: add run-startup-bench.sh and startup-bench.sh to the shellcheck/shfmt allowlists in validate-repo.sh and dev-quick-check.sh so they can't escape the repo shell gate; redirect prep_mode hook output to stderr so a docker-pull etc. can't prepend non-report lines to the stdout Markdown report; and time live samples on CLOCK_MONOTONIC (perl Time::HiRes / python3 monotonic_ns, mirroring C5) so an NTP step or sleep/wake can't corrupt a sample; re-trimmed doc/test comments to stay under the 1200-line PR-shape limit) (go test/shellcheck/shfmt/markdownlint green; user-visible benchmark integrity + CI coverage) --- docs/session-startup-benchmarks.md | 36 +++---- internal/startupbench/bench_test.go | 146 ++++++++++++++++------------ scripts/bench/run-startup-bench.sh | 63 +++++------- scripts/bench/startup-bench.sh | 66 +++++-------- scripts/dev-quick-check.sh | 2 + scripts/validate-repo.sh | 2 + 6 files changed, 154 insertions(+), 161 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index f8f9bd9d..e1bada06 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -2,17 +2,12 @@ Workcell starts an isolated session by resolving the runtime image, booting the container runtime (Colima today, Apple `container` under evaluation in C1), and -completing the supervisor handshake before an agent can run. **C2** is the -program that measures that start latency, drives it down with cached images and -an optional kept-warm lane, and publishes reproducible numbers. This page -records the methodology and the rerun steps; it is the C2 sibling of -[syscall-shim-benchmarks.md](syscall-shim-benchmarks.md) (C5). - -The numbers are produced on a host with a live container runtime, not in the -PR-blocking CI lanes (a real session start needs a booted VM). The results tables -below are **placeholders pending a live capture** — see -[Filling in the numbers](#filling-in-the-numbers). Do not treat the template -values as measured. +completing the supervisor handshake. **C2** measures that start latency and drives +it down with cached images and an optional kept-warm lane — the sibling of +[syscall-shim-benchmarks.md](syscall-shim-benchmarks.md) (C5). Numbers are captured +on a host with a live runtime, not in PR CI (a real start needs a booted VM); the +results tables below are **placeholders pending a live capture** +([Filling in the numbers](#filling-in-the-numbers)) — not measured values. ## What is measured @@ -31,12 +26,12 @@ image-cache win from the kept-warm-lane win so each is credited independently. ## Methodology -The harness (`scripts/bench/startup-bench.sh`) times one mode: it runs the -session-start command `WORKCELL_STARTUP_WARMUP` times (discarded) to settle -first-touch page-cache and loader costs, then `WORKCELL_STARTUP_ITERATIONS` -measured times, and reports the sample distribution. The stats conventions match -the C5 exec-guard harness exactly, so the two pages' numbers are directly -comparable: +The harness (`scripts/bench/startup-bench.sh`) times one mode: `WORKCELL_STARTUP_ +WARMUP` discarded launches settle first-touch page-cache/loader costs, then +`WORKCELL_STARTUP_ITERATIONS` measured launches are timed on a **monotonic** clock +(`CLOCK_MONOTONIC`, so an NTP step or sleep/wake mid-launch can't corrupt a +sample). Stats conventions match the C5 exec-guard harness, so the pages compare +directly: - **median** (`sorted[floor(n/2)]`, the outlier-robust headline), **p90** (`sorted[floor(n*9/10)]` clamped, the tail a slow start shows), **mean/stddev** @@ -91,10 +86,9 @@ signals a broken clock rather than a 0% spread that would read as `STABLE`. ## Results -**Status: numbers pending live capture.** The tables below are templates — a -sustained upstream mirror outage blocking the image build left no live runtime to -capture real figures. Replace the `TODO` cells once a live run is captured (see -[Filling in the numbers](#filling-in-the-numbers)). Do not fabricate values. +**Status: numbers pending live capture.** Replace the `TODO` cells once a live run +is captured (see [Filling in the numbers](#filling-in-the-numbers)); do not +fabricate values. ### Start latency by mode (median of N samples, R runs) diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 5d423b27..78205838 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -7,6 +7,7 @@ package startupbench import ( + "bytes" "os" "os/exec" "path/filepath" @@ -24,11 +25,11 @@ func repoRoot(tb testing.TB) string { return filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) } -// runScript runs a bench script with args + extra env, returning exit code and -// combined output. The child env is built explicitly (not inherited) so a stray -// exported WORKCELL_STARTUP_* can't leak in: only PATH/HOME/TMPDIR + test-set keys -// are carried (a test key overrides the base). -func runScript(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string) { +// runScriptSplit runs a bench script with args + extra env, returning exit code, +// stdout and stderr separately. The child env is built explicitly (not inherited) +// so a stray exported WORKCELL_STARTUP_* can't leak in: only PATH/HOME/TMPDIR + +// test-set keys are carried (a test key overrides the base). +func runScriptSplit(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string, string) { tb.Helper() root := repoRoot(tb) cmd := exec.Command(filepath.Join(root, filepath.FromSlash(relScript)), args...) @@ -46,15 +47,25 @@ func runScript(tb testing.TB, relScript string, env map[string]string, args ...s for k, v := range base { cmd.Env = append(cmd.Env, k+"="+v) } - out, err := cmd.CombinedOutput() + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() if err == nil { - return 0, string(out) + return 0, stdout.String(), stderr.String() } if exitErr, ok := err.(*exec.ExitError); ok { - return exitErr.ExitCode(), string(out) + return exitErr.ExitCode(), stdout.String(), stderr.String() } - tb.Fatalf("run %s failed: %v\n%s", relScript, err, out) - return -1, "" + tb.Fatalf("run %s failed: %v\n%s%s", relScript, err, stdout.String(), stderr.String()) + return -1, "", "" +} + +// runScript is runScriptSplit with stdout+stderr combined (order not preserved). +func runScript(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string) { + tb.Helper() + code, stdout, stderr := runScriptSplit(tb, relScript, env, args...) + return code, stdout + stderr } // writeExec writes an executable helper script for a test. @@ -65,6 +76,25 @@ func writeExec(tb testing.TB, path, script string) { } } +// liveEnv returns a live-run env (auto-detect bypassed, all prep hooks no-op, +// RUNS>=2, gate widened so real timing can't flake) merged with extra, which +// overrides. Tests set only what they exercise. +func liveEnv(extra map[string]string) map[string]string { + env := map[string]string{ + "WORKCELL_STARTUP_RUNTIME": "colima", + "WORKCELL_STARTUP_CMD": "true", + "WORKCELL_STARTUP_RUNS": "2", + "WORKCELL_STARTUP_STABILITY_PCT": "100000000", + "WORKCELL_STARTUP_COLD_PREP": ":", + "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", + "WORKCELL_STARTUP_WARM_PREP": ":", + } + for k, v := range extra { + env[k] = v + } + return env +} + const harness = "scripts/bench/startup-bench.sh" const driver = "scripts/bench/run-startup-bench.sh" @@ -80,8 +110,7 @@ func parseFields(line string) map[string]string { } func TestHarnessStatsOddSampleSet(t *testing.T) { - // Deliberately unsorted; the harness sorts first. n=5 so median is the 3rd - // value and p90 (index floor(5*9/10)=4) is the max. + // Unsorted input; harness sorts. n=5 -> median=3rd value, p90 (idx 4)=max. code, out := runScript(t, harness, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "50 10 40 20 30"}, "cold", "0", "0") @@ -145,8 +174,7 @@ func TestHarnessRejectsNonIntegerSample(t *testing.T) { } func TestHarnessLivePathTimesTarget(t *testing.T) { - // No canned samples: times a benign target, exercising the real clock. Values - // are host-dependent, so we assert structure (n and the fields present). + // No canned samples: times a benign target on the real clock; assert structure. code, out := runScript(t, harness, nil, "warm", "3", "1", "--", "true") if code != 0 { t.Fatalf("exit %d, out=%q", code, out) @@ -191,8 +219,7 @@ func TestDriverDryRunStablePasses(t *testing.T) { } func TestDriverDryRunUnstableFailsGate(t *testing.T) { - // Two per-run groups with very different medians (20 vs 200) blow past the - // default 15% threshold, so the gate must fail. + // Two groups with very different medians (20 vs 200) exceed the 15% threshold. code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "10 20 30;100 200 300"}) if code != 2 { @@ -217,18 +244,12 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { "mode=\"$1\"; iters=\"$2\"; warmup=\"$3\"\n"+ "printf '%s %s\\n' \"$mode\" \"$warmup\" >> \"${HARNESS_LOG}\"\n"+ "printf 'mode=%s n=%s mean_ns=1 median_ns=1 p90_ns=1 stddev_ns=0 min_ns=1 max_ns=1\\n' \"$mode\" \"$iters\"\n") - env := map[string]string{ - "HARNESS_LOG": logPath, - "WORKCELL_STARTUP_HARNESS": stub, - "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip - "WORKCELL_STARTUP_CMD": "true", - "WORKCELL_STARTUP_ITERATIONS": "2", - "WORKCELL_STARTUP_WARMUP": "1", - "WORKCELL_STARTUP_RUNS": "2", // live requires >=2; stub output is constant so the gate stays STABLE - "WORKCELL_STARTUP_COLD_PREP": ":", - "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", - "WORKCELL_STARTUP_WARM_PREP": ":", - } + env := liveEnv(map[string]string{ + "HARNESS_LOG": logPath, + "WORKCELL_STARTUP_HARNESS": stub, + "WORKCELL_STARTUP_ITERATIONS": "2", + "WORKCELL_STARTUP_WARMUP": "1", + }) code, out := runScript(t, driver, env) if code != 0 { t.Fatalf("driver exit %d: %s", code, out) @@ -265,7 +286,7 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { func TestRunScriptEnvIsHermetic(t *testing.T) { // A stray exported WORKCELL_STARTUP_* must not leak in: a no-runtime run must - // still cleanly SKIP (not become a canned dry run) or run the leaked hook. + // still cleanly SKIP (not become a dry run) nor run the leaked hook. t.Setenv("WORKCELL_STARTUP_SAMPLES_NS", "999") t.Setenv("WORKCELL_STARTUP_COLD_PREP", "echo LEAKED_PREP_RAN") code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_RUNTIME": "none"}) @@ -281,25 +302,19 @@ func TestRunScriptEnvIsHermetic(t *testing.T) { } func TestDriverColdRepsPerMeasuredSample(t *testing.T) { - // cold re-runs COLD_PREP before every measured sample (a start warms the cache); - // warm/cache-hit share one prep. Hooks append a byte per call so we count them. + // cold re-runs COLD_PREP per sample (a start warms the cache); warm/cache-hit + // share one prep. Hooks append a byte per call so we count invocations. dir := t.TempDir() coldF := filepath.Join(dir, "cold") warmF := filepath.Join(dir, "warm") chF := filepath.Join(dir, "cachehit") - env := map[string]string{ - "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip - "WORKCELL_STARTUP_CMD": "true", - "WORKCELL_STARTUP_ITERATIONS": "3", - "WORKCELL_STARTUP_WARMUP": "0", - "WORKCELL_STARTUP_RUNS": "2", - // This test is about prep counts, not stability; widen the threshold so - // the (real) gate timing `true` never flakes. - "WORKCELL_STARTUP_STABILITY_PCT": "100000000", + env := liveEnv(map[string]string{ + "WORKCELL_STARTUP_ITERATIONS": "3", + "WORKCELL_STARTUP_WARMUP": "0", "WORKCELL_STARTUP_COLD_PREP": "printf c >> " + coldF, "WORKCELL_STARTUP_WARM_PREP": "printf w >> " + warmF, "WORKCELL_STARTUP_CACHE_HIT_PREP": "printf h >> " + chF, - } + }) code, out := runScript(t, driver, env) if code != 0 { t.Fatalf("driver exit %d: %s", code, out) @@ -328,8 +343,7 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { } func TestDriverDryRunSkipsPrep(t *testing.T) { - // A canned dry run must NEVER execute prep hooks, even with live hooks - // exported; the marker file must not exist afterward. + // A canned dry run must NEVER execute prep hooks; the marker must not exist. dir := t.TempDir() marker := filepath.Join(dir, "prep-ran") env := map[string]string{ @@ -351,9 +365,31 @@ func TestDriverDryRunSkipsPrep(t *testing.T) { } } +func TestDriverPrepOutputStaysOffReport(t *testing.T) { + // A prep hook's stdout (e.g. `docker pull` progress) must go to stderr, not the + // report on stdout, or `run.sh > report.md` yields unparseable Markdown. + env := liveEnv(map[string]string{ + "WORKCELL_STARTUP_COLD_PREP": "echo PREP_STDOUT_MARKER", + "WORKCELL_STARTUP_CACHE_HIT_PREP": "echo PREP_STDOUT_MARKER", + "WORKCELL_STARTUP_WARM_PREP": "echo PREP_STDOUT_MARKER", + }) + code, stdout, stderr := runScriptSplit(t, driver, env) + if code != 0 { + t.Fatalf("driver exit %d:\n%s%s", code, stdout, stderr) + } + if strings.Contains(stdout, "PREP_STDOUT_MARKER") { + t.Errorf("prep-hook stdout leaked into the report stream:\n%s", stdout) + } + if !strings.Contains(stdout, "# session-start latency benchmark results") { + t.Errorf("report missing from stdout:\n%s", stdout) + } + if !strings.Contains(stderr, "PREP_STDOUT_MARKER") { + t.Errorf("prep-hook output should appear on stderr:\n%s", stderr) + } +} + func TestDriverRejectsInvalidNumericControls(t *testing.T) { - // RUNS=0 / non-numeric controls could exit 0 with no benchmarking or a - // misleading STABLE, so each control is validated as an integer at its floor. + // Invalid numeric controls (RUNS=0, non-integer) must fail fast, not exit 0. cases := []struct{ name, key, val string }{ {"RUNS_zero", "WORKCELL_STARTUP_RUNS", "0"}, {"RUNS_nonnumeric", "WORKCELL_STARTUP_RUNS", "abc"}, @@ -388,19 +424,12 @@ func TestDriverPreservesCommandArgv(t *testing.T) { "set -euo pipefail\n"+ ": > \"${ARGV_FILE}\"\n"+ "for a in \"$@\"; do printf '%s\\n' \"$a\" >> \"${ARGV_FILE}\"; done\n") - env := map[string]string{ + env := liveEnv(map[string]string{ "ARGV_FILE": argvF, - "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip "WORKCELL_STARTUP_CMD": rec + " alpha 'beta gamma'", "WORKCELL_STARTUP_ITERATIONS": "1", "WORKCELL_STARTUP_WARMUP": "0", - "WORKCELL_STARTUP_RUNS": "2", - // About argv boundaries, not stability; widen the threshold. - "WORKCELL_STARTUP_STABILITY_PCT": "100000000", - "WORKCELL_STARTUP_COLD_PREP": ":", - "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", - "WORKCELL_STARTUP_WARM_PREP": ":", - } + }) code, out := runScript(t, driver, env) if code != 0 { t.Fatalf("driver exit %d: %s", code, out) @@ -465,14 +494,7 @@ func TestDriverSkipsWhenRuntimeClientButNoDaemon(t *testing.T) { func TestDriverLiveRequiresTwoRuns(t *testing.T) { // A single-run live capture has no repeatability evidence, so RUNS >= 2 is required. - live := map[string]string{ - "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip - "WORKCELL_STARTUP_CMD": "true", - "WORKCELL_STARTUP_RUNS": "1", - "WORKCELL_STARTUP_COLD_PREP": ":", - "WORKCELL_STARTUP_CACHE_HIT_PREP": ":", - "WORKCELL_STARTUP_WARM_PREP": ":", - } + live := liveEnv(map[string]string{"WORKCELL_STARTUP_RUNS": "1"}) code, out := runScript(t, driver, live) if code == 0 { t.Fatalf("live run with RUNS=1 should fail fast, got exit 0: %s", out) diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index a5702c8a..ec559c12 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -2,13 +2,11 @@ # # run-startup-bench.sh -- drive the session-start latency benchmark (C2). # -# For each mode (cold, cache-hit, warm) it runs a per-mode prep hook to establish -# the runtime state, then times WORKCELL_STARTUP_ITERATIONS full session starts of -# the target command via scripts/bench/startup-bench.sh. `cold` re-preps before -# every measured sample (warmup 0) so no sample is a cache-hit. The measurement is -# repeated for WORKCELL_STARTUP_RUNS passes and the driver FAILS if any mode's -# run-to-run median spread exceeds the stability threshold -- the C2 sibling of -# the C5 cross-run gate. +# For each mode (cold, cache-hit, warm) it runs a per-mode prep hook, then times +# WORKCELL_STARTUP_ITERATIONS session starts via scripts/bench/startup-bench.sh. +# `cold` re-preps before every measured sample (warmup 0) so no sample is a +# cache-hit. Repeated for WORKCELL_STARTUP_RUNS passes; the driver FAILS if any +# mode's run-to-run median spread exceeds the stability threshold (C5's sibling). # # CI/offline safety: session starts need a live runtime; when none is available # the driver exits 0 with a clear skip message. WORKCELL_STARTUP_SAMPLES_NS @@ -28,10 +26,9 @@ # WORKCELL_STARTUP_CACHE_HIT_PREP shell run before the cache-hit pass (prime cache, no warm lane) # WORKCELL_STARTUP_WARM_PREP shell run before the warm pass (prime cache + warm lane) # WORKCELL_STARTUP_RUNTIME override runtime detection (a name, or "none") -# WORKCELL_STARTUP_SAMPLES_NS canned samples -> dry-run, no runtime needed. -# A ';' splits per-run groups (each ';'-segment is -# one run's samples), which drives RUNS and lets a -# dry run rehearse an unstable cross-run spread. +# WORKCELL_STARTUP_SAMPLES_NS canned samples -> dry-run, no runtime needed; a +# ';' splits per-run groups (each is one run) and +# drives RUNS, so a dry run can rehearse instability # WORKCELL_STARTUP_OUTPUT also write the Markdown report to this file set -euo pipefail @@ -96,9 +93,8 @@ if [ -n "${SAMPLES}" ]; then else detected="${WORKCELL_STARTUP_RUNTIME:-}" if [ -z "${detected}" ]; then - # Only select a runtime whose daemon is usable (an installed client with no - # working daemon would else pick live mode then hard-fail). An explicit - # WORKCELL_STARTUP_RUNTIME override skips this probe. + # Only select a runtime whose daemon is usable (else an installed-but-dead + # client picks live mode then hard-fails). Explicit override skips the probe. for candidate in colima container docker; do command -v "${candidate}" >/dev/null 2>&1 || continue if runtime_usable "${candidate}"; then @@ -121,10 +117,8 @@ else "time) is required for a live run." >&2 exit 1 fi - # Live runs must establish each driven mode's documented runtime state. A - # missing prep hook would leave prep_mode a no-op, so the harness would measure - # whatever state happened to be present -- no real cold eviction or warm-lane - # priming -- yet still exit STABLE with publishable-looking numbers. Fail fast. + # Live runs must establish each mode's runtime state; a missing prep hook would + # leave prep_mode a no-op and measure arbitrary state as publishable. Fail fast. for mode in ${MODES}; do case "${mode}" in cold) prep_var="WORKCELL_STARTUP_COLD_PREP" ;; @@ -140,14 +134,12 @@ else done # Parse WORKCELL_STARTUP_CMD into an argv array honoring shell quoting, so an # argument with spaces (e.g. --workspace '/path/with space') keeps its boundary - # instead of being word-split or glob-expanded. This is the documented contract: - # quote such arguments in WORKCELL_STARTUP_CMD as you would on a shell line. + # instead of being word-split/globbed. Quote such args as on a shell line. eval "CMD_ARGV=( ${WORKCELL_STARTUP_CMD} )" fi -# Numeric controls must be sane before benchmarking: RUNS=0 or a non-integer -# would otherwise silently produce no measurements or a misleading STABLE. RUNS -# is validated after any dry-run per-group override above. +# Numeric controls must be sane before benchmarking (RUNS=0 or a non-integer would +# else produce no measurements or a misleading STABLE); RUNS after any override. validate_int "WORKCELL_STARTUP_ITERATIONS" "${ITERATIONS}" 1 validate_int "WORKCELL_STARTUP_WARMUP" "${WARMUP}" 0 validate_int "WORKCELL_STARTUP_RUNS" "${RUNS}" 1 @@ -171,18 +163,19 @@ field() { printf '%s\n' "$1" | sed -n "s/.*[[:space:]]$2=\([0-9]*\).*/\1/p" } -# Run the per-mode prep hook that establishes cold vs warm runtime state. +# Run the per-mode prep hook that establishes cold vs warm runtime state. Hook +# stdout (e.g. `docker pull` progress) goes to stderr so it cannot pollute the +# report on stdout; the exit status is preserved. prep_mode() { case "$1" in cold) eval "${WORKCELL_STARTUP_COLD_PREP:-:}" ;; cache-hit) eval "${WORKCELL_STARTUP_CACHE_HIT_PREP:-:}" ;; warm) eval "${WORKCELL_STARTUP_WARM_PREP:-:}" ;; - esac + esac >&2 } -# Invoke the harness for one warm/cache-hit mode: those modes legitimately share -# a single prep for the whole pass. In dry-run the canned samples for this run -# are passed straight through; live, the parsed session command is timed. +# Invoke the harness for one warm/cache-hit mode (they share one prep per pass): +# dry-run passes this run's canned samples; live times the parsed session command. run_harness() { # $1 mode, $2 run index (1-based) if [ "${DRY_RUN}" -eq 1 ]; then @@ -194,12 +187,9 @@ run_harness() { fi } -# Measure the cold mode with a genuine first start per sample. A single session -# start warms the cache the next start would otherwise hit, so evicting once -# before the pass would leave only the first sample cold. Re-run the cold-prep -# hook before EACH measured sample, time each start on its own (warmup 0), then -# aggregate the samples through the harness stats core so the reported cold row -# matches the other modes exactly. Emits the same key=value stats line. +# Measure cold with a genuine first start per sample: re-run the cold-prep hook +# before EACH measured sample (a start warms the cache the next would hit), time +# each on its own (warmup 0), then aggregate through the harness stats core. measure_cold() { # $1 run index (1-based) if [ "${DRY_RUN}" -eq 1 ]; then @@ -269,9 +259,8 @@ while [ "${run_index}" -le "${RUNS}" ]; do run_index=$((run_index + 1)) done -# Cross-run stability gate: for each mode, the spread of the median across all -# runs as a percentage of the smallest run's median. If any mode exceeds the -# threshold the run is not reproducible and the driver fails. +# Cross-run stability gate: per mode, the median spread across runs as a percent +# of the smallest run's median; if any mode exceeds the threshold the driver fails. GATE_STATUS="STABLE" if [ "${RUNS}" -ge 2 ]; then { diff --git a/scripts/bench/startup-bench.sh b/scripts/bench/startup-bench.sh index cf1a35c9..cc9c4df5 100755 --- a/scripts/bench/startup-bench.sh +++ b/scripts/bench/startup-bench.sh @@ -2,33 +2,22 @@ # # startup-bench.sh -- session-start latency microbenchmark harness (C2). # -# Times one "session start" latency sample -- the wall-clock cost of launching a -# workcell session -- for a single mode (cold / warm / cache-hit), repeated for a -# reproducible N-sample median + p90. A small, dependency-free measurement core; -# the driver (run-startup-bench.sh) orchestrates the passes and stability gate. -# See docs/session-startup-benchmarks.md. The sample unit is one full launch (not -# a micro-loop). Stats conventions match the C5 exec-guard harness exactly: +# Times session-start latency for one mode (cold / warm / cache-hit) over an +# N-sample median + p90. A small, dependency-free core; run-startup-bench.sh +# orchestrates the passes and stability gate. See docs/session-startup-benchmarks.md. +# Stats conventions match the C5 exec-guard harness exactly: # median = sorted[floor(n/2)] (0-based) # p90 = sorted[floor(n*9/10)] (0-based, clamped to n-1) -# mean = sum/n -# stddev = sqrt(sumsq/n - mean^2) (population) +# mean = sum/n ; stddev = sqrt(sumsq/n - mean^2) (population) # -# Usage: -# startup-bench.sh [--] [target-cmd ...] -# mode one of: cold warm cache-hit -# iterations measured samples (each = one full target launch) -# warmup discarded warmup launches run before measurement -# target-cmd the session-start command to time (e.g. ./scripts/workcell ...) +# Usage: startup-bench.sh [--] [target-cmd ...] +# mode cold|warm|cache-hit; iterations/warmup = measured/discarded launches. +# Dry-run: if WORKCELL_STARTUP_SAMPLES_NS is a whitespace-separated list of +# non-negative integers, those are the samples verbatim and NO command launches +# (how the driver/unit tests exercise the stats core with no runtime). # -# Dry-run path: if WORKCELL_STARTUP_SAMPLES_NS is a whitespace-separated list of -# non-negative integers, those are the measured samples verbatim and NO command -# is launched -- how the driver/unit tests exercise the stats core with no runtime. -# -# Output (one line, key=value pairs, all times in nanoseconds): -# mode= n= mean_ns= median_ns= p90_ns=

\ -# stddev_ns= min_ns= max_ns= -# -# A failed target launch (non-zero exit) aborts with a non-zero status and no +# Output (one line, ns): mode=<> n=<> mean_ns=<> median_ns=<> p90_ns=<> \ +# stddev_ns=<> min_ns=<> max_ns=<>. A failed launch aborts non-zero with no # numbers, so a broken session start can never masquerade as a measurement. set -euo pipefail @@ -105,29 +94,24 @@ fi [ "$#" -ge 1 ] || die "no target command given (and WORKCELL_STARTUP_SAMPLES_NS unset)" [ "${ITERATIONS}" -ge 1 ] || die "iterations must be >= 1 for a live measurement" -# Pick a monotonic-enough nanosecond clock. GNU date has %N; macOS date does -# not, so fall back to perl/python for sub-second resolution. +# Pick a MONOTONIC nanosecond clock (mirrors the C5 exec-guard CLOCK_MONOTONIC). +# Wall-clock (`date +%s%N`) is unusable: an NTP step or sleep/wake mid-launch +# could make end-start negative or absorb the step. perl/python3 expose +# CLOCK_MONOTONIC; plain `date` does not, so it is not a fallback. CLOCK="" -probe="$(date +%s%N 2>/dev/null || true)" -case "${probe}" in - '' | *[!0-9]*) ;; - *) CLOCK="date" ;; -esac -if [ -z "${CLOCK}" ]; then - if command -v perl >/dev/null 2>&1; then - CLOCK="perl" - elif command -v python3 >/dev/null 2>&1; then - CLOCK="python3" - else - die "no nanosecond clock available (need GNU date, perl, or python3)" - fi +if command -v perl >/dev/null 2>&1 && + perl -MTime::HiRes=clock_gettime,CLOCK_MONOTONIC -e1 >/dev/null 2>&1; then + CLOCK="perl" +elif command -v python3 >/dev/null 2>&1; then + CLOCK="python3" +else + die "no monotonic nanosecond clock (need perl Time::HiRes or python3)" fi now_ns() { case "${CLOCK}" in - date) date +%s%N ;; - perl) perl -MTime::HiRes=time -e 'printf "%.0f", Time::HiRes::time() * 1e9' ;; - python3) python3 -c 'import time; print(int(time.time() * 1e9))' ;; + perl) perl -MTime::HiRes=clock_gettime,CLOCK_MONOTONIC -e 'printf "%.0f", clock_gettime(CLOCK_MONOTONIC) * 1e9' ;; + python3) python3 -c 'import time; print(time.monotonic_ns())' ;; esac } diff --git a/scripts/dev-quick-check.sh b/scripts/dev-quick-check.sh index 0c51a1df..8243da42 100755 --- a/scripts/dev-quick-check.sh +++ b/scripts/dev-quick-check.sh @@ -26,6 +26,8 @@ require_tool rustfmt require_cargo_subcommand clippy shell_files=( + "${ROOT_DIR}/scripts/bench/run-startup-bench.sh" + "${ROOT_DIR}/scripts/bench/startup-bench.sh" "${ROOT_DIR}/scripts/bootstrap-dev.sh" "${ROOT_DIR}/scripts/check-dead-code.sh" "${ROOT_DIR}/scripts/check-public-repo-hygiene.sh" diff --git a/scripts/validate-repo.sh b/scripts/validate-repo.sh index b3f091eb..db4f4fbf 100755 --- a/scripts/validate-repo.sh +++ b/scripts/validate-repo.sh @@ -132,6 +132,8 @@ shell_files=( "${ROOT_DIR}/scripts/build-and-test.sh" "${ROOT_DIR}/scripts/ci-plan.sh" "${ROOT_DIR}/scripts/bench/run-exec-guard-bench.sh" + "${ROOT_DIR}/scripts/bench/run-startup-bench.sh" + "${ROOT_DIR}/scripts/bench/startup-bench.sh" "${ROOT_DIR}/scripts/workcell" "${ROOT_DIR}/scripts/check-workflows.sh" "${ROOT_DIR}/scripts/ci/build-validator-image.sh" From 81b6ec85124f444a35cea42955b283085aad3ac5 Mon Sep 17 00:00:00 2001 From: Omkhar Arasaratnam Date: Sun, 5 Jul 2026 13:37:35 -0400 Subject: [PATCH 9/9] ^B Time the measured loop in-process and re-prep cache-hit per sample (P2) (two measurement-validity fixes: move the per-sample timing loop into ONE long-lived perl/python process reading CLOCK_MONOTONIC in-process (with a launch()/system per sample), so a ~20ms interpreter startup no longer sits inside every measured interval and inflates fast warm/cache-hit medians - mirrors C5's in-process loop; and re-run the cache-hit prep hook before every measured sample like cold, since with warmup>=1 the discarded warmup was spending the cache-hit state before any measurement, drifting cache-hit toward warm; emitted line format unchanged so the Go stats tests + driver aggregation still pass; re-trimmed comments/prose to 1193 lines under the 1200 shape limit) (go test/shellcheck/shfmt/markdownlint green, format-preserving; user-visible benchmark measurement integrity) --- docs/session-startup-benchmarks.md | 65 ++++++++-------- internal/startupbench/bench_test.go | 111 ++++++++++++++++++---------- scripts/bench/run-startup-bench.sh | 85 ++++++++------------- scripts/bench/startup-bench.sh | 54 +++++++------- 4 files changed, 158 insertions(+), 157 deletions(-) diff --git a/docs/session-startup-benchmarks.md b/docs/session-startup-benchmarks.md index e1bada06..f11ab3f5 100644 --- a/docs/session-startup-benchmarks.md +++ b/docs/session-startup-benchmarks.md @@ -28,10 +28,11 @@ image-cache win from the kept-warm-lane win so each is credited independently. The harness (`scripts/bench/startup-bench.sh`) times one mode: `WORKCELL_STARTUP_ WARMUP` discarded launches settle first-touch page-cache/loader costs, then -`WORKCELL_STARTUP_ITERATIONS` measured launches are timed on a **monotonic** clock -(`CLOCK_MONOTONIC`, so an NTP step or sleep/wake mid-launch can't corrupt a -sample). Stats conventions match the C5 exec-guard harness, so the pages compare -directly: +`WORKCELL_STARTUP_ITERATIONS` measured launches run inside one long-lived timer +process (like C5's in-process loop) on a **monotonic** clock (`CLOCK_MONOTONIC`), +so neither an NTP step/sleep-wake nor a per-sample interpreter launch corrupts or +inflates a sample. Stats conventions match the C5 exec-guard harness, so the pages +compare directly: - **median** (`sorted[floor(n/2)]`, the outlier-robust headline), **p90** (`sorted[floor(n*9/10)]` clamped, the tail a slow start shows), **mean/stddev** @@ -57,11 +58,11 @@ Live runs are guarded so a misconfigured capture cannot look publishable: - **`WORKCELL_STARTUP_RUNS >= 2`** — the stability gate needs cross-run evidence, so a single-run capture is rejected. -For `cold` the driver re-runs `WORKCELL_STARTUP_COLD_PREP` before **every** -measured sample (warmup `0`) and aggregates the per-sample timings — a start warms -the cache the next start would hit, so evicting once per pass would leave only the -first sample truly cold, and the cold-prep hook must be **repeatable**. `warm` and -`cache-hit` share one prep per pass and keep `WORKCELL_STARTUP_WARMUP`. +For `cold` **and `cache-hit`** the driver re-runs the mode's prep hook before +**every** measured sample (warmup `0`) and aggregates the per-sample timings — a +start warms the cache/lane the next start would spend, so prepping once per pass +would leave only the first sample genuine; those hooks must be **repeatable**. +Only `warm` legitimately shares one prep per pass and keeps `WORKCELL_STARTUP_WARMUP`. `WORKCELL_STARTUP_CMD` is parsed with shell quoting, so a spaced argument keeps its boundary (`--workspace '/path/with space'` stays one argv element, not word-split @@ -79,10 +80,9 @@ signals a broken clock rather than a 0% spread that would read as `STABLE`. ### Runner caveats -- Numbers are **relative** to the measuring host's hardware and runtime backend; - treat the cold-vs-warm delta, not the absolute medians, as the portable signal. -- Session start includes VM boot, which is noisier than a userspace microbenchmark; - expect a wider stddev than the C5 exec-guard numbers. +Numbers are **relative** to the host's hardware and runtime backend, so treat the +cold-vs-warm delta (not absolute medians) as the portable signal; session start +includes VM boot, so expect a wider stddev than the C5 exec-guard numbers. ## Results @@ -108,23 +108,19 @@ fabricate values. ## Filling in the numbers -1. On a host with a live container runtime, wire the prep hooks and session - command, then run the driver (see [Rerunning](#rerunning)) with - `WORKCELL_STARTUP_OUTPUT` set to capture the report. -2. Confirm the run exits `0` (stability gate passed, numbers reproducible). A - non-zero exit means the spread was too wide to publish — investigate first. -3. Transcribe the report's per-mode medians, p90s and stability table into the - tables above, fill `vs cold` with the deltas, and record the host, runtime - backend, and `N`/`R`. +On a host with a live runtime, run the driver (see [Rerunning](#rerunning)) with +`WORKCELL_STARTUP_OUTPUT` set. A `0` exit means the stability gate passed and the +numbers are reproducible (non-zero means the spread was too wide — fix first). +Transcribe the report's medians, p90s and stability table into the tables above, +fill `vs cold`, and record the host, runtime backend, and `N`/`R`. ## Rerunning From the repository root on a host with a container runtime: ```sh -# Wire the prep hooks + session-start command to your runtime. WORKCELL_STARTUP_CMD -# is shell-quoted (quote args with spaces); COLD_PREP is re-run per sample (make -# it idempotent); a live run needs all three prep hooks and RUNS >= 2. +# A live run needs all three prep hooks and RUNS >= 2. WORKCELL_STARTUP_CMD is +# shell-quoted; COLD_PREP/CACHE_HIT_PREP re-run per sample, so make them idempotent. export WORKCELL_STARTUP_CMD='./scripts/workcell ' export WORKCELL_STARTUP_COLD_PREP='' export WORKCELL_STARTUP_CACHE_HIT_PREP='' @@ -136,19 +132,17 @@ export WORKCELL_STARTUP_OUTPUT=session-startup-results.md ``` Tunable via environment: `WORKCELL_STARTUP_ITERATIONS`, `WORKCELL_STARTUP_WARMUP` -(forced to `0` for `cold`), `WORKCELL_STARTUP_RUNS`, +(forced to `0` for `cold`/`cache-hit`), `WORKCELL_STARTUP_RUNS`, `WORKCELL_STARTUP_STABILITY_PCT`, `WORKCELL_STARTUP_CMD`, the three `*_PREP` hooks, -and `WORKCELL_STARTUP_OUTPUT`. The numeric controls are validated up front -(`ITERATIONS`/`RUNS` integers `>= 1`, `WARMUP`/`STABILITY_PCT` integers `>= 0`); -anything else fails fast rather than silently misreporting. +and `WORKCELL_STARTUP_OUTPUT`. Numeric controls are validated up front +(`ITERATIONS`/`RUNS` `>= 1`, `WARMUP`/`STABILITY_PCT` `>= 0`); anything else fails +fast rather than silently misreporting. ### Dry run without a runtime -The driver is CI-safe: with no runtime available it prints a clear message and -exits `0` (skip, not fail). To rehearse the full report and stability gate on any -host — no runtime needed — feed canned samples. The canned dry run needs no prep -hooks and never executes them, so any `WORKCELL_STARTUP_*_PREP` still exported -from a previous live run is ignored: +The driver is CI-safe: with no runtime it exits `0` with a clear skip message. To +rehearse the full report and stability gate on any host, feed canned samples — the +dry run runs no prep hooks (any exported `*_PREP` is ignored) and times nothing: ```sh # One stable run set (gate passes, exit 0): @@ -158,9 +152,8 @@ WORKCELL_STARTUP_SAMPLES_NS='10 20 30 40 50' ./scripts/bench/run-startup-bench.s WORKCELL_STARTUP_SAMPLES_NS='10 20 30;100 200 300' ./scripts/bench/run-startup-bench.sh ``` -This canned path is what the unit tests in `internal/startupbench` use to pin the -median/p90/stddev math, the stability gate, and the skip behavior without a -container build or a live VM. +The unit tests in `internal/startupbench` use this canned path (plus benign live +targets) to pin the stats math, the gate, and the guards without a container. ## Where this fits diff --git a/internal/startupbench/bench_test.go b/internal/startupbench/bench_test.go index 78205838..30cdb083 100644 --- a/internal/startupbench/bench_test.go +++ b/internal/startupbench/bench_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strconv" "strings" "testing" ) @@ -25,28 +26,33 @@ func repoRoot(tb testing.TB) string { return filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..")) } -// runScriptSplit runs a bench script with args + extra env, returning exit code, -// stdout and stderr separately. The child env is built explicitly (not inherited) -// so a stray exported WORKCELL_STARTUP_* can't leak in: only PATH/HOME/TMPDIR + -// test-set keys are carried (a test key overrides the base). -func runScriptSplit(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string, string) { - tb.Helper() - root := repoRoot(tb) - cmd := exec.Command(filepath.Join(root, filepath.FromSlash(relScript)), args...) - cmd.Dir = root +// hermeticEnv builds an explicit child env (not inherited) so a stray exported +// WORKCELL_STARTUP_* can't leak in: only PATH/HOME/TMPDIR + extra (which overrides). +func hermeticEnv(extra map[string]string) []string { base := map[string]string{"PATH": os.Getenv("PATH")} for _, k := range []string{"HOME", "TMPDIR"} { if v, ok := os.LookupEnv(k); ok { base[k] = v } } - for k, v := range env { + for k, v := range extra { base[k] = v } - cmd.Env = make([]string, 0, len(base)) + env := make([]string, 0, len(base)) for k, v := range base { - cmd.Env = append(cmd.Env, k+"="+v) + env = append(env, k+"="+v) } + return env +} + +// runScriptSplit runs a bench script with args + extra env, returning exit code, +// stdout and stderr separately, with a hermetic child environment. +func runScriptSplit(tb testing.TB, relScript string, env map[string]string, args ...string) (int, string, string) { + tb.Helper() + root := repoRoot(tb) + cmd := exec.Command(filepath.Join(root, filepath.FromSlash(relScript)), args...) + cmd.Dir = root + cmd.Env = hermeticEnv(env) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -190,6 +196,38 @@ func TestHarnessLivePathTimesTarget(t *testing.T) { } } +func TestHarnessTimesLaunchesInOneProcess(t *testing.T) { + // The measured loop must run in ONE long-lived timer process. Each launch + // records its parent PID: all share one PPID (the timer) != the harness's. + dir := t.TempDir() + ppidF := filepath.Join(dir, "ppids") + rec := filepath.Join(dir, "ppid.sh") + writeExec(t, rec, "#!/usr/bin/env bash\necho \"$PPID\" >> \""+ppidF+"\"\n") + root := repoRoot(t) + cmd := exec.Command(filepath.Join(root, filepath.FromSlash(harness)), "warm", "3", "0", "--", rec) + cmd.Dir = root + cmd.Env = hermeticEnv(nil) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("harness failed: %v\n%s", err, out) + } + data, err := os.ReadFile(ppidF) + if err != nil { + t.Fatalf("read ppids: %v", err) + } + ppids := strings.Fields(string(data)) + if len(ppids) != 3 { + t.Fatalf("want 3 launches, got %d: %q", len(ppids), data) + } + if harnessPid := strconv.Itoa(cmd.Process.Pid); ppids[0] == harnessPid { + t.Errorf("target parent == harness pid %s: launches ran in the harness shell, not a dedicated in-process timer", harnessPid) + } + for _, p := range ppids[1:] { + if p != ppids[0] { + t.Errorf("launches had different parents %v; want a single timer process", ppids) + } + } +} + func TestDriverSkipsWithoutRuntime(t *testing.T) { code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_RUNTIME": "none"}) if code != 0 { @@ -234,8 +272,7 @@ func TestDriverDryRunUnstableFailsGate(t *testing.T) { } func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { - // cold forces warmup=0; the driver drives all three modes incl. cache-hit. A - // stub harness (WORKCELL_STARTUP_HARNESS seam) records the mode + warmup it got. + // cold/cache-hit force warmup=0 (re-prep per sample), only warm keeps it; a stub harness logs each mode's warmup. dir := t.TempDir() logPath := filepath.Join(dir, "harness.log") stub := filepath.Join(dir, "stub-harness.sh") @@ -274,19 +311,18 @@ func TestDriverColdSkipsWarmupAndDrivesCacheHit(t *testing.T) { } } if got := warmupByMode["cold"]; got != "0" { - t.Errorf("cold warmup = %q, want 0 (P1: cold must not warm before measuring)", got) + t.Errorf("cold warmup = %q, want 0 (must not warm before measuring)", got) + } + if got := warmupByMode["cache-hit"]; got != "0" { + t.Errorf("cache-hit warmup = %q, want 0 (re-preps per sample like cold)", got) } if got := warmupByMode["warm"]; got != "1" { t.Errorf("warm warmup = %q, want 1 (configured warmup preserved)", got) } - if got := warmupByMode["cache-hit"]; got != "1" { - t.Errorf("cache-hit warmup = %q, want 1 (configured warmup preserved)", got) - } } func TestRunScriptEnvIsHermetic(t *testing.T) { - // A stray exported WORKCELL_STARTUP_* must not leak in: a no-runtime run must - // still cleanly SKIP (not become a dry run) nor run the leaked hook. + // A stray exported WORKCELL_STARTUP_* must not leak in: a no-runtime run must still SKIP (not a dry run) nor run the hook. t.Setenv("WORKCELL_STARTUP_SAMPLES_NS", "999") t.Setenv("WORKCELL_STARTUP_COLD_PREP", "echo LEAKED_PREP_RAN") code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_RUNTIME": "none"}) @@ -301,9 +337,8 @@ func TestRunScriptEnvIsHermetic(t *testing.T) { } } -func TestDriverColdRepsPerMeasuredSample(t *testing.T) { - // cold re-runs COLD_PREP per sample (a start warms the cache); warm/cache-hit - // share one prep. Hooks append a byte per call so we count invocations. +func TestDriverColdAndCacheHitRepPerSample(t *testing.T) { + // cold/cache-hit re-run their prep hook per sample, warm once per pass; hooks append a byte per call. dir := t.TempDir() coldF := filepath.Join(dir, "cold") warmF := filepath.Join(dir, "warm") @@ -326,19 +361,18 @@ func TestDriverColdRepsPerMeasuredSample(t *testing.T) { } return len(data) } - // 3 cold samples x 2 runs -> 6 cold preps; warm/cache-hit share one prep per - // pass -> 2 each. + // 3 samples x 2 runs -> 6 preps each for cold/cache-hit; warm -> 2 (one/pass). if got := countPreps(coldF); got != 6 { - t.Errorf("cold prep ran %d time(s), want 6 (once per measured sample x 2 runs)", got) + t.Errorf("cold prep ran %d time(s), want 6 (per measured sample x 2 runs)", got) + } + if got := countPreps(chF); got != 6 { + t.Errorf("cache-hit prep ran %d time(s), want 6 (per measured sample x 2 runs)", got) } if got := countPreps(warmF); got != 2 { t.Errorf("warm prep ran %d time(s), want 2 (one prep per pass x 2 runs)", got) } - if got := countPreps(chF); got != 2 { - t.Errorf("cache-hit prep ran %d time(s), want 2 (one prep per pass x 2 runs)", got) - } - if !strings.Contains(out, "| cold |") || !strings.Contains(out, " 3 |") { - t.Errorf("aggregated cold row missing/incorrect: %s", out) + if !strings.Contains(out, "| cache-hit |") || !strings.Contains(out, " 3 |") { + t.Errorf("aggregated cache-hit row missing/incorrect: %s", out) } } @@ -366,8 +400,7 @@ func TestDriverDryRunSkipsPrep(t *testing.T) { } func TestDriverPrepOutputStaysOffReport(t *testing.T) { - // A prep hook's stdout (e.g. `docker pull` progress) must go to stderr, not the - // report on stdout, or `run.sh > report.md` yields unparseable Markdown. + // A prep hook's stdout (e.g. `docker pull`) must go to stderr, not the stdout report (else `run.sh > report.md` breaks). env := liveEnv(map[string]string{ "WORKCELL_STARTUP_COLD_PREP": "echo PREP_STDOUT_MARKER", "WORKCELL_STARTUP_CACHE_HIT_PREP": "echo PREP_STDOUT_MARKER", @@ -415,8 +448,7 @@ func TestDriverRejectsInvalidNumericControls(t *testing.T) { } func TestDriverPreservesCommandArgv(t *testing.T) { - // WORKCELL_STARTUP_CMD is shell-quoted: a spaced arg (--workspace '/a b') must - // reach the target as one argv element. The recorder leaves the last launch. + // Shell-quoted CMD: a spaced arg (--workspace '/a b') must reach as one element. dir := t.TempDir() argvF := filepath.Join(dir, "argv") rec := filepath.Join(dir, "record.sh") @@ -446,8 +478,7 @@ func TestDriverPreservesCommandArgv(t *testing.T) { } func TestDriverLiveRequiresPrepHooks(t *testing.T) { - // On a LIVE run a missing mode prep hook must fail fast (naming the mode + - // env var); the dry-run path needs no prep hooks and must keep passing. + // On a LIVE run a missing mode prep hook must fail fast (naming mode + env var); dry-run needs none. live := map[string]string{ "WORKCELL_STARTUP_RUNTIME": "colima", // bypass the no-runtime skip "WORKCELL_STARTUP_CMD": "true", @@ -473,8 +504,7 @@ func TestDriverLiveRequiresPrepHooks(t *testing.T) { } func TestDriverSkipsWhenRuntimeClientButNoDaemon(t *testing.T) { - // A host with the runtime client but no usable daemon must cleanly skip (exit - // 0). Fake clients that exist but whose health probe fails, first on PATH. + // Runtime client present but daemon unusable must cleanly skip. Fake clients whose health probe fails, first on PATH. dir := t.TempDir() for _, name := range []string{"colima", "container", "docker"} { writeExec(t, filepath.Join(dir, name), "#!/usr/bin/env bash\nexit 1\n") @@ -513,8 +543,7 @@ func TestDriverLiveRequiresTwoRuns(t *testing.T) { } func TestDriverZeroMedianIsUnstable(t *testing.T) { - // A median of 0 in one run and nonzero in another is degenerate (a 0 ns start - // is impossible), not a 0% spread that reads STABLE; the gate must fail. + // A 0 median in one run vs nonzero in another is degenerate (impossible), not a 0% spread; the gate must fail. code, out := runScript(t, driver, map[string]string{"WORKCELL_STARTUP_SAMPLES_NS": "0 0 0;10 20 30"}) if code != 2 { diff --git a/scripts/bench/run-startup-bench.sh b/scripts/bench/run-startup-bench.sh index ec559c12..1cdc3908 100755 --- a/scripts/bench/run-startup-bench.sh +++ b/scripts/bench/run-startup-bench.sh @@ -4,32 +4,15 @@ # # For each mode (cold, cache-hit, warm) it runs a per-mode prep hook, then times # WORKCELL_STARTUP_ITERATIONS session starts via scripts/bench/startup-bench.sh. -# `cold` re-preps before every measured sample (warmup 0) so no sample is a -# cache-hit. Repeated for WORKCELL_STARTUP_RUNS passes; the driver FAILS if any -# mode's run-to-run median spread exceeds the stability threshold (C5's sibling). +# `cold` and `cache-hit` re-prep before every measured sample (warmup 0) so a +# discarded warmup can't spend that state; only `warm` shares one prep per pass. +# Repeated for WORKCELL_STARTUP_RUNS passes; the driver FAILS if any mode's +# run-to-run median spread exceeds the stability threshold (C5's sibling). # -# CI/offline safety: session starts need a live runtime; when none is available -# the driver exits 0 with a clear skip message. WORKCELL_STARTUP_SAMPLES_NS -# switches to a canned dry-run that exercises the report + gate with no runtime -# (used by the unit tests). See docs/session-startup-benchmarks.md. -# -# Configuration (all optional, via environment): -# WORKCELL_STARTUP_ITERATIONS measured samples per mode (default 5) -# WORKCELL_STARTUP_WARMUP discarded warmup samples (default 1; forced to 0 for cold) -# WORKCELL_STARTUP_RUNS full measurement passes (default 2) -# WORKCELL_STARTUP_STABILITY_PCT max allowed cross-run median spread (default 15) -# WORKCELL_STARTUP_CMD session-start command to time (required live); -# parsed with shell quoting, so quote args with -# spaces (e.g. --workspace '/path/with space') -# WORKCELL_STARTUP_COLD_PREP shell re-run before EACH cold sample (evict cache -# + stop warm lane); must be repeatable -# WORKCELL_STARTUP_CACHE_HIT_PREP shell run before the cache-hit pass (prime cache, no warm lane) -# WORKCELL_STARTUP_WARM_PREP shell run before the warm pass (prime cache + warm lane) -# WORKCELL_STARTUP_RUNTIME override runtime detection (a name, or "none") -# WORKCELL_STARTUP_SAMPLES_NS canned samples -> dry-run, no runtime needed; a -# ';' splits per-run groups (each is one run) and -# drives RUNS, so a dry run can rehearse instability -# WORKCELL_STARTUP_OUTPUT also write the Markdown report to this file +# With no live runtime the driver exits 0 with a clear skip message; +# WORKCELL_STARTUP_SAMPLES_NS switches to a canned dry-run (no runtime) used by the +# unit tests. All configuration env vars and the full methodology are documented +# in docs/session-startup-benchmarks.md. set -euo pipefail ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/.." @@ -132,22 +115,17 @@ else exit 1 fi done - # Parse WORKCELL_STARTUP_CMD into an argv array honoring shell quoting, so an - # argument with spaces (e.g. --workspace '/path/with space') keeps its boundary - # instead of being word-split/globbed. Quote such args as on a shell line. + # Parse CMD into argv honoring quoting (--workspace '/a b' stays one word). eval "CMD_ARGV=( ${WORKCELL_STARTUP_CMD} )" fi -# Numeric controls must be sane before benchmarking (RUNS=0 or a non-integer would -# else produce no measurements or a misleading STABLE); RUNS after any override. +# Validate numeric controls (RUNS=0/non-integer would else misreport); RUNS last. validate_int "WORKCELL_STARTUP_ITERATIONS" "${ITERATIONS}" 1 validate_int "WORKCELL_STARTUP_WARMUP" "${WARMUP}" 0 validate_int "WORKCELL_STARTUP_RUNS" "${RUNS}" 1 validate_int "WORKCELL_STARTUP_STABILITY_PCT" "${STABILITY_PCT}" 0 -# A publishable live capture needs cross-run evidence: with one run the stability -# section is skipped and a 0 exit would misleadingly read as "gate passed". Dry-run -# canned mode may use whatever RUNS the data implies (a rehearsal, not publishable). +# A publishable live capture needs >=2 runs: one run skips the gate yet exits 0. if [ "${DRY_RUN}" -eq 0 ] && [ "${RUNS}" -lt 2 ]; then echo "run-startup-bench: a live run requires WORKCELL_STARTUP_RUNS >= 2 for" \ "cross-run stability evidence (got ${RUNS}); a single run is not publishable." >&2 @@ -163,9 +141,8 @@ field() { printf '%s\n' "$1" | sed -n "s/.*[[:space:]]$2=\([0-9]*\).*/\1/p" } -# Run the per-mode prep hook that establishes cold vs warm runtime state. Hook -# stdout (e.g. `docker pull` progress) goes to stderr so it cannot pollute the -# report on stdout; the exit status is preserved. +# Run the per-mode prep hook; its stdout goes to stderr so it can't pollute the +# report (exit status preserved). prep_mode() { case "$1" in cold) eval "${WORKCELL_STARTUP_COLD_PREP:-:}" ;; @@ -174,8 +151,8 @@ prep_mode() { esac >&2 } -# Invoke the harness for one warm/cache-hit mode (they share one prep per pass): -# dry-run passes this run's canned samples; live times the parsed session command. +# Invoke the harness for warm (one prep/pass): dry-run passes canned samples, +# live times the parsed session command. run_harness() { # $1 mode, $2 run index (1-based) if [ "${DRY_RUN}" -eq 1 ]; then @@ -187,26 +164,26 @@ run_harness() { fi } -# Measure cold with a genuine first start per sample: re-run the cold-prep hook -# before EACH measured sample (a start warms the cache the next would hit), time -# each on its own (warmup 0), then aggregate through the harness stats core. -measure_cold() { - # $1 run index (1-based) +# Measure cold/cache-hit with a genuine first start per sample: re-run the mode's +# prep hook before EACH sample (warmup 0), then aggregate via the harness stats. +measure_reprep() { + # $1 mode, $2 run index (1-based) + local mode="$1" if [ "${DRY_RUN}" -eq 1 ]; then # Dry-run: canned samples, no prep, no launch -- emit the group's stats. - local gi=$(($1 - 1)) + local gi=$(($2 - 1)) [ "${gi}" -lt "${#SAMPLE_GROUPS[@]}" ] || gi=$((${#SAMPLE_GROUPS[@]} - 1)) - WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" cold "${ITERATIONS}" 0 + WORKCELL_STARTUP_SAMPLES_NS="${SAMPLE_GROUPS[gi]}" "${HARNESS}" "${mode}" "${ITERATIONS}" 0 return fi local samples=() one i=0 while [ "${i}" -lt "${ITERATIONS}" ]; do - prep_mode cold - one="$("${HARNESS}" cold 1 0 -- "${CMD_ARGV[@]}")" + prep_mode "${mode}" + one="$("${HARNESS}" "${mode}" 1 0 -- "${CMD_ARGV[@]}")" samples+=("$(field "${one}" min_ns)") i=$((i + 1)) done - WORKCELL_STARTUP_SAMPLES_NS="${samples[*]}" "${HARNESS}" cold "${#samples[@]}" 0 + WORKCELL_STARTUP_SAMPLES_NS="${samples[*]}" "${HARNESS}" "${mode}" "${#samples[@]}" 0 } { @@ -216,7 +193,7 @@ measure_cold() { echo "- host: $(uname -srm)" echo "- online CPUs: $(getconf _NPROCESSORS_ONLN 2>/dev/null || echo unknown)" echo "- runtime: ${RUNTIME}" - echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}; cold re-preps + warmup 0 per sample) x ${RUNS} run(s)" + echo "- iterations: ${ITERATIONS} (warmup ${WARMUP}; cold/cache-hit re-prep + warmup 0 per sample) x ${RUNS} run(s)" echo "- stability threshold: ${STABILITY_PCT}% cross-run median spread" echo } >"${REPORT}" @@ -234,13 +211,13 @@ while [ "${run_index}" -le "${RUNS}" ]; do } >>"${REPORT}" for mode in ${MODES}; do - if [ "${mode}" = "cold" ]; then - line="$(measure_cold "${run_index}")" - else - # Dry-run uses canned samples, so it must not execute any prep hook (an - # operator may have live hooks exported from a previous run). + # cold and cache-hit re-prep per measured sample; only warm shares one prep + # for the whole pass (and dry-run runs no prep -- canned samples never launch). + if [ "${mode}" = "warm" ]; then [ "${DRY_RUN}" -eq 1 ] || prep_mode "${mode}" line="$(run_harness "${mode}" "${run_index}")" + else + line="$(measure_reprep "${mode}" "${run_index}")" fi med="$(field "${line}" median_ns)" p90="$(field "${line}" p90_ns)" diff --git a/scripts/bench/startup-bench.sh b/scripts/bench/startup-bench.sh index cc9c4df5..32dc791c 100755 --- a/scripts/bench/startup-bench.sh +++ b/scripts/bench/startup-bench.sh @@ -108,31 +108,33 @@ else die "no monotonic nanosecond clock (need perl Time::HiRes or python3)" fi -now_ns() { - case "${CLOCK}" in - perl) perl -MTime::HiRes=clock_gettime,CLOCK_MONOTONIC -e 'printf "%.0f", clock_gettime(CLOCK_MONOTONIC) * 1e9' ;; - python3) python3 -c 'import time; print(time.monotonic_ns())' ;; - esac -} - -launch() { - "$@" >/dev/null 2>&1 || die "target launch failed (exit $?): $*" -} +# Time the WHOLE loop inside ONE long-lived process (like C5's in-process loop) so +# no per-sample interpreter launch sits in a measured interval -- only the target's +# fork/exec. Args (warmup iterations -- target...); target is exec'd (no shell) so +# quoting survives, output discarded, non-zero launch aborts. +# shellcheck disable=SC2016 # perl/python program text: $vars are NOT shell expansions +timing_perl='use strict; use warnings; use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC); +my $warm = shift @ARGV; my $iter = shift @ARGV; +sub launch { my $p = fork; die "fork failed\n" unless defined $p; + if (!$p) { open STDOUT, ">", "/dev/null"; open STDERR, ">", "/dev/null"; exec { $ARGV[0] } @ARGV; exit 127 } + waitpid $p, 0; die "target launch failed\n" if $? } +launch() for (1 .. $warm); +for (1 .. $iter) { my $t = clock_gettime(CLOCK_MONOTONIC); launch(); printf "%.0f\n", (clock_gettime(CLOCK_MONOTONIC) - $t) * 1e9 }' +# shellcheck disable=SC2016 # python program text: $-free, quotes intentional +timing_python='import sys, time, subprocess +warm = int(sys.argv[1]); iters = int(sys.argv[2]); argv = sys.argv[3:] +def launch(): + if subprocess.call(argv, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) != 0: + sys.exit("target launch failed") +for _ in range(warm): + launch() +for _ in range(iters): + t = time.monotonic_ns(); launch(); print(time.monotonic_ns() - t)' -warm_index=0 -while [ "${warm_index}" -lt "${WARMUP}" ]; do - launch "$@" - warm_index=$((warm_index + 1)) -done - -samples=() -sample_index=0 -while [ "${sample_index}" -lt "${ITERATIONS}" ]; do - start="$(now_ns)" - launch "$@" - end="$(now_ns)" - samples+=("$((end - start))") - sample_index=$((sample_index + 1)) -done +case "${CLOCK}" in + perl) raw="$(perl -e "${timing_perl}" -- "${WARMUP}" "${ITERATIONS}" "$@")" || die "target launch failed during timing" ;; + python3) raw="$(python3 -c "${timing_python}" "${WARMUP}" "${ITERATIONS}" "$@")" || die "target launch failed during timing" ;; +esac -emit "${samples[@]}" +[ -n "${raw}" ] || die "no samples produced" +printf 'mode=%s %s\n' "${MODE}" "$(printf '%s\n' "${raw}" | stats)"