Capture Windows benchmark results: benchmarks write their own result file - #119
Merged
Conversation
…file Windows CI could see exit codes but never a number. The benchmarks run under DLL injection (withdll.exe), and their stdout was not captured -- hoard.txt held withdll's banner and nothing else. So Windows was crash-tested but never performance-tested, and a Windows performance regression would have gone completely unnoticed. (It is also why removing `#define inline __forceinline` could not be verified there: no number existed to look at.) alloc8#14 fixed the underlying handle bug -- the Detours withdll sample launches the target with a zeroed STARTUPINFO, so it never sets STARTF_USESTDHANDLES and the target does not inherit a REDIRECTED stdout. That fix is correct and worth having. But it is not sufficient on its own: with the fixed withdll in place, the benchmark output still did not appear. Rather than keep bisecting a Windows-only path through CI, make capture robust by construction: each benchmark appends its own result line to the file named by BENCH_OUT (bench_common.h's bench_report), opening and fclosing it itself. That depends on neither handle inheritance nor on the CRT flushing a buffer during teardown, so the result survives either way. stdout behaviour is unchanged, so Linux and macOS see exactly what they saw before. The Windows job now prints the results and fails loudly if the file is missing -- silence can no longer masquerade as success. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 15, 2026
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.
Windows CI could see exit codes but never a number.
The problem
The benchmarks run under DLL injection (
withdll.exe), and their stdout was not captured —hoard.txtheld withdll's banner and nothing else. So Windows was crash-tested but never performance-tested, and a Windows performance regression would have gone completely unnoticed.It's also why removing
#define inline __forceinline(#117) couldn't be verified there: no number existed to look at.What I found
alloc8#14 (merged) fixed the underlying handle bug: the Detours
withdllsample launches the target with a zeroedSTARTUPINFO, so it never setsSTARTF_USESTDHANDLESand the target doesn't inherit a redirected stdout. That fix is correct and worth having on its own.But it wasn't sufficient — with the fixed withdll in place, the benchmark output still didn't appear. Rather than keep bisecting a Windows-only path one slow CI cycle at a time, I made capture robust by construction.
The fix
Each benchmark appends its own result line to the file named by
BENCH_OUT(bench_common.h'sbench_report), opening andfclose-ing it itself:That depends on neither handle inheritance nor the CRT flushing a buffer during teardown, so the result survives either way.
What this unlocks
Windows can now be performance-measured at all — which is the prerequisite for extending the QoS gate to it.
🤖 Generated with Claude Code