Skip to content

SPMI JIT metric diffs: use neutral grey colors instead of green/red for percentages#125155

Closed
Copilot wants to merge 5 commits intomainfrom
copilot/update-jit-metric-color-scheme
Closed

SPMI JIT metric diffs: use neutral grey colors instead of green/red for percentages#125155
Copilot wants to merge 5 commits intomainfrom
copilot/update-jit-metric-color-scheme

Conversation

Copy link
Contributor

Copilot AI commented Mar 3, 2026

JIT metric diffs used green/red coloring for negative/positive percentages, implying improvement/regression semantics that don't hold across all metrics — whether a metric going up or down is "good" varies by metric.

Changes

  • superpmi.py: Add neutral parameter (default False) to format_pct(). When True, uses #888888 (positive) and #555555 (negative) instead of green/red. ASM diffs and throughput diffs are unaffected.
  • write_metricdiff_markdown_summary: Add local fmt_pct/fmt_pct_of wrappers that forward to format_pct(..., neutral=True), replacing all format_pct/compute_and_format_pct calls in that function.
  • assertionprop.cpp: Change max assertions count from 256 to 512 in countFunc to trigger metric diffs in CI and validate the color changes.

Security

No security-relevant changes.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…my JIT comment

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Update color scheme for JIT metric diffs SPMI JIT metric diffs: use neutral grey colors instead of green/red for percentages Mar 4, 2026
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 4, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@EgorBo
Copy link
Member

EgorBo commented Mar 4, 2026

@copilot please make a dummy change that will have an impact on metrics, e.g. let's change the max assertions count to be 512

…mmy comment

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 4, 2026

@copilot please make a dummy change that will have an impact on metrics, e.g. let's change the max assertions count to be 512

Done in 4a28673 — changed countFunc in assertionprop.cpp from {64, 128, 256, 128, 64} to {64, 128, 512, 128, 64}, and removed the dummy comment from jitconfigvalues.h since this real JIT change will trigger the metric diff job.

@jakobbotsch
Copy link
Member

Hmm, it is quite hard to read without the green/red. Perhaps it is better to keep it, we just should be careful about how we interpret the numbers. (I am mainly worried that people will start optimizing for these numbers unmotivated...)

@EgorBo
Copy link
Member

EgorBo commented Mar 4, 2026

Hmm, it is quite hard to read without the green/red. Perhaps it is better to keep it, we just should be careful about how we interpret the numbers. (I am mainly worried that people will start optimizing for these numbers unmotivated...)

yeah, doesn't look readable. let's keep it as is.

@EgorBo EgorBo closed this Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants