Skip to content

Decrease run-to-run variance of the bare metal runner#923

Draft
epompeii wants to merge 10 commits into
develfrom
claude/runner-variance-optimization-293nsj
Draft

Decrease run-to-run variance of the bare metal runner#923
epompeii wants to merge 10 commits into
develfrom
claude/runner-variance-optimization-293nsj

Conversation

@epompeii

@epompeii epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member

What

Reduces run-to-run variance of the bare metal runner on both the Firecracker sandbox path and the no-sandbox path. All new measures are runtime sysfs/procfs/cgroup configuration shipped inside the single static runner binary (no host boot args required), on by default with individual disable flags, best-effort with graceful skips on ARM and locked-down hosts, and restored on shutdown.

One commit per phase:

1. Host tuning extensions, runner run fix, preflight warnings

  • New tuning knobs (each with a keep-enabled CLI flag): NUMA balancing, timer migration, soft watchdog, and KSM disabled via sysctls; deep C-states held off via a /dev/cpu_dma_latency PM QoS fd held open for the runner lifetime (crash-safe, works on x86 and ARM)
  • Device IRQs, the IRQ default affinity, and the unbound workqueue cpumask steered to housekeeping cores (--no-irq-steering to disable), applied via a new layout-scoped tuning phase after CpuLayout::detect()
  • Bug fix: one-shot runner run applied tuning but never detected the CPU layout, so cpuset pinning only happened under runner up; it now mirrors the up path
  • Static preflight warnings at startup: irqbalance running, no cgroup v2, virtualized host, missing isolcpus=/nohz_full= boot args

2. No-sandbox path isolation and metrics

  • The local execution path previously ran the benchmark with no CPU isolation at all; it now gets a per-run cgroup cpuset on the benchmark cores, with race-free attachment via Command::pre_exec (the child moves itself into the pre-opened cgroup.procs fd between fork and exec) and a sched_setaffinity fallback for unprivileged hosts
  • The local path now emits RunMetrics (wall clock plus cgroup CPU/memory, transport: "local"); it previously emitted none

3. Scheduler-domain isolation and vCPU determinism (Firecracker)

  • /sys/fs/cgroup/bencher becomes an isolated cpuset partition, the runtime equivalent of isolcpus=: benchmark cores leave the root scheduling domain so nothing else can be balanced onto them. Kernel acceptance is verified by read-back, with a fallback chain isolated -> root -> member (--no-cpuset-partition to disable)
  • Each Firecracker fc_vcpu N thread is pinned to its own dedicated benchmark core after InstanceStart (required companion to the isolated partition, which does no load balancing); VMM/API threads pin to the last benchmark core
  • Swap is disabled for the VM cgroup in the live path, and RunMetrics.cgroup is now filled from the existing cgroup metrics reader (previously always None)

4. Guest determinism and host THP

  • Guest kernel cmdline gains norandmaps nokaslr transparent_hugepage=never (guest ASLR/KASLR off, deterministic guest memory backing), and is now arch-conditional: aarch64 uses keep_bootcon and reboot=k instead of the x86-only reboot=t
  • Host transparent hugepages default to never (enabled and defrag), with --thp <never|madvise|always|leave>; leave preserves the old do-not-touch behavior

Note

Release notes for operators:

  • All new measures are on by default. Existing runner up deployments will start steering IRQs, disabling NUMA balancing/watchdogs/KSM, holding C-states off, creating an isolated cpuset partition, and pinning host THP to never on upgrade. Each has an individual disable flag, and --no-tuning disables everything.
  • The host THP default change may cause a one-time baseline step for memory-heavy benchmarks. --thp leave opts out.
  • Tuning restores on clean shutdown only. After a SIGKILL or panic-abort, sysctls/IRQ affinities/THP/partition stay applied and the pre-tuning values cannot be recovered by the next run; a reboot or manual reset recovers them. The C-state hold and per-run cgroups self-heal.

Testing

  • Unit tests for every new helper (tempfile-based sysfs/procfs trees), 193 tests passing in bencher_runner + bencher_runner_cli
  • cargo clippy -Dwarnings natively and cross-compiled to x86_64-unknown-linux-gnu (./scripts/clippy.sh), cargo check --no-default-features, cargo test --doc, ./scripts/test.sh --linux-only
  • Operator-side validation still to come on Linux/KVM hardware: cargo test-runner, plus before/after bencher noise and repeated runner run --iter N on x86_64 and aarch64, with and without --sandbox firecracker

epompeii added 4 commits July 7, 2026 03:58
Add new host tuning measures to reduce benchmark run-to-run variance:
- Disable NUMA balancing, timer migration, soft watchdog, and KSM
- Hold CPUs out of deep C-states via /dev/cpu_dma_latency (PM QoS)
- Steer device IRQs and unbound workqueues to housekeeping cores

Fix a gap where one-shot runner run applied tuning but never detected
the CPU layout, so cpuset pinning only happened under runner up.

Add static preflight warnings at startup for conditions the runner
cannot fix at runtime: irqbalance running, no cgroup v2, virtualized
host, and missing isolcpus/nohz_full boot args.

All new measures are on by default with individual CLI disable flags,
skip gracefully when the underlying files are missing, and restore
their original values on shutdown.
The local (no sandbox) execution path previously ran the benchmark with
no CPU isolation at all: the child shared cores with the runner's own
threads. Bring it in line with the Firecracker path:

- Create a per-run cgroup with a cpuset restricted to the benchmark
  cores, reusing CgroupManager
- Attach the child race-free via Command::pre_exec: the child writes
  itself into the pre-opened cgroup.procs fd between fork and exec, so
  it never runs startup work on the wrong cores
- Fall back to sched_setaffinity when cgroup setup fails (non-root,
  no cgroup v2); isolation is best-effort and never blocks the run
- Emit RunMetrics (wall clock plus cgroup CPU and memory usage) from
  the local path with transport "local"; it previously emitted none
Detach the benchmark cores from the root scheduling domain by turning
the parent /sys/fs/cgroup/bencher cgroup into an isolated cpuset
partition, the runtime equivalent of the isolcpus= boot arg. The kernel
load balancer can no longer pull other tasks onto benchmark cores.
Validity is verified by read-back (the kernel reports rejected
partitions inline), with a fallback chain isolated -> root -> member.
Per-run cgroups inherit the partition automatically. Disable with
--no-cpuset-partition.

Pin each Firecracker vCPU thread (fc_vcpu N) to its own dedicated
benchmark core after InstanceStart, with VMM/API threads pinned to the
last benchmark core. This is the required companion to the isolated
partition, which does no load balancing between its cores, and also
removes vCPU migration noise (cold caches, TLB refills) without it.

Also disable swap for the VM cgroup in the live Firecracker path and
fill RunMetrics.cgroup from the existing cgroup metrics reader, which
previously always emitted None.
Extend the default guest kernel cmdline with determinism params, all
stock options on the bundled Firecracker CI 6.1.174 kernel:
- norandmaps: guest ASLR off (overridable via kernel_cmdline, giving
  opt-out parity with the host --aslr flag)
- nokaslr: every run boots a fresh VM, so guest KASLR is a per-run
  variance source unique to the sandbox path
- transparent_hugepage=never: deterministic guest memory backing

Make the default cmdline arch-conditional: reboot=t is an x86 triple
fault; aarch64 now uses reboot=k plus keep_bootcon instead of the
previous x86-flavored cmdline.

Pin the host transparent hugepage mode to 'never' by default (enabled
and defrag), with a --thp <never|madvise|always|leave> override;
'leave' preserves the previous behavior of not touching THP. Note for
release: memory-heavy benchmarks may see a one-time baseline step from
the THP change.
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

PR: #923
Base: devel
Head: claude/runner-variance-optimization-293nsj
Commit: 98696e5bb6d71d1a42342219c38d819efaa6d948


I have a thorough understanding of the changes. Here is my review.

PR Review: Host tuning, CPU isolation, and cpuset partitioning for the runner

