SPMI: per-context tpdiff examples#129948
Conversation
Add a "top method regressions / improvements" section to the tpdiff markdown summary, mirroring how asmdiffs surfaces specific contexts. The per-context data already lives in the details CSV; aggregate it once and emit FullOpts/MinOpts top-N tables by PDIFF %. Release JITs don't currently report `MethodFullName`, so the table also shows the SPMI context number for fallback lookup via `mcs -dumpMap` or `superpmi -c N`. Fixes dotnet#85755. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@jakobbotsch PTAL Sample outputExisting pivot section (unchanged): Overall (-4.84%)
FullOpts (-4.82%)
New per-context section (trimmed here to top 5 of each; the actual report shows top 20): Top method regressions (FullOpts, by PDIFF %)
Top method improvements (FullOpts, by PDIFF %)
The Method column is empty here because Release JITs (what tpdiff uses) don't currently |
There was a problem hiding this comment.
Pull request overview
Adds per-context throughput “examples” to SuperPMI tpdiff markdown summaries by extracting per-context instruction deltas from the existing details CSV and emitting top-N regression/improvement tables (split by FullOpts/MinOpts). Also updates the tuple shape flowing through aggregate_diff_metrics/tpdiff reporting and makes the markdown writer tolerant of older 3-tuple JSON summaries.
Changes:
- Extend
aggregate_diff_metrics()to collect per-context throughput diffs (base vs diff instruction counts) and return them to callers. - Plumb per-context throughput data through
replay_with_throughput_diff()and into markdown summary generation. - Add a new markdown section that lists top per-context regressions/improvements (FullOpts and MinOpts) by PDIFF%.
- Drop legacy 3-tuple tolerance and the try/except / .get defensive paths; callers always produce the current shape. - Bound per-context tp_diff retention via four top-K heaps (200 each) so very large MCH files can't blow up memory. - Escape '|' / HTML / newlines in markdown cells; drop the HTML-looking "<no name reported by JIT>" placeholder (use an empty cell instead).
|
@tannergooding @jakobbotsch can you re-review? |
tannergooding
left a comment
There was a problem hiding this comment.
Changes still LGTM, functionally speaking. Jakob is much more familiar with the long term implementation and goals of the script
Add a "top method regressions / improvements" section to the tpdiff markdown summary, mirroring how asmdiffs surfaces specific contexts. The per-context data already lives in the details CSV; aggregate it once and emit FullOpts/MinOpts top-N tables by PDIFF %.
Release JITs don't currently report
MethodFullName, so the table also shows the SPMI context number for fallback lookup viamcs -dumpMaporsuperpmi -c N.Fixes #85755.