test(fspy): benchmark tracking overhead - #566
Merged
Merged
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
wan9chi
marked this pull request as ready for review
July 24, 2026 02:38
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8541d088c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fspy benchmarklinuxmacoswindows |
wan9chi
force-pushed
the
agent/benchmark-fspy-overhead
branch
15 times, most recently
from
July 28, 2026 02:25
f2782c1 to
04dba7f
Compare
Add a single multithreaded open-and-close workload, a Linux static variant, and three-platform Namespace reporting with PR/main baselines. Co-authored-by: GPT-5.6 Codex <codex@openai.com>
Make the reporter shell explicit and document why its text-only workflow_run boundary is intentionally safe. Co-authored-by: GPT-5.6 Codex <codex@openai.com>
Allow upload-artifact to include the dot-prefixed benchmark results directory. Co-authored-by: GPT-5.6 Codex <codex@openai.com>
Use a shorter process-level sampling window and inherit child stderr so runtime failures remain diagnosable. Co-authored-by: GPT-5.6 Codex <codex@openai.com>
Use pull-request write permission for the sticky benchmark report endpoint. Co-authored-by: GPT-5.6 Codex <codex@openai.com>
Preferring an earlier run of the same PR as the baseline masked cumulative regressions, so the baseline is now always the latest main artifact. Gate the main baseline artifact name on refs/heads/main so a workflow_dispatch run from another branch cannot pollute it. Remove the fork report workflow: workflow_run events carry an empty pull_requests array for PRs from forks, so its trigger condition could never match. Fork PRs keep job summaries and artifacts. Fold the PR comment assembly and upsert into fspy-benchmark.mjs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Node runs .mts files directly by stripping types, so the script stays build-free while gaining typed structures for the result JSON, Criterion estimates, and GitHub API payloads. The new tsconfig gives the scripts directory Node types and enforces erasable-only syntax. Comment the non-obvious constraints: baseline selection, re-run exclusion, ratio normalization, and the workload constants mirrored from the Criterion bench. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Linux and Windows runner images preinstall Node 20, which cannot run .mts files; type stripping needs the pinned 22.19. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With a single pass over the fixture, the tracked/untracked delta on the preload paths (2-8 ms) is the same order as spawn-time variance on CI runners: overhead swung 11 points between identical runs, and on Windows the confidence interval exceeded the delta. Repeat the open loop 16 times so per-open interception cost dominates, extend the measurement window to five seconds, and render each mean's confidence interval in the report. Bump the result schema and skip comparisons across schema versions, since ratios from different workloads are not comparable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
Co-authored-by: GPT-5 Codex <codex@openai.com>
The reported number was the difference between a tracked and an untracked launch, which is not reproducible: identically configured runners disagree on absolute launch times by tens of percent, so the same commit measured 23.3 ms and 26.8 ms for the same workload on Linux, and on macOS the difference kept landing below zero and was clamped to 1 ns. A baseline saved on main cannot tell a regression from that. Measure what fspy actually costs a process instead, as two separate things, each in the way that reproduces best. Starting a process under tracking stays a wall-clock measurement, because that is the quantity, reported as a share of an untracked launch measured back to back. Normalizing that way cancels most of the difference between runner instances, and reporting a share rather than a difference keeps a given slowdown moving the number by the same amount on every platform. Interception is now timed from inside the target, which reports how long its batches of opens took. A launch is one number covering milliseconds, so anything the runner does during it lands in the result; batches are a few microseconds long and there are hundreds per launch, so the median discards the ones the scheduler spoiled. That took the spread on macOS from 4% to 0.7%, and it made the benchmark faster, because a handful of launches now carries more information than hundreds did. Runs agree within about 1% to 4%, measured by benchmarking the same commit on six runner instances per platform. Two rows are left out where they do not reproduce: concurrent accesses on macOS, where a second busy thread makes every open cost about three times as much by an amount that changes from launch to launch, and the tail of seccomp accesses, which is dominated by how quickly the runner wakes the supervisor. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Every attempt to compare a pull request run with a baseline recorded by an earlier run on main fought the same fact: two runs land on two runner instances, and identically configured instances disagree on absolute times by tens of percent, and still by up to ten percent after normalizing each measurement against an untracked launch on the same machine. A threshold above that noise is a threshold above any regression worth catching. Stop comparing across runs. The launcher — the only benchmark piece that links fspy — is now built twice per pull request job, against the fspy under review and against the merge-base fspy, and the harness interleaves launches of both builds back to back on the same machine, with an untracked arm in the same rotation pricing tracking itself as context. Each iteration yields the head/base ratio per metric, the reported change is the median of those ratios, and a row that moves past its threshold fails the job. Criterion is gone with the cross-run baselines: medians of interleaved ratio pairs never fit its model of timing a closure, and everything it still provided — warmup, sample counts, change detection — is a handful of lines without it. So are the saved baseline artifacts, the main-push runs that recorded them, and the download action that fetched them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rotating one fixed cycle keeps every arm's neighbors the same forever — the baseline always ran right behind the untracked launch — and the leftovers a launch inherits from its neighbor do not cancel out, which biased the A/A change on the launch row by almost a percent. Cycle all orderings instead, so position and neighbors both even out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing outside the fspy and benchmark crates, the locked dependencies, and the toolchain can change what fspy costs, so skip the benchmark for every other pull request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The benchmark harness has a x86_64-unknown-linux-musl artifact dependency among its regular dependencies, so documenting the workspace now builds the static benchmark target and needs the musl standard library, like the clippy and test jobs already do. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four rows per backend made the report harder to read than the two questions it answers: what starting a tracked process costs, and what an access costs while tracking. Keep one row for each. The access row keeps the two-thread workload, because a process that fspy tracks rarely accesses files from one thread, and its 8% threshold, set from the spread that concurrent interception shows between runner instances. The single-threaded row and the tail percentile were finer instruments for telling contention and scatter apart from cost, at the price of two more rows to explain; a red access row now just leaves that bisection to the investigation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collapse the workload/row/suite triple into one suite struct now that every suite has exactly one row, and read the suite's metric straight out of the launcher's line instead of storing both numbers and selecting later. Round iterations up to whole order cycles instead of hand-picking multiples. Make the launcher the one owner of the missing path, passed to the target as an argument, so the two crates cannot drift apart on it. Require the target's arguments instead of silently substituting defaults nothing uses, derive per-thread suffixes instead of wrapping an eight-entry table, and share the barrier by scoped borrow instead of an Arc. Declare the tokio macros feature on fspy itself, which uses select!, instead of relying on whichever dependent enables it; the launcher then declares only what it uses. Drop the workspace-level cargo-shear ignore that the package-level list already covers, and document why the static target is a separate package over the same source. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Share the head build's target directory with the baseline launcher build and set the binary aside, instead of compiling the whole dependency graph a second time into a separate directory that the cache then carries. Save the cache only on main, where every other run can restore it, and add a main push trigger so something actually seeds it and the overhead history stays recorded. Skip uploading reports nothing consumes: fork pull requests get no comment, and the comment job reads the artifacts minutes after they upload, not in ninety days. Documenting the workspace no longer installs a musl standard library on every run just for the benchmark's static target; the doc build skips the benchmark crate instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The benchmark finishes in a third of the CI critical path once the cache is warm, so path-filtering it saves nothing anyone waits for, and an unfiltered workflow can become a required check one day, which a path-filtered one cannot. On pull requests that leave fspy alone the change column compares two identical builds and reads zero; that is the price of not maintaining a list of every path that can move fspy's codegen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The launcher dropped the tokio macros feature as unused, which it is — by the launcher. fspy uses select! without declaring the feature, and older revisions of it only compile when a dependent enables it, which the launcher does on every benchmark run when it is built against the merge-base fspy. Linux hid the breakage because fspy pulls the seccomp crate there, which declares the feature itself; macOS and Windows have no such dependent. Keep fspy declaring the feature for itself, keep the launcher enabling it for the revisions that do not, and say why. Also stop failing the summary and upload steps a second time when the benchmark failed before writing a report; the comment job already prints a placeholder for a missing platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The benchmark ran on thresholds set from measured noise and failed the job past them. Now that it runs on every pull request, most runs compare two identical fspy builds, where a threshold can only ever produce a false alarm. Print the change and its quartile spread and leave the judgment to the reader; the job stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
wan9chi
force-pushed
the
agent/benchmark-fspy-overhead
branch
from
July 28, 2026 08:37
63b9d52 to
dc57cf2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
fspy has no benchmark that can tell whether a change made tracking more expensive.
Earlier versions of this PR saved a baseline from a
mainrun and compared later runs against it. That cannot work. The two runs land on different runner instances, and identical instances disagree by more than any regression worth catching: tens of percent on absolute times, and still up to ten percent after normalizing against an untracked launch on the same machine.Changes
Nothing is compared across runs. Each pull request job builds
fspy_benchmark_launcher— the only benchmark piece that links fspy — twice: against the fspy under review, and against the merge-base fspy, from the same source. The harness launches both builds back to back on one machine, cycling every ordering, and reports the median head/base ratio. Whatever the runner does to the numbers, it does to both sides.Two rows per backend:
launch(wall clock of a tracked launch that opens nothing) andaccess(open batches timed inside the target by two threads). Backends:LD_PRELOADon Linux, seccomp user notification on Linux (static musl target),DYLD_INSERT_LIBRARIESon macOS, Detours on Windows.The benchmark only reports; it never fails the job. Each row prints the median change with its quartile spread, plus the overhead of tracking against an untracked launch as context.
The noise floor is small enough to read changes directly. Benchmarking the same fspy twice stayed within ±2.5% across four instances per platform (run 30322324380) and within ±2.3% across four repeats of one commit (run 30341333642). An artificially slowed fspy moved every row far past that on every platform (run 30322573395).
The benchmark runs on every pull request and finishes well inside the CI critical path (about one minute per platform with a warm cache). Runs on
mainkeep the cache warm and record the overhead history. Results post as job summaries and one sticky PR comment. Criterion, the saved baselines, and the baseline download are gone.