Overall this is a high-quality, well-structured change. The code is idiomatic, extensively unit-tested (with fake /proc and /sys trees via tempdirs), thoroughly documented, and consistently best-effort with graceful degradation. It complies well with the project standards in CLAUDE.md (strong types, thiserror, camino, clap parser split, #[expect] with reasons, no select!, no shared mutable state). No blocking bugs found.

Strengths

  • Excellent RAII discipline. TuningGuard restores in reverse order, and the drop ordering is correct: the manual Drop::drop restores saved while held_fds are still open, then fields drop (closing the /dev/cpu_dma_latency fd last). The partition restore correctly saves cpus → mems → mode so teardown demotes the partition before shrinking the cpuset.
  • Race-free child attachment in local_isolation.rs: moving the benchmark into the cgroup and setting affinity in pre_exec (between fork and exec) avoids the startup-on-wrong-core window. The SAFETY comment correctly justifies async-signal-safety and no-allocation.
  • Robust kernel-format handling. format_cpumask, parse_cpu_id_list, bracketed sysfs parsing, and partition_mode_token (normalizing isolated invalid (...) read-backs) are all carefully implemented and tested.
  • Building the layout from online CPU IDs rather than a count correctly handles SMT-disabled interleaved topologies (AMD) where the online set is non-contiguous.
  • Partition verification via read-back is the right call: the kernel reports rejected cpuset partitions inline rather than via write errno, and verify_partition_state gates on it with an isolated → root → member fallback.

Observations (non-blocking)

  1. Global-state / single-runner assumption (tuning/mod.rs:20-23). The sysctls, IRQ affinities, THP mode, and the bencher cpuset partition are host-global and keyed to a fixed cgroup path, so two concurrent runners on one host will restore state out from under each other. This is documented, but it is an operational footgun worth enforcing (e.g. a host-level lock/PID file) rather than only documenting, since a second runner silently corrupts the first's isolation.

  2. Crash-restore gap (tuning/mod.rs:9-18). On SIGKILL/panic-abort, tuned values persist and a later run reads them as "current", so the true pre-tuning value is lost until reboot. Clearly documented; the self-healing fd and per-run cgroups mitigate the worst cases. Acceptable, but flagging for visibility.

  3. Per-run pinning latency (firecracker/pin.rs:19). pin_vcpu_threads blocks up to DISCOVERY_TIMEOUT (500 ms) polling /proc/<pid>/task after InstanceStart. It breaks early once all vCPUs appear, so the common case is fast, and the cgroup cpuset is the hard confinement regardless. Fine, just note the added tail latency.

  4. Belt-and-suspenders in configure_command. When both the cgroup move and the affinity fallback are available, the child does both (local_isolation.rs:138-151). Harmless (the affinity set equals the cgroup cpuset), and it makes the affinity path a genuine fallback when the cgroup write fails. No change needed.

  5. enable_controllers now always runs even when the parent cgroup pre-exists (cgroup.rs:41-45). This is intentional (the partition may create bencher without controllers) and cannot introduce new failures, since the verification gate passes whenever the required controllers are already enabled. Good.

Minor nits

  • cpu.rs:44-49: an empty /sys/devices/system/cpu/online would make parse_cpu_id_list return None (empty string parses to None) and fall back to available_cores() — correct, though the !ids.is_empty() guard is then redundant with the parse behavior. No action needed.
  • The --no-tuning flag deliberately does not disable core pinning (run.rs comment), only the tuning knobs. This is a reasonable design choice and is documented, but worth confirming it matches operator expectations, since --no-tuning still creates cgroups and pins the benchmark.

Testing note

I could not run cargo test/cargo clippy/the Linux cross-compile checks in this environment (approval gate). Per CLAUDE.md, please confirm CI runs ./scripts/clippy.sh and ./scripts/test.sh --linux-only for the touched target_os = "linux" crates, since much of this code is Linux-gated and the non-Linux stubs carry unused_mut/unused_variables expects that only the cross-compile catches. The new nosandbox_metrics scenario in test_runner is a good end-to-end guard for the local-transport metrics change.

Net: solid, careful work. I'd approve after confirming the CI Linux checks pass, and I'd suggest tracking the concurrent-runner mutual-clobbering case as a follow-up hardening item rather than a docs-only note.


Model: claude-opus-4-8

Save the cpuset partition restore entry before attempting any mode
write. A write can succeed at the syscall level while the kernel
rejects the partition in the read-back; previously the fallback path
left the partition file dirty on the persistent bencher cgroup, and a
later apply could even save the dirty state as its restore value. The
saved value is now normalized to the mode token, since a rejected
partition reads back as '<mode> invalid (<reason>)', which is not a
valid value to write back.

Make enable_controllers verification-gated: write failures are now
tolerated when the required controllers are already enabled, e.g.
pre-configured by an admin for an unprivileged runner. This was a
behavior regression from making the call unconditional. Also fix the
controller check to match whole tokens; 'cpuset' alone previously
satisfied the 'cpu' controller via substring matching.
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchclaude/runner-variance-optimization-293nsj
Testbedintel-v1
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
microseconds (µs)
(Result Δ%)
Upper Boundary
microseconds (µs)
(Limit %)
Adapter::Json📈 view plot
🚷 view threshold
4.60 µs
(-0.54%)Baseline: 4.62 µs
4.90 µs
(93.78%)
Adapter::Magic (JSON)📈 view plot
🚷 view threshold
4.47 µs
(-0.70%)Baseline: 4.50 µs
4.72 µs
(94.59%)
Adapter::Magic (Rust)📈 view plot
🚷 view threshold
25.87 µs
(+1.31%)Baseline: 25.54 µs
26.64 µs
(97.13%)
Adapter::Rust📈 view plot
🚷 view threshold
3.51 µs
(+0.69%)Baseline: 3.49 µs
3.60 µs
(97.48%)
Adapter::RustBench📈 view plot
🚷 view threshold
3.51 µs
(+0.66%)Baseline: 3.48 µs
3.59 µs
(97.58%)
🐰 View full continuous benchmarking report in Bencher

Review follow-ups from PR #923, documentation only:
- tuning module docs now state that restoration happens on clean
  shutdown only, and that a crash makes the pre-tuning values
  unrecoverable for the next run (the C-state fd hold and per-run
  cgroups self-heal by construction)
- note that /dev/cpu_dma_latency only supports the literal zero value
  here; a non-zero latency would need i32::to_ne_bytes
- note that vCPU threads run briefly before pinning and the cgroup
  cpuset is the hard confinement, with pinning as a refinement
@epompeii

epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Addressed the review asks in be106d0 and 3d12eea:

  • Fixed the partition restore gap: the restore entry is now saved before any mode write, normalized to the mode token, with a regression test for the dirty-file case (be106d0)
  • Made enable_controllers verification-gated so pre-configured unprivileged runners keep working, and fixed the controller check to match whole tokens (be106d0)
  • Documented the crash-restore limitation in the tuning module docs, the zero-only /dev/cpu_dma_latency encoding, and the cpuset-vs-pinning guarantee (3d12eea)
  • Expanded the PR description release notes to cover the on-by-default behavior change for existing runners

Confirming the compliance asks: cargo fmt, cargo clippy -Dwarnings (native and cross-compiled via ./scripts/clippy.sh), cargo nextest run (193 passing), cargo test --doc, cargo check --no-default-features, and ./scripts/test.sh --linux-only are all green locally on every commit.

Not changed: guest ASLR stays cmdline-only (norandmaps via the user-overridable kernel_cmdline was a deliberate design decision), and --thp keeps FromStr parsing to match the existing SandboxLogLevel convention.

Comment-only follow-ups from the PR #923 review:
- document the CliTuning flag polarity rationale (keep-enabled flags
  for hardware features, --no- flags for runner-side actions)
- note that at full vCPU width the highest-index vCPU shares its core
  with the VMM threads in assign_core
- note that CPU core pinning in runner run is deliberately independent
  of --no-tuning, matching runner up
- note why the local spawn-failure path emits no run metrics
@epompeii

epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Round 3 review nits addressed in 3f4020e (comment-only):

  • CliTuning now documents the flag polarity rationale: keep-enabled flags name hardware features (--smt, --turbo, --cstates), --no- flags name skipped runner actions (--no-irq-steering, --no-cpuset-partition), since a bare --irq-steering would read as enabling something already on by default
  • assign_core documents that at full vCPU width the highest-index vCPU shares its core with VMM threads
  • runner run documents that core pinning is deliberately independent of --no-tuning, matching the long-standing runner up behavior (the tuning config gates the tuning knobs, including partition and steering); confirming this is intended
  • the local spawn-failure path documents why it emits no metrics (nothing ran)

On the workqueue scope of --no-irq-steering: the flag's own help text already reads "Do not steer device IRQs and kernel workqueues to housekeeping cores", so the scope is documented at the point of use; keeping the shorter flag name.

Linux cross-checks (./scripts/clippy.sh, ./scripts/test.sh --linux-only) plus nextest, doc tests, and no-default-features check remain green on this commit; both the unqualified size_of in local_isolation.rs and the unconditional CpuLayout import compile on both targets.

Extend cargo test-runner with a non-sandboxed scenario asserting the
local path emits the BENCHER_METRICS stderr marker with transport
'local' (it previously emitted no metrics at all).

Note in the tuning module docs that tuning assumes a single runner
process per host: the sysctls, IRQ affinities, THP mode, and cpuset
partition are host-global, so a second concurrent runner's shutdown
restores them out from under the first.
@epompeii

epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Round 4 review points, addressed in e07f278:

  • Point 1 (concurrent runners): added the suggested doc note; tuning assumes a single runner process per host since the mutated state is host-global
  • Point 2 (--no-tuning scope): confirmed intended. --no-tuning disables host-global mutations; per-run core pinning and the transient per-run cgroup are core runner behavior, matching what runner up has always done. Documented in run.rs
  • Point 4 (transport "local"): confirmed safe. The only consumer that inspects the transport value is the sandboxed-only metrics_transport_type scenario, and RunMetrics deserializes transport as a plain string. Also added a new nosandbox_metrics scenario to cargo test-runner that pins down the local-path marker and transport value
  • Points 5 and 6: leaving as is. The 'pinned 0 of N' summary is deliberate signal that pinning failed, and FromStr parsing for --thp matches the existing SandboxLogLevel convention

Cross-compile checks, nextest, doc tests, and no-default-features remain green on this commit.

The local isolation module documents that it mirrors the Firecracker
path, but only applied the cpuset. Also disable swap on the per-run
cgroup so benchmark memory stays resident on the no-sandbox path;
swap thrashing adds run-to-run variance.
@epompeii

epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Round 5 observations, addressed in 8c233dd:

  • Observation 2 (swap asymmetry): fixed. LocalIsolation now also disables swap on the per-run cgroup, so the local path fully mirrors the Firecracker path
  • Observation 1 (enable_controllers on pre-existing parents): confirmed intended. The verification read is the gate, so unprivileged runners with admin-pre-enabled controllers still succeed; a parent that exists but lacks required controllers would previously have failed later at the first cgroup write anyway, just with a worse error
  • Observations 3 and 4: known, documented constraints (last-core sharing at full vCPU width; single-runner-per-host assumption)

Cross-compile checks (./scripts/clippy.sh, ./scripts/test.sh --linux-only), nextest, doc tests, and no-default-features are green on this commit.

CpuLayout::detect() previously counted online CPUs and assumed the IDs
were contiguous 0..N. Disabling SMT (on by default) on topologies with
interleaved sibling numbering, common on AMD, leaves a non-contiguous
online set like 0,2,4,6; the count-based layout would then pin cpusets
and affinities to offline cores. Parse the actual ID list from
/sys/devices/system/cpu/online instead, keeping the count-based path
as a fallback. with_core_count() now delegates to with_cpu_ids().

Also normalize the remaining emdashes in the tuning output strings and
the relocated host-tuning comments to hyphens, per the no-emdash policy
in CLAUDE.md; new code already used hyphens, so output was mixed.
@epompeii

epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Round 6 findings, addressed in 98696e5:

  • Finding 1 (emdash in the relocated comment): fixed, along with the same pre-existing comment in up/mod.rs
  • Finding 3 (mixed separators in tuning output): normalized all output strings in the tuning module to hyphens per the CLAUDE.md no-emdash policy
  • Finding 2 (contiguous core ID assumption): fixed now rather than deferred, since this PR's SMT-off default is what creates the non-contiguous condition. CpuLayout::detect() parses the actual ID list from /sys/devices/system/cpu/online (with the count-based path kept as fallback), so interleaved-sibling topologies pin to real online cores. New with_cpu_ids constructor with tests for the interleaved case
  • Runner docs note on --no-tuning pinning: leaving for the docs site separately; the PR description and code comments cover it

@epompeii

epompeii commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Round 7 review: all observations are known, documented constraints or no-action items. Two follow-ups:

  • The concurrent-runner mutual-clobbering hardening is now tracked as runner: enforce single-runner-per-host for host-global tuning #924 (advisory flock before tuning applies), per the suggestion to make it a follow-up item rather than a docs-only note
  • Confirming the CI question: the Test / Cargo Test job runs the workspace test suite and the Runner jobs build both Linux targets; the cross-compile lint scripts (./scripts/clippy.sh, ./scripts/test.sh --linux-only) are additionally run locally on every commit of this branch, and the non-Linux expect attributes compile clean on both targets

CI is green on this branch apart from the two pre-existing online lint failures (Lint / Zizmor Online, Lint / Cargo Deny Advisories), which also fail on devel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant