Make profiling script failures visible instead of silently truncating#9127
Open
tautschnig wants to merge 1 commit into
Open
Make profiling script failures visible instead of silently truncating#9127tautschnig wants to merge 1 commit into
tautschnig wants to merge 1 commit into
Conversation
Profiling a long-running benchmark could produce a report saying only "Insufficient samples collected", indistinguishable from a genuinely short benchmark, when in fact the run had been killed by the timeout or perf post-processing had failed. Several fixes: - Record a timed_out flag per benchmark (propagated through multi-run averaging), warn loudly when a run is KILLED by the timeout, and list the affected benchmarks in the final report instead of the generic insufficient-samples message. - Warn when perf.data exists but yields zero parsed samples, distinguishing a truncated recording from a too-short benchmark. - Sample at 297 Hz instead of 997 Hz when the timeout indicates a long-running benchmark: DWARF stack recording at 997 Hz produces multi-GB perf.data files and enough overhead that the benchmark itself may no longer finish within the timeout. - Pass --no-buildid to perf record and --no-inline to perf report and perf script, and bound perf report with a timeout: build-id and inlined-frame post-processing spawn addr2line processes that were observed to hang indefinitely on stale ~/.debug build-id cache entries, stalling the whole profiling run after the benchmark had already completed. Neither is needed: results are analyzed in place, and the parsed report lines and collapsed stacks do not use inlined frames. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
tautschnig
requested review from
a team,
kroening and
peterschrammel
as code owners
July 22, 2026 09:10
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Improves profiling robustness by making perf/timeout failures explicit (rather than appearing as “insufficient samples”), and by reducing perf overhead for long-running benchmarks.
Changes:
- Add per-benchmark
timed_outtracking and surface timeout-killed runs in the final “no samples” report. - Adjust perf recording/reporting to avoid known hangs (
--no-buildid,--no-inline) and add timeouts aroundperf report. - Reduce sampling frequency for long-timeout benchmarks to control perf.data size/overhead.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| scripts/profiling/runner.py | Adds timeout visibility, adaptive sampling frequency, and safer perf invocation/processing. |
| scripts/profiling/analysis.py | Improves final “no samples” message by listing benchmarks killed by timeout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+16
to
+23
| # Sampling frequency for perf record. For benchmarks that are expected to | ||
| # run long (large timeout), DWARF stack recording at 997 Hz produces | ||
| # multi-GB perf.data files and enough recording overhead that the benchmark | ||
| # itself may no longer finish within the timeout. 297 Hz still yields plenty | ||
| # of samples on runs of a minute or more. | ||
| SAMPLING_FREQ = 997 | ||
| SAMPLING_FREQ_LONG = 297 | ||
| LONG_RUN_TIMEOUT = 300 |
Comment on lines
+150
to
+154
| report = subprocess.run( | ||
| ["perf", "report", "-i", str(perf_data), "--stdio", "--no-inline", | ||
| "--no-children", "--percent-limit", "0.5", "-n"], | ||
| capture_output=True, text=True, timeout=300) | ||
| report_stdout = report.stdout |
Comment on lines
+153
to
+158
| capture_output=True, text=True, timeout=300) | ||
| report_stdout = report.stdout | ||
| except subprocess.TimeoutExpired: | ||
| report_stdout = "" | ||
| warn(f"[{name}] perf report timed out after 300s — no hotspot data " | ||
| f"for this benchmark") |
| # CI containers/VMs). | ||
| perf_event = _detect_perf_event() | ||
|
|
||
| freq = SAMPLING_FREQ if timeout <= LONG_RUN_TIMEOUT else SAMPLING_FREQ_LONG |
Comment on lines
307
to
308
| if script.returncode != 0: | ||
| warn(f"[{result['name']}] perf script failed (exit {script.returncode})") |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #9127 +/- ##
===========================================
- Coverage 80.83% 80.82% -0.01%
===========================================
Files 1715 1715
Lines 189948 189948
Branches 73 73
===========================================
- Hits 153540 153533 -7
- Misses 36408 36415 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Profiling a long-running benchmark could produce a report saying only "Insufficient samples collected", indistinguishable from a genuinely short benchmark, when in fact the run had been killed by the timeout or perf post-processing had failed. Several fixes: