Bound v0.57 differential trace capture deterministically - #67
Conversation
The comparison flow now stops QEMU after a configurable trace prefix instead of relying on the guest finisher to terminate the process. Cleanup also owns the child process so interrupted runs cannot leak an emulator. Constraint: The mcopy trace fixture intentionally enters a pass/fail terminal loop after its MMIO result. Rejected: Add a platform-specific timeout dependency | macOS does not provide the GNU timeout command by default. Confidence: high Scope-risk: narrow Directive: Keep the default prefix above the six-commit fallback comparison window. Tested: clean merged QEMU versus pyCircuit fallback trace, six commits matched Not-tested: Full non-fallback pyCircuit trace
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the QEMU trace generation script to run QEMU in the background and poll its trace file until a minimum number of records is reached, rather than running it indefinitely. It also adds a cleanup function to terminate the QEMU process upon script exit. The feedback suggests increasing the polling sleep interval from 10ms to 100ms to prevent high CPU utilization caused by spawning processes in a tight loop, and safely capturing the line count to avoid potential syntax errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| while kill -0 "${QEMU_PID}" >/dev/null 2>&1; do | ||
| if [[ -s "${QEMU_TRACE}" ]] && | ||
| [[ "$(wc -l <"${QEMU_TRACE}")" -ge "${QEMU_TRACE_MIN_RECORDS}" ]]; then | ||
| kill -TERM "${QEMU_PID}" >/dev/null 2>&1 || true | ||
| break | ||
| fi | ||
| sleep 0.01 | ||
| done |
There was a problem hiding this comment.
Polling every 10ms (sleep 0.01) in a tight loop spawns two external processes (wc and sleep) up to 100 times per second. This process-spawning overhead can cause extremely high CPU utilization and significantly slow down execution, especially on macOS or virtualized environments. Additionally, fractional sleep values are not strictly POSIX-compliant and may fail on some minimal environments.
Consider increasing the sleep interval to 0.1 (100ms) or 0.05 (50ms) to drastically reduce CPU thrashing while remaining highly responsive. Also, capturing the output of wc -l in a variable with a default fallback (e.g., ${num_lines:-0}) prevents potential Bash syntax errors if the command substitution returns an empty string.
| while kill -0 "${QEMU_PID}" >/dev/null 2>&1; do | |
| if [[ -s "${QEMU_TRACE}" ]] && | |
| [[ "$(wc -l <"${QEMU_TRACE}")" -ge "${QEMU_TRACE_MIN_RECORDS}" ]]; then | |
| kill -TERM "${QEMU_PID}" >/dev/null 2>&1 || true | |
| break | |
| fi | |
| sleep 0.01 | |
| done | |
| while kill -0 "${QEMU_PID}" >/dev/null 2>&1; do | |
| if [[ -s "${QEMU_TRACE}" ]]; then | |
| num_lines=$(wc -l <"${QEMU_TRACE}") | |
| if [[ ${num_lines:-0} -ge "${QEMU_TRACE_MIN_RECORDS}" ]]; then | |
| kill -TERM "${QEMU_PID}" >/dev/null 2>&1 || true | |
| break | |
| fi | |
| fi | |
| sleep 0.1 | |
| done |
1 participant
Summary
Validation