Have the profiler report computed values and warnings in the json too - #9392
Open
abadams wants to merge 6 commits into
Open
Have the profiler report computed values and warnings in the json too#9392abadams wants to merge 6 commits into
abadams wants to merge 6 commits into
Conversation
The JSON output (HL_PROFILER_JSON_OUTPUT) now includes, per pipeline, a "warnings" array holding the full text of each performance warning that fired, and, per Func, the recompute ratio shown in the report's recompute column. The warning text is the same plain-English message the report prints, so consumers (LLMs especially) don't have to parse the table. It's collected into a small growable string per pipeline while the text report renders it, then emitted in the JSON pass. Also document, in HalideRuntime.h, how each profiler struct field aggregates across runs: peaks are maxima, time is summed over billed_runs, the remaining counters are summed over runs (divide by runs for a per-run value), the active-threads pair is a ready-made average, and memory_current is a live snapshot. Several consumers were dividing the wrong fields or not dividing the counters at all. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The report aggregates each Func's counters with those of its descendants for its parent rows; expose that same subtree rollup for every counter in the JSON, so agents consuming the JSON don't have to walk the parent pointers to reproduce it. Every field is summed, including the memory and stack peaks (a Func and its descendants can be live at once, so summing is a pessimistic bound on the subtree's peak footprint). The subtree rollup is now computed once per pipeline while printing the report (which needs it anyway) and persisted for the JSON pass, rather than recomputed. parallel_tasks, which the report latches downward for its "realized inside N tasks" warning, is computed into a small side array so the report keeps its latched value while the JSON gets a uniform subtree sum. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The JSON warnings array only carried the per-Func (numbered) warnings; the pipeline-level ones the report prints as bullets — too many auto-named Funcs, too few samples, and expensive frees — were omitted, and the array wasn't emitted at all unless a per-Func warning also fired. Emit the pipeline-level warnings too (in the same order as the report), and build the array whenever any warning fires. The message text is factored into a shared helper so the report and JSON can't drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per-Func (numbered) warnings were appended to the pipeline's top-level warnings array. Move them into a "warnings" array on each Func's JSON entry instead, grouped by canonical id (they fire per canonical Func, so every instance shows the same set). The top-level array now holds only the pipeline-level warnings. The JSON escaping is factored into a shared template helper used by both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cumulative subtree rollup summed every counter but kept a separate downward-latched parallel_tasks just for the report's warning, leaving the JSON cumulative parallel_tasks a subtree sum. Latch it directly in the cumulative stats instead, so the report and the JSON see the same value, and document that parallel_tasks is the odd one out (a latched task count, not a subtree sum). Drops the separate latched_tasks array. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Abort (halide_abort_if_false) if any of the JSON-side allocations return null — the three side arrays, the pipeline-level warnings string, the per-Func warnings array and its per-canonical strings, and the persisted cumulative copy. A failed small allocation means the system is in a bad state, so bailing out immediately beats limping along. Legitimate "not built" nulls (a skipped pipeline's cumulative, a pipeline/Func with no warnings) are still handled and emit the empty case. - Bound the cumulative parent-propagation and parallel_tasks latch with parent < num_funcs, not just parent >= 0. The tree builder deliberately tolerates orphans whose parent points outside the array; without the upper bound those would index cum_stats out of range (a pre-existing gap that this change had widened to the full counter region). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
|
Did you mean to create a stack here? |
Member
Author
|
Yes I did. Should be a stack now. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## abadams/profiler_report_isatty #9392 +/- ##
===============================================================
Coverage 70.09% 70.09%
===============================================================
Files 261 261
Lines 79362 79362
Branches 19349 19349
===============================================================
+ Hits 55628 55632 +4
- Misses 17898 17911 +13
+ Partials 5836 5819 -17 ☔ 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
2 participants
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.
Agents would rather ingest json than pretty-printed tables. This PR changes the profiler backend to put everything in the json output - not just the raw counters. This mostly means the warnings and the cumulative stats. This PR also makes it clearer in HalideRuntime.h what the denominator is for each counter. Most are summed across runs.