Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPMI: Fix 'misses' fraction computation and calculate total contexts correctly #93179

Merged

Conversation

jakobbotsch
Copy link
Member

  • The misses fraction was computed as misses / diffed, which can produce odd results (such as > 100% of contexts being misses). Compute it as misses / total.
  • Fix a few places computing total contexts as "successful + failing". After the recent aggregation change the "failing" category no longer includes misses, so this has been updated to "successful + failing + misses".
  • Aggregate "diffed contexts" as we want this in a few places (previously it was approximated as "successful compiles" in the diff, which is usually the same thing, but not exactly)
  • Remove a bunch of unnecessary int(x) calls after recent changes to do aggregation in superpmi.py

…correctly

* The misses fraction was computed as misses / diffed, which can produce
  odd results (such as > 100% of contexts being misses). Compute it as
  misses / total.
* Fix a few places computing total contexts as "successful + failing".
  After the recent aggregation change the "failing" category no longer
  includes misses, so this has been updated to "successful + failing +
  misses".
* Aggregate "diffed contexts" as we want this in a few places
  (previously it was approximated as "successful compiles" in the diff,
  which is usually the same thing, but not exactly)
* Remove a bunch of unnecessary int(x) calls after recent changes to do
  aggregation in superpmi.py
@ghost ghost assigned jakobbotsch Oct 7, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 7, 2023
@ghost
Copy link

ghost commented Oct 7, 2023

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

Issue Details
  • The misses fraction was computed as misses / diffed, which can produce odd results (such as > 100% of contexts being misses). Compute it as misses / total.
  • Fix a few places computing total contexts as "successful + failing". After the recent aggregation change the "failing" category no longer includes misses, so this has been updated to "successful + failing + misses".
  • Aggregate "diffed contexts" as we want this in a few places (previously it was approximated as "successful compiles" in the diff, which is usually the same thing, but not exactly)
  • Remove a bunch of unnecessary int(x) calls after recent changes to do aggregation in superpmi.py
Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-CodeGen-coreclr

Milestone: -

@jakobbotsch
Copy link
Member Author

E.g. for @EgorBo's PR at #93176
Before:
image

After:
image

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib PTAL @BruceForstall

@jakobbotsch jakobbotsch merged commit 6296aac into dotnet:main Oct 9, 2023
110 checks passed
@jakobbotsch jakobbotsch deleted the fix-superpmi-misses-percentage branch October 9, 2023 17:52
@ghost ghost locked as resolved and limited conversation to collaborators Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

2 participants