test(curl): prove write-callback CPU is not billed as network wait end to end - #1002
Open
Guikingone wants to merge 3 commits into
Open
test(curl): prove write-callback CPU is not billed as network wait end to end#1002Guikingone wants to merge 3 commits into
Guikingone wants to merge 3 commits into
Conversation
…d to end Closes #890. #880 stopped billing PHP callback CPU as outgoing network wait, but the coverage it left behind never looked at the number a user reads: `note_wait_excluding`'s saturating subtract is unit-tested in `elephc-monitoring-contract` and the nested-callback timer is stack-tested in the bridge, while nothing re-ran the proven PHP program through `--with-curl --with-monitoring` and asked the profile what it says. The path runs compiler -> probe/instr runtimes -> the curl bridge's published hook slots -> the exact capture -> the saved graph, and every piece can be individually correct with the reported number still being the burn. One program, compiled once through the CLI, run twice under `elephc monitor --save`: once burning ~130 ms of CPU inside a `CURLOPT_WRITEFUNCTION`, once burning the same CPU immediately after `curl_exec()` as a control. Both do one loopback GET against `http_fixture.rs` and one `burn()`; they differ only in which side of `curl_exec()` the burn sits on, so the two captures must agree. The control is what makes that a statement at all -- an absolute "network wait is small" bound would also pass on a build that stopped recording wait. Two runs rather than one program calling both shapes because `curl_exec()` is itself an elephc-PHP prelude function, so IT is the frame the wait lands on: a single run doing both folds them into one `curl_exec` node (`calls 2`) with nothing left to compare. Measured: 1.1 ms (in-callback) against 1.2 ms (control). With `note_wait_excluding(elapsed, callback_ns)` reverted to `note_wait(elapsed)` -- the pre-#880 behaviour at that one call site -- the in-callback run reports 133.9 ms and the fixture fails naming both numbers. A 210x gap against a 5 ms slack. This is the only curl fixture that compiles through the CLI: `--with-monitoring` is a compiler feature, not a link-plan one, so the harness's bare `ld` invocation cannot produce the binary. The managed natives therefore resolve through the PRODUCTION resolver, which keys artifacts on the toolchain fingerprint while `skip_without_curl_native`'s structural discovery does not -- a cache built by a different compiler satisfies the skip gate and still fails this compile. That failure is deliberately loud, since a second quieter skip gate is the shape `scripts/ci/run_curl_codegen_shard.sh` exists to prevent, and CI materializes the cache with `native install --locked` in the same job. The fixture writes the same canonical `examples/curl-get` manifest/lock pair through `include_str!` rather than keeping a copy that could drift. `curl_native::managed_native_cache_root()` is new: `elephc_cli_command` isolates the child's `XDG_CACHE_HOME`, which is right for every fixture that must not see developer state and wrong for one that needs the managed curl archives. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Greptile SummaryAdds an end-to-end cURL monitoring regression test that compiles an instrumented PHP fixture through the CLI, compares network-wait accounting with CPU work inside versus after the write callback, and uses the managed native cache explicitly.
Confidence Score: 5/5The PR appears safe to merge with no outstanding findings. The latest change fully addresses the prior control-response concern by validating the expected ten-byte body for both monitored runs. The other previous findings were also fixed and their threads are resolved; no new correctness, security, or repository-rule violations remain.
|
| Filename | Overview |
|---|---|
| tests/codegen/curl/mod.rs | Registers the new cURL monitoring regression-test module. |
| tests/codegen/curl/monitoring.rs | Adds a cohesive end-to-end fixture proving write-callback CPU is excluded from reported network wait; the latest change also validates the control response. |
| tests/codegen/support/curl_native.rs | Adds a documented helper that returns the managed native cache root for CLI-driven fixtures. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Fixture[PHP monitoring fixture] --> Compile[Compile with cURL and monitoring]
Compile --> Inside[Run burn inside write callback]
Compile --> Control[Run burn after curl_exec]
Inside --> InsideCapture[Save monitoring capture]
Control --> ControlCapture[Save monitoring capture]
InsideCapture --> Compare[Compare curl_exec network wait]
ControlCapture --> Compare
Reviews (3): Last reviewed commit: "test(curl): check the fixture body on BO..." | Re-trigger Greptile
…cache drift Three review follow-ups on #890, all of them about the fixture surviving things that have nothing to do with what it measures. THE BURN NO LONGER DEPENDS ON CHUNKING. libcurl does not promise to deliver a ten-byte body in one write callback, and the callback burned per chunk while the control burned exactly once after `curl_exec()`. A split body would have made the two sides do different amounts of work and fail the comparison for a reason unrelated to wait accounting. The callback now burns on its first invocation only, and the chunk count is reported so a split is visible rather than merely survived. THE PROGRAM'S OUTPUT IS DELIMITED. It shares one stdout with the monitor's own report, and reading "the first line" worked only because the report happens to open with a newline today. The fixture brackets its output with `<<`/`>>` and the harness reads between them. THE MANAGED-NATIVE MISMATCH IS A SKIP, NOT A MYSTERY FAILURE. The production resolver keys artifacts on a toolchain fingerprint that covers `PATH` and `TMPDIR`, not just the compiler, while `skip_without_curl_native`'s structural discovery ignores all of it -- so a cache that satisfies the gate can become unusable after nothing more than a shell change, and the local cure is a multi-minute from-source rebuild of the whole curl closure. That one diagnostic now skips, reported through the SAME `SKIP_GATE_MARKER` the existing gate uses, so `scripts/ci/run_curl_codegen_shard.sh` still turns it into a hard CI failure: CI materializes the cache with `native install --locked` in the same job and therefore the same environment, so reaching that branch there means the shard really did lose curl coverage. Every other compile failure still panics. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
…back one Review follow-up, and a gap the previous commit introduced: replacing the whole-output equality with a burn-result comparison left the control run's byte count unchecked. The burn result cannot stand in for it -- `burn` is pure and runs exactly once on each side either way -- so a control that received a short or empty response would still match, and the test would then compare network waits from two transfers that are not equivalent. Both runs are now asserted to report the ten-byte body before their burn results are compared, with the failing side named. Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
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
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.
Closes #890.
The gap
#880 stopped billing PHP callback CPU as outgoing network wait. What it left behind was coverage that never looked at the number a user reads:
note_wait_excluding's saturating subtract has unit coverage inelephc-monitoring-contract, and the nested-callback timer has stack-level coverage in the bridge, but nothing re-ran the proven PHP program through--with-curl --with-monitoringand asked the profile what it says.That matters here more than it usually does. The path runs compiler → probe/instr runtimes → the curl bridge's published hook slots → the exact capture → the saved graph. Every piece can be individually correct and the reported number still be the burn.
What the fixture does
One program, compiled once through the CLI with
--with-curl --with-monitoring, run twice underelephc monitor --save:insideCURLOPT_WRITEFUNCTIONafter(control)curl_exec()Both do exactly one loopback GET against
http_fixture.rs's server and exactly one ~130 msburn(). They differ only in which side ofcurl_exec()the burn sits on, so the two captures must agree — and the control is what makes that a real statement, since an absolute "network wait is small" bound would also pass on a build that stopped recording wait at all.Two runs rather than one program calling both shapes:
curl_exec()is itself an elephc-PHP prelude function, so it is the frame the wait lands on, and a single run doing both folds them into onecurl_execnode (calls 2) with nothing left to compare.Measured
Against the current bridge:
With
note_wait_excluding(elapsed, callback_ns)reverted tonote_wait(elapsed)— the pre-#880 behaviour at that one call site — the same fixture:and the test fails with its own message:
A 210x gap against a 5 ms slack, so the threshold is not doing any work the physics doesn't already do.
Why it compiles through the CLI
It is the only curl fixture that does.
--with-monitoringis a compiler feature — probe emission, instrumented frames — not a link-plan one, socompile_and_run's bareldinvocation cannot produce this binary.The consequence is worth stating plainly: the managed native packages then resolve through the production resolver, which keys artifacts on a toolchain fingerprint that covers
PATHandTMPDIRas well as the compiler (native_deps::toolchain::fingerprinted_environment), whileskip_without_curl_native's structural discovery ignores all of it. A cache that satisfies the gate can therefore be unusable here after nothing more than a shell change — which happened to me mid-review, with a ~30-minute from-source rebuild of the whole curl closure as the cure.So that one diagnostic skips, reported through the same
SKIP_GATE_MARKERthe existing gate uses. That keeps CI honest without a second, quieter gate:scripts/ci/run_curl_codegen_shard.shgreps that token out of the shard log and fails the job, and CI materializes the cache withnative install --locked --manifest-path examples/curl-get/elephc.tomlin the same job — same environment, same fingerprint — so reaching that branch there means the shard really did lose curl coverage. Every other compile failure still panics.The fixture writes that same canonical manifest/lock pair via
include_str!rather than keeping a copy that could drift.Every archive it needs (
libelephc_curl.a,libelephc_probe.a,libelephc_instr.a) is already in.config/nextest.toml's archive include list.Also here
curl_native::managed_native_cache_root()—elephc_cli_commandisolates the child'sXDG_CACHE_HOME, which is right for every fixture that must not see developer state and wrong for one that needs the managed curl archives, so this hands the childELEPHC_NATIVE_CACHEexplicitly.Verification
cargo test --test codegen_tests curl::passes (including the new fixture, 22.6 s);cargo buildis warning-free. No production code changes.🤖 Generated with Claude Code
https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr