Skip to content

feat: add SIRUN_READY_FD ready signal to exclude startup time#80

Merged
rochdev merged 15 commits into
mainfrom
rochdev/ready-signal
Jun 4, 2026
Merged

feat: add SIRUN_READY_FD ready signal to exclude startup time#80
rochdev merged 15 commits into
mainfrom
rochdev/ready-signal

Conversation

@rochdev

@rochdev rochdev commented Jun 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Add SIRUN_READY_FD env var: Sirun always provides a writable pipe fd to every benchmarked process
  • Write any byte to it when startup is complete; Sirun resets wall.time, user.time, system.time, cpu.pct.wall.time, and (Linux) instructions to measure only the post-ready period
  • CPU times are read from the live process at signal time (/proc/{pid}/stat on Linux, proc_pidinfo on macOS) and subtracted from the final RUSAGE_CHILDREN delta
  • max.res.size always covers the full process lifetime (kernel-tracked peak, cannot be sampled mid-run)
  • If the process exits without writing to the fd, Sirun falls back silently to full-lifetime timing

Usage

// Node.js — after all imports and startup work
require('fs').writeSync(parseInt(process.env.SIRUN_READY_FD), 'x');
# Python
import os
os.write(int(os.environ['SIRUN_READY_FD']), b'x')
# Shell
echo x >&$SIRUN_READY_FD

Test Plan

  • cargo test passes (all tests including ready_signal_resets_wall_time and ready_signal_cpu_pct_bounded)
  • Verify wall.time is near-zero after 500ms startup sleep + ready signal (examples/ready-signal.json)
  • Verify cpu.pct.wall.time <= 100% after CPU-intensive startup + ready signal (examples/ready-signal-cpu.json)
  • Verify existing benchmarks without SIRUN_READY_FD usage are unaffected

🤖 Generated with Claude Code

@rochdev
rochdev marked this pull request as ready for review June 3, 2026 21:13
Comment thread .gitignore
The pipe already signals child exit via EOF when the write end closes,
so racing the pipe against child.status() is unnecessary. Sequential
await is simpler and removes the futures dependency entirely.
bengl
bengl previously approved these changes Jun 4, 2026
@datadog-prod-us1-3

This comment has been minimized.

bengl
bengl previously approved these changes Jun 4, 2026
…ed CI flakiness on macOS)

async_std::fs::File on macOS uses kqueue-based I/O: when the pipe event
fires, the async task is rescheduled through the executor before resuming.
That extra scheduling hop means proc_pidinfo is called slightly later than
with spawn_blocking, which uses a dedicated OS thread that calls proc_pidinfo
synchronously right after the read returns with no scheduler hop in between.
On a loaded CI runner that window is large enough for the process state to
change, causing proc_pidinfo to return None and the CPU subtraction to be
skipped.
@rochdev
rochdev force-pushed the rochdev/ready-signal branch from 8e6173e to afee830 Compare June 4, 2026 17:40
…SAGE_CHILDREN

The bash fixture used $(seq 1 300000) which forks a subprocess — seq's
CPU accumulates in RUSAGE_CHILDREN transitively but is invisible to
proc_pidinfo which only measures bash's own task. The mismatch caused
cpu.pct.wall.time > 100% on ~74% of macOS CI runs.

Python keeps all CPU in a single process with no fork/exec after signaling,
so proc_pidinfo and RUSAGE_CHILDREN always agree.
@rochdev
rochdev marked this pull request as draft June 4, 2026 20:00
…ne_byte

CI failures were caused by the bash fixture's subprocess CPU leaking into
RUSAGE_CHILDREN, not by the read mechanism. Both approaches pass the 100x
test with the Python3 fixture.
@rochdev
rochdev marked this pull request as ready for review June 4, 2026 20:18
@rochdev
rochdev merged commit ef03e02 into main Jun 4, 2026
4 checks passed
@rochdev
rochdev deleted the rochdev/ready-signal branch June 4, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants