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

JIT: prefer edge counts; keep class profiles even if tossing counts #85406

Merged
merged 1 commit into from
Apr 27, 2023

Conversation

AndyAyersMS
Copy link
Member

If there are both edge and block counts for a method, prefer to use the edge counts (block counts are no longer the default, so are more likely to be stale).

Sometimes we decide not to use count data because of correlation or solver issues. When this happens, keep the class profile data viable and let the code that deals with class profiles handle the possibility of stale or mismatched data.

Addresses some aspects of #84446, though it's still not clear why we see static profiles with both block and edge counts.

If there are both edge and block counts for a method, prefer to use the
edge counts (block counts are no longer the default, so are more likely
to be stale).

Sometimes we decide not to use count data because of correlation or
solver issues. When this happens, keep the class profile data viable
and let the code that deals with class profiles handle the possibility
of stale or mismatched data.

Addresses some aspects of dotnet#84446, though it's still not clear why we see
static profiles with both block and edge counts.
@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 Apr 26, 2023
@ghost ghost assigned AndyAyersMS Apr 26, 2023
@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

@ghost
Copy link

ghost commented Apr 26, 2023

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

Issue Details

If there are both edge and block counts for a method, prefer to use the edge counts (block counts are no longer the default, so are more likely to be stale).

Sometimes we decide not to use count data because of correlation or solver issues. When this happens, keep the class profile data viable and let the code that deals with class profiles handle the possibility of stale or mismatched data.

Addresses some aspects of #84446, though it's still not clear why we see static profiles with both block and edge counts.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS AndyAyersMS requested a review from EgorBo April 26, 2023 17:10
}
else if (haveEdgeCounts)
else if (haveBlockCounts)
Copy link
Contributor

@IDisposable IDisposable Apr 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be safer to also handle the haveBlockCounts path if the dataIsGood returned from fgIncorporateEdgeCounts() was false? That way we would prefer the edge counts, but if they were not incorporable, then we would try using the block counts if we have them...

bool dataIsGood = false;

if (haveEdgeCounts)
{
    dataIsGood = fgIncorporateEdgeCounts();
}
if (!dataIsGood && haveBlockCounts)
{
    dataIsGood = fgIncorporateBlockCounts();
}

or maybe even this?

bool dataIsGood = (haveEdgeCounts && fgIncorporateEdgeCounts()) || 
                  (haveBlockCounts && fgIncorporateBlockCounts());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a reasonable suggestion, but I really don't expect to see block counts anymore, and especially do not expect to see both block and edge counts for the same method -- #80481 enabled edge counts for all cases.

As noted in #84446, for some unknown reason the StandardOptimization.mibc file that comes from data collected by our internal optimization process (which merges profile data from multiple runs of things) had some cases where a method hand both block and edge counts. We are in the process of updating this profile data (see eg #85193); I should check if that's still the case.

@AndyAyersMS
Copy link
Member Author

OSX timeout issues are known.

@AndyAyersMS AndyAyersMS merged commit e029183 into dotnet:main Apr 27, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 27, 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.

3 participants