ci(benchmark): validate benchmark results

[ithewei]

Summary

benchmark.yml ran every benchmark but never checked the numbers, so a regression or a broken measurement could not fail the job. This PR saves each result and validates it with a new .github/scripts/benchmark_check.sh.

Measurement bugs fixed along the way:

• iperf direct baseline missing in 24 of the last 27 runs. The client connected before iperf -s was listening (tcp connect failed: Connection refused), and iperf still exits 0, so tcp_proxy_server had nothing to be compared against. The step now waits for both ports to listen and uses -y C for parseable output.
• tinyhttpd closed the connection after every file response. http_send_file called hio_close at EOF even for keep-alive requests, so wrk saw half of its requests unanswered and "tinyhttpd send file" mostly measured connection setup. Fixed in examples/tinyhttpd.c; the next request is now read only after the file is fully sent, so pipelined responses cannot interleave with the file body.
• tinyhttpd then stalled 40 ms per file request. With the connection kept open, Nagle held the body (written after the head) until the client's delayed ACK: 2.4k req/s, p50 41 ms. TCP_NODELAY is now set on accepted connections: 96k req/s, 1 ms.

Checks

Absolute numbers swing by up to 2x between GitHub-hosted runners (e.g. httpd send file 110k–243k req/s), so performance is judged by ratios against peers measured in the same run. nginx serves /ping from its default site on port 80 (return 200 "pong") for the ping comparisons. Ratios are compared at one decimal place; every check fails the job.

check (same run)	limit
libhv echo / median of the other echo servers	>= 1.0
tcp proxy / direct (iperf)	>= 0.4
httpd file (with FileCache) / nginx file	>= 1.2
tinyhttpd file (no FileCache) / nginx file	>= 0.8
httpd 404 / nginx 404	>= 0.8
tinyhttpd 404 / nginx 404	>= 0.8
httpd ping / nginx ping	>= 0.8
tinyhttpd ping / nginx ping	>= 1.0

wrk parse errors also fail the job (printed only when found), and every endpoint must return the expected status (200 / 404) before it is benchmarked. Results go to the job summary, with both sides of each ratio (MB/s, Gbit/s or req/s). Steps run with bash -eo pipefail so a failing command is not hidden by tee, and the job has a 30-minute timeout.

Verification

• Over the last 27 runs plus this PR's runs, every limit passes at one decimal place (libhv echo min 0.99, httpd file min 1.33, httpd 404 min 0.95, tinyhttpd 404 min 1.02; tcp proxy 0.45–0.56 over 5 runs; tinyhttpd file 1.06 with both fixes).
• The first CI run of this PR, before TCP_NODELAY, fails the tinyhttpd file check (0.0 < 0.8), so that regression would now be caught.
• tinyhttpd under ASan/UBSan: two file requests reuse one connection; Connection: close and HTTP/1.0 close after the response; file, /ping, /404, file on one connection; a 1 MB multi-chunk file twice on one connection, byte-identical; two pipelined requests answered in order. No sanitizer reports.

[Copilot]

🟡 Changes recommended

Ratio thresholds are evaluated after rounding, allowing some below-floor results to pass.

1 open finding

• Compare unrounded ratio when enforcing the threshold · New

What changed in this PR

Adds validation for benchmark correctness and performance while fixing inaccurate iperf and tinyhttpd measurements.

Changes:

• Captures benchmark output and validates same-run performance ratios.
• Adds startup, HTTP status, timeout, and failure-propagation safeguards.
• Preserves tinyhttpd keep-alive after file responses.

File	Description
examples/​tinyhttpd.c	Completes file responses before reusing connections.
.github/​workflows/​benchmark.yml	Captures outputs and runs validation.
.github/​scripts/​benchmark_check.sh	Parses and checks benchmark results.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Zero-throughput informational benchmarks can currently pass validation.

1 open finding

• Compare unrounded ratio when enforcing the threshold

Previously missed (1)

In code that hasn't changed since last review

Reject zero or non-numeric requests/sec measurements

.github/​scripts/​benchmark_check.sh:63

A zero numerator is considered valid here. In particular, wrk reports integer requests/sec, so tinyhttpd_file can complete only one response, stall one request on each of the 100 connections, report Requests/sec: 0, satisfy MAX_UNANSWERED, and then pass because its ratio is info-only. Reject zero/non-numeric inputs before applying the mode so a broken measurement cannot pass the job.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Empty-file response framing and incomplete echo baseline validation can produce hangs or false benchmark passes.

2 open findings

• Reject missing or zero peer baselines before median calculation · New
• Compare unrounded ratio when enforcing the threshold

Previously missed (1)

In code that hasn't changed since last review

Keep-alive hangs on zero-byte file responses

examples/​tinyhttpd.c:192

Keeping the connection open here breaks empty-file responses. http_response_dump only emits Content-Length when it is greater than zero, so a successful GET of a zero-byte file has neither a length nor chunked framing; once this path keeps the socket open, an HTTP/1.1 client cannot detect the response boundary and both sides wait indefinitely. Emit Content-Length: 0 for file responses (or mark the response nonpersistent before its headers are sent).

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Zero-result echo peers can falsely pass validation, and empty-file keep-alive responses lack valid framing.

2 open findings

• Reject missing or zero peer baselines before median calculation
• Compare unrounded ratio when enforcing the threshold

Previously missed (1)

In code that hasn't changed since last review

Keep-alive response lacks framing for zero-byte files

examples/​tinyhttpd.c:159

Keeping the connection alive is not valid for a zero-byte file response: http_response_dump only emits Content-Length when it is greater than zero, so the client has no framing signal and waits for a body or connection close. Preserve close-delimited framing for zero-length responses (or emit Content-Length: 0) before reusing the connection.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Missing echo-peer measurements can be silently excluded while validation still passes.

2 open findings

• Reject missing or zero peer baselines before median calculation
• Compare unrounded ratio when enforcing the threshold

🧠 Review effort: Balanced

[Copilot]

🟢 Approval recommended

The benchmark validation and tinyhttpd lifecycle changes are internally consistent with the surrounding APIs and workflow.

2 open findings

• Reject missing or zero peer baselines before median calculation
• Compare unrounded ratio when enforcing the threshold

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Zero-byte file responses can hang persistent HTTP/1.1 connections because they lack response framing.

2 open findings

• Reject missing or zero peer baselines before median calculation
• Compare unrounded ratio when enforcing the threshold

Previously missed (1)

In code that hasn't changed since last review

Emit Content-Length: 0 or close connection for empty files

examples/​tinyhttpd.c:159

A zero-byte file now leaves this connection open even though http_response_dump omits Content-Length when the length is 0. For an HTTP/1.1 response with neither a length nor chunked encoding, clients must wait for EOF to delimit the body, so the request can hang and a pipelined response cannot be parsed. Emit Content-Length: 0 for empty files, or close rather than reusing this connection.

🧠 Review effort: Balanced