Perform a complete review

[jaydrogers]

Hey @danpastori,

So I have all the plumbing done for BenchKit. This includes a lot of work to make it more community focused and a sandbox for us all to learn from.

🫵 Where I need you

Please dive deep into the code and see if this approach makes sense. It will be a good test for you to read through the docs and see if you can figure this out. If not, just drop your questions below and I will answer them through documentation.

Testing accuracy

The biggest thing I am curious about:

• Do we need 4 endpoints to test? Do they all make sense?

Right now we have:

1. Static
2. JSON
3. DB Read
4. I/O Load

Please highly scrutinize this code and make sure we have accuracy. If you need to open a PR for me to review your changes, by all means go right ahead.

Thanks!

[danpastori]

@jaydrogers Here are some thoughts on BenchKit:

Overall

Overall, this is really awesome and will be extremely helpful validating environments and set ups. As quickly as PHP moves the optimizations will be able to be seen with each release which is awesome!

Few Items to Address in Later Releases

I think for version 1.0, this is good and we can start to get some feedback, but here are some items that I'd bring to your attention for future releases.

1. Make Isolated PHP Tests More Prominent

In the longer version of the tests, we run PHP Bench against PHP functions. Functions that perform hashing, array sorting, etc. I think we should bring some of these into the "quick" tests. We don't need a ton of iterations, but what's cool about these tests are they isolate how fast PHP itself is running. No dependencies. Just straight PHP. I think this will be very interesting as the PHP language itself gets optimized and also how it runs in certain environments such as FrankenPHP vs NGINX vs Apache. We already have this built in, but this should be in the quick tests and on the display card.

2. Re-visit HTTP Tests

To evaluate a self-hosted environment, the most important metric is how fast does that server respond to HTTP tests, what load, what response time, etc. The way these are set up now is all HTTP tests are run on the same server. While this is able to be replicated across all environments and performs tests the same way, there are a lot of variables that get mixed in that could pollute the results. I feel like if we are going to get flamed on the internet, this is the specific test section. A) Because it's the most important. B) Because it's the most inconsistent.

I think we break this down based off of the 2 speeds of test run:

• Quick Test: Checkbox notifying that OHA will run on the same machine and will be subject to slight in-accuracies. It's still valuable, but it's also not "as accurate as science"
• Long Test: Include the possibility of hitting your BenchKit box from another box and capturing the timing. This will bake in directly to the results right from the BenchKit box. This will be the most accurate representation of the speed of the self-hosted box if we can get the load of the test off of the same machine. We could have a specialized Docker container built just to run these commands if needed. Open to ideas.

Below is an outline from my chat with Claude on how this should be implemented:

AI assisted proposed plan on how remote OHA should run.

Proposal: remote load generation for the HTTP stage

Status: proposal / request for comment
Affects: http stage, run orchestration, result schema, gallery comparability

Summary

Today BenchKit runs oha on the same machine it is testing. The application, the database, and the load generator all compete for the same CPU. That is the right default — it is what makes an A/B comparison clean, because the request path is identical on both sides — but it means the absolute requests-per-second figure is always lower than the machine could actually serve.

This proposal adds a second way to run the HTTP stage, without removing the first:

Mode	Generator runs	What it is good for
Local (default, today's behaviour)	On the target box	A/B comparisons, quick results, zero setup
Distributed (new)	On a second machine you control	The honest absolute number

In Distributed mode the run pauses when it reaches the HTTP stage, prints a command to run on a second machine, shows the traffic arriving live, ingests the generator's results, and continues the run with those numbers baked into the result document as though the stage had run normally.

Both modes gain a shared guardrail — described in The generator-bound check — that detects when the load generator, rather than the server, was the bottleneck.

---

Why generator placement matters more than it looks

oha -c 50 is a closed-loop generator: it holds 50 requests in flight and sends the next one only when a reply comes back. Its maximum achievable rate is therefore bounded by round-trip time, not by the server:

max RPS ≈ connections / (RTT + server service time)

With the standard load (-c 50), that ceiling looks like this:

Generator location	Typical RTT	Ceiling at -c 50
Same box (today)	~0.05 ms	~48,000 rps
Second box, same datacenter	~0.3 ms	~38,000 rps
Second box, same region	~2 ms	~16,000 rps
Laptop, same city	~15 ms	~3,100 rps
Laptop, cross-country	~60 ms	~820 rps

This is the single most important constraint in the whole design, and it has two consequences.

The generator must be near the target. A second box in the same datacenter, region, or private network. At 60 ms of RTT, a server capable of 20,000 rps and a server capable of 2,000 rps both report roughly 820 — the number stops describing the server and starts describing the network path. That is worse than the co-location problem it was meant to fix, because a co-located generator at least still scales with the server.

A laptop over the public internet is not a valid generator for this stage. It also brings home upstream bandwidth limits, NAT connection-tracking limits and ephemeral port churn at 50+ connections, wifi jitter, ISP shaping, a real TLS handshake per connection, and possibly a CDN in front. It is the least controlled environment in the entire measurement chain.

Driving load from a laptop is still a useful thing to do — but it answers "what does a user on the internet experience?", which is an end-to-end latency question, not a throughput question. That belongs as its own separately-labelled measurement and is explicitly out of scope here.

What this proposal does not change

BenchKit itself keeps running on the machine under test. This matters: ServerSpecs reads the local /proc/cpuinfo, YABS and fio run locally, DatabaseSpecs reads the local connection, and phpbench runs locally. Only the four oha JSON files arrive from outside. The orchestrator never leaves the machine it is describing, so every recorded spec stays correct — the only thing that moves is where the traffic originates.

---

Mode 1 — Local generator (default)

Behaviour is unchanged from today. BenchmarkStages::httpStage() builds the oha chain via HttpBenchCommand and runs it as a subprocess on the same box.

What changes is disclosure:

1. http-meta.json records generator.mode: "local".
2. The result page carries an explicit note that the generator shared the CPU with the server, sized to the machine: on a many-core box the cost is real but small, on 1–2 vCPUs it is not small, and it grows as the server gets faster (a faster server needs a busier generator to saturate it).
3. The generator-bound check runs and can raise a caveat.

The message is "this number is a floor, and it is a good floor for comparing two runs on this machine" — not "this number is wrong."

Optional: report what the generator cost

A cheap addition that turns an unquantified caveat into a number: wrap the oha invocations with /usr/bin/time -v (or read getrusage(RUSAGE_CHILDREN) around the stage) and record generator CPU-seconds against wall × cores in the meta file.

The result then says "the load generator used 11% of available CPU" instead of a paragraph of prose, and RunCaveats can raise a chip past a threshold. This works on every existing installation and requires no second machine.

---

Mode 2 — Distributed generator

The flow

 ┌─ Target box (BenchKit) ────────────┐          ┌─ Generator box ──────────┐
 │                                    │          │                          │
 │  1. Hardware stage      (runs)     │          │                          │
 │  2. Network stage       (runs)     │          │                          │
 │  3. HTTP stage:                    │          │                          │
 │       issues a token               │          │                          │
 │       prints a command  ───────────┼─ copy ──▶│  4. paste + run          │
 │       waits...                     │◀── ping ─┤     handshake            │
 │       "Generator connected"        │          │                          │
 │       "Route 1 of 4: warmup"    ◀──┼── ping ──┤     oha warmup           │
 │       "Route 1 of 4: measuring" ◀──┼── ping ──┤     oha measure          │
 │       "Received /bench/static"  ◀──┼── POST ──┤     upload JSON           │
 │        ... x4 ...                  │          │                          │
 │       "Load test complete"         │◀── done ─┤  5. exits                │
 │  6. PHP stage           (runs)     │          │                          │
 │  7. Snapshot saved                 │          │                          │
 └────────────────────────────────────┘          └──────────────────────────┘

Step 1 — Selecting the mode

A new setting on the start screen, alongside the existing HTTP duration/connections/io_ms controls:

• Load generator: This machine (default) / Another machine

Choosing "Another machine" changes the run estimate. The HTTP stage's contribution today is computed in useSettings.js as 4 × (duration + 3) / 60 minutes; in Distributed mode the estimate should show the same figure plus "+ time to set up the generator", or simply be marked as open-ended, because it depends on a human.

Step 2 — The pause

When the run reaches the HTTP stage in Distributed mode:

1. httpStage() performs its existing preparation — BenchmarkHttpItems::ensure(), WebRuntimeSpecs::capture(), writeMeta(). All of this stays exactly as it is; the target still describes its own serving process.
2. It generates a single-use, run-scoped token.
3. Instead of returning the oha chain, it returns php artisan benchmark:await-remote-load as the stage command.

This last point is worth calling out because it means no changes to the run loop at all. RunBenchmark::runStage() already takes a command string, runs it through BenchmarkProcess, streams every output line into the run log tagged with the stage, and polls RunState::cancelRequested() about once a second. A waiting Artisan command slots into that contract unmodified:

• it prints progress lines, which stream to every watching browser tab through the existing run.log byte-offset mechanism;
• it polls for the cancel flag itself and exits cleanly;
• it exits non-zero on timeout, and the stage is marked error like any other failed stage;
• BenchmarkProcess's 30-second heartbeat already prevents the console from reading as hung.

Step 3 — The command the user runs

The waiting command prints something copy-pasteable:

╭─ Waiting for a remote load generator ────────────────────────────────╮
│                                                                      │
│  Run this on a machine near this server — same datacenter or         │
│  region. A laptop over the internet will measure your network,       │
│  not this box.                                                       │
│                                                                      │
│    curl -fsSL https://198.51.100.10/bench/generator/a1b2c3d4 -o bk.sh \
│      && sh bk.sh                                                     │
│                                                                      │
│  Requires: oha (brew install oha / cargo install oha)                │
│  Waiting... 0:42 elapsed, times out in 29:18                         │
╰──────────────────────────────────────────────────────────────────────╯

The script is generated by the server, from HttpBenchCommand. This is a hard requirement, not a convenience. The load must be identical to what a local run produces: 3 seconds of discarded warmup per route (HttpBenchCommand::WARMUP_SECONDS), then -c {connections} --no-tui --redirect 0 --output-format json, --insecure when the target is HTTPS, ?ms={io_ms} appended to the /bench/io route, and the four routes run in HttpBenchmarkResults::ROUTES order.

If the generator script and HttpBenchCommand are allowed to drift, a Distributed run is no longer the standard BenchKit load and cannot sit next to a Local one. (The existing "Test from your own machine" modal has already drifted this way — it hand-writes oha -z 30s -c 50 … and omits the warmup entirely.) Generating from one source of truth fixes both.

A --dry-run or "show me the raw commands" option should exist for anyone who won't pipe a downloaded script to a shell.

Step 4 — Seeing the traffic arrive

This is the part users most want to see, and the tempting implementation is the wrong one.

Do not instrument the bench routes. Adding a counter, a log line, or any per-request bookkeeping to /bench/static and friends puts work inside the thing being measured. The routes are deliberately minimal — that is the entire point of the static route.

Use a control plane instead. The generator script sends a handful of out-of-band pings to a separate endpoint, none of them during a measured window:

Ping	When	Console line it produces
handshake	Before anything runs	Generator connected from 203.0.113.7 — oha 1.4.5, 8 cores, RTT 0.4ms
route.start	Before each route's warmup	Route 2 of 4: /bench/json — warming up (3s)
route.measure	After warmup, before measurement	Route 2 of 4: /bench/json — measuring (30s)
route.result	After each route	Received /bench/json — 18,204 req/s
done	At the end	Load test complete

Nine or ten HTTP requests total, all between measured windows, zero cost to the measurement. The handshake alone gives the "I can see it connecting" moment, and it is also where the generator's own description gets recorded.

The RTT reported at handshake should be measured by the generator (a few sequential requests to /bench/static before any load), because it feeds the generator-bound check.

Step 5 — Ingesting the results

New endpoints in routes/bench.php, which is already registered with Route::middleware([]) — no session, no CSRF:

Method	Path	Purpose
GET	/bench/generator/{token}	Returns the generated shell script
POST	/bench/generator/{token}/status	Control-plane pings (step 4)
POST	/bench/generator/{token}/result	One route's oha JSON

The result endpoint writes the uploaded JSON to HttpBenchmarkResults::routePath($key) — the exact path a local run would have written. Everything downstream then works unchanged: detail() already parses oha's native --output-format json, execute() already assembles the stage, AssembleResultsDocument already picks it up, and the result page already renders it. The ingestion surface is genuinely small because the file format is already the right one.

Validation on upload, before the file is written:

• token matches the current run's token and the run is still in the waiting state;
• route key is one of HttpBenchmarkResults::ROUTES;
• body parses as JSON, is under a size cap, and contains summary.requestsPerSec;
• summary.totalData > 0 — the existing hasMeasuredTraffic() rule, which catches a web server answering empty 200s without ever invoking PHP;
• status code distribution is overwhelmingly 2xx.

A rejected upload should print the reason to the run console, not fail silently — the user is standing at a terminal on another machine and needs to know why.

Step 6 — Resuming

Once all four routes have arrived (or the done ping is received), the waiting command exits 0, the stage is marked completed, and the run continues into the PHP stage. From that point the run is indistinguishable from a local one apart from its recorded generator metadata.

---

The generator-bound check

This is the most important piece of the proposal and should ship first, before any of the remote machinery. Without it, nothing stops someone running the generator from a laptop in another country and submitting the result.

oha's JSON already reports summary.fastest, which is a usable floor-RTT proxy. The check is:

ceiling  = connections / fastest_seconds
bound    = observed_rps >= ceiling * AT_CEILING

If a run lands near that ceiling, the generator's connection count and round-trip time were the limit — the throughput figure describes the network path, not the server.

This is the same shape as the pool-limited detection that already exists in HttpBenchmarkResults::isPoolLimited():

return $observed >= ($workers * (1000 / $ioMs)) * self::AT_CEILING;

Same AT_CEILING constant (0.85), same nullable-when-undetectable behaviour, same caveat-chip plumbing on the result page. A isGeneratorBound() sitting next to it is a small, self-contained change that improves every existing installation — including local runs, where it will occasionally catch a genuinely saturated loopback.

Suggested caveat copy:

The load generator was the bottleneck. At 50 connections and 61 ms of round-trip time, no generator could have exceeded ~820 req/s regardless of how fast this server is. This number describes the network path between the generator and the server. Run the generator closer to the target.

---

Data model

http-meta.json

HttpBenchmarkResults::writeMeta() gains a generator block:

{
  "target": "http://localhost:8080",
  "mode": "loopback",
  "tls": false,
  "duration_seconds": 30,
  "connections": 50,
  "io_ms": 100,
  "workers": 16,
  "generator": {
    "mode": "remote",
    "rtt_ms": 0.41,
    "source_ip": "10.0.1.24",
    "oha_version": "1.4.5",
    "host": "Ubuntu 24.04, 8 cores",
    "cpu_share": null
  }
}

For a local run, mode: "local", source_ip and host null, rtt_ms derived from the fastest observed latency, and cpu_share populated if the optional generator-cost measurement is implemented.

The parsed stage output gains a matching generator object plus a generator_bound boolean, alongside the existing oversubscribed and pool_limited flags.

Schema version

Two readings, and they should be settled before implementation:

The additive reading. Every existing run is provably local — remote generation did not exist — so a consumer can back-fill generator.mode = "local" for anything below the new version with no ambiguity. No published number changes meaning. Under the repo's stated rule ("bump whenever a measurement changes enough that old runs cannot sit in the same gallery as new ones"), this does not require a bump.

The partition reading. The moment the gallery contains remote runs, the aggregate changes meaning: a chart that mixes a co-located generator with a datacenter-local one is charting two different measurements, in the same way that mixing io_ms=100 with io_ms=500 would be.

My recommendation is the additive reading — do not bump for the field — combined with a hard rule that the gallery partitions on generator.mode and never draws the two populations on one axis. That gets the correctness without evicting every existing submission from the gallery.

If a bump is taken anyway, it needs all three of: AssembleResultsDocument::SCHEMA_VERSION, SCHEMA_VERSION in docs/shared/submission/validate.mjs, and a SUPERSEDED_SCHEMAS entry saying why — tests/Feature/SchemaVersionTest.php enforces the first two agree and that the third exists.

Validator changes either way:

• accept an optional generator block and type-check it;
• require generator.mode to be present on runs at or above the version that introduced it;
• reject any submission where generator_bound is true, with a message explaining that the generator, not the server, was being measured.

---

Security

The ingest endpoints accept data that ends up in a published result, so they need more care than the read-only bench routes.

• Token. Generated per run, at the moment the HTTP stage starts. Cryptographically random, single-use per route, invalid the instant the stage ends (completed, timed out, or cancelled). Not derived from the run ID, which is predictable and appears in URLs.
• Window. The endpoints 404 whenever no run is waiting. The attack surface exists only during the wait.
• First-writer-wins. Once a route's JSON has been accepted, later uploads for the same route are rejected. Otherwise a second generator can overwrite a legitimate result.
• Size cap and shape validation on upload, as listed in step 5.
• Rate limiting on the status and result endpoints.
• Log the source IP of every accepted upload into the run console and the meta file, so the result carries a record of where its numbers came from.

The FAQ entry on public exposure should be extended: BenchKit remains a temporary instance you shut down when finished, and Distributed mode means the generator box must be able to reach it — which usually means a private network between the two, not a public IP.

None of this makes an uploaded number trustworthy in an adversarial sense; a determined submitter can always fabricate JSON. It makes accidental corruption — a second generator, a stale script, a half-finished run — impossible, which is the realistic threat.

---

UI changes

Start screen. A "Load generator: This machine / Another machine" control in the HTTP settings group. Choosing "Another machine" shows a short inline warning about generator placement before the run starts, not after.

Run console. The waiting state needs to be visually distinct from a running stage — a boxed panel with the command, a copy button, elapsed time, and the timeout countdown, rather than a scrolling log. The control-plane pings then fill in below it. A "cancel this run" affordance must remain reachable throughout, and the existing cancel-flag mechanism already covers it.

Result page. A generator badge next to the HTTP section — local or remote — and, for remote runs, the generator's description and measured RTT. When generator_bound is true, a caveat chip in the same style as the existing pool-limited chip.

Existing modal. ExternalEndpointsModal.vue ("Test from your own machine") stays, but its purpose narrows: it is for ad-hoc probing and for the coordinated-omission-corrected latency command, which remains genuinely useful and is unaffected by all of this. Its throughput commands should be regenerated from HttpBenchCommand so they stop drifting, and it should link to Distributed mode for anyone who wants results in their run rather than in their terminal.

---

Failure modes

Situation	Behaviour
Nobody ever runs the command	Stage times out (default 30 min, configurable), marked error, run continues to the PHP stage. Everything else in the run is still saved.
Generator dies mid-run	Routes already uploaded are kept; the stage times out. A partial HTTP stage should be recorded as partial, not silently presented as complete.
User cancels while waiting	Existing cancel flag; the waiting command exits, the run is cancelled and not saved — same as any other cancelled run.
Generator can't reach the target	Handshake never arrives, so the console shows nothing after the command. The waiting panel should say what it is waiting for and suggest checking firewall rules between the two boxes.
Two generators run at once	First-writer-wins per route; the second is rejected with a console message.
Uploaded JSON is malformed or shows zero bytes	Rejected on upload with the reason printed to the console; that route stays unfilled.
Target restarts mid-wait	The run is reconciled to interrupted by the existing PID-liveness logic in RunState::current(); the token dies with it.

---

Suggested implementation order

Each phase is independently shippable and useful on its own.

Phase 1 — Disclosure and the guardrail. generator.mode in the meta file (always local for now), isGeneratorBound(), the result-page caveat, validator support, docs. No remote machinery at all. This immediately improves every existing installation and makes the existing "test from your own machine" advice enforceable rather than advisory.

Phase 2 — One source of truth for the command. Refactor HttpBenchCommand so it can emit either the local shell chain or a portable script for another machine. Regenerate ExternalEndpointsModal's commands from it. Fixes the existing drift as a side effect.

Phase 3 — The waiting stage. benchmark:await-remote-load, the token, the generator script endpoint, the status endpoint, the console panel. At the end of this phase a user can watch traffic arrive but must still paste results in by hand.

Phase 4 — Ingestion. The result endpoint, validation, first-writer-wins, resumption. This is the phase that completes the feature.

Phase 5 — Gallery partitioning. Separate local and remote populations wherever runs are aggregated or compared.

---

Open questions

1. Enforce or document the placement rule? Should a generator-bound run be refused as a gallery submission, or accepted with a prominent caveat? I lean toward refusing — a number that measures the wrong thing is worse than a missing row — but it is a policy call.
2. Timeout default. 30 minutes is a guess. Long enough to provision a second box, short enough that an abandoned run doesn't hold the single-run claim indefinitely.
3. How much should the generator have to prove? Reporting RTT and its own CPU count is easy. Requiring a minimum core count or a maximum RTT before accepting a run is stricter and might frustrate legitimate setups.
4. Should the generator script be a shell script, a container image, or both? docker run serversideup/benchkit-generator --target https://... --token ... is one command with no oha install step, and removes an entire class of "wrong oha version" problems.
5. Multi-generator runs. Two or three generator boxes driving one target would push a large machine harder than a single box can. Interesting, considerably more complex, and probably a later proposal — but the token and ingest design should not actively prevent it.
6. Should Distributed mode also capture the corrected-latency run? The -q <rate> --latency-correction command gives honest tail latency, and a remote generator is exactly the right place to run it. It needs a throughput number first to pick a rate, so it would be a fifth step after the four routes.

3. Small Fixes

• With the database testing, each CRUD operation performs a different amount of operations. We just need to show that to the user or explain that somewhere. I believe read is like 1000 records, create is 500, etc.

Other than that, this is all looking great!