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: extend initial profile repair to more cases #84741

Merged
merged 1 commit into from Apr 13, 2023

Conversation

AndyAyersMS
Copy link
Member

In particular, fill in profiles for cases where we have an incomplete dynamic profile because a method rapidly transitions to OSR, and that method has important code that is not observed to execute by Tier0 instrumentation (things that happen after the OSR triggering loop exits) so having zero profiles for the remainder causes significant perf degradation.

Filling in the missing profile data entails:

  • Allowing repair mode to reset exit likelihoods on zero-weight blocks as well as on inconsistent blocks. Since PGO never saw these blocks run, it has no opinion on their exit likelihoods.
  • Adjust loop exit edge likelihoods if we end up capping the cyclic probability in the loop, so that some profile weight will exit the loop and light up any previously zero weight blocks that may follow.

In particular, fill in profiles for cases where we have an incomplete
dynamic profile because a method rapidly transitions to OSR, and
that method has important code that is not observed to execute by Tier0
instrumentation (things that happen after the OSR triggering loop exits)
so having zero profiles for the remainder causes significant perf
degradation.

Filling in the missing profile data entails:
* Allowing repair mode to reset exit likelihoods on zero-weight blocks as
well as on inconsistent blocks. Since PGO never saw these blocks run,
it has no opinion on their exit likelihoods.
* Adjust loop exit edge likelihoods if we end up capping the cyclic
probability in the loop, so that some profile weight will exit the loop
and light up any previously zero weight blocks that may follow.
@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 12, 2023
@ghost ghost assigned AndyAyersMS Apr 12, 2023
@ghost
Copy link

ghost commented Apr 12, 2023

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

Issue Details

In particular, fill in profiles for cases where we have an incomplete dynamic profile because a method rapidly transitions to OSR, and that method has important code that is not observed to execute by Tier0 instrumentation (things that happen after the OSR triggering loop exits) so having zero profiles for the remainder causes significant perf degradation.

Filling in the missing profile data entails:

  • Allowing repair mode to reset exit likelihoods on zero-weight blocks as well as on inconsistent blocks. Since PGO never saw these blocks run, it has no opinion on their exit likelihoods.
  • Adjust loop exit edge likelihoods if we end up capping the cyclic probability in the loop, so that some profile weight will exit the loop and light up any previously zero weight blocks that may follow.
Author: AndyAyersMS
Assignees: AndyAyersMS
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

See #84264 (comment) for an example.

@EgorBo PTAL
cc @dotnet/jit-contrib

Expect some minor diffs but also a few large ones, in particular in benchmarks.run_pgo where we now properly optimize some methods we had been skimping on.

@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2023

<deleted> ah, nvm, my example didn't use PGO

@AndyAyersMS
Copy link
Member Author

<deleted> ah, nvm, my example didn't use PGO

This repair all happens very early before there are any internal blocks. But I would generally expect internal blocks to be able to inherit weights from their surroundings (or if introducing flow, inherit fractions of weights).

At some point I will introduce a repair that happens later in the phase pipeline, but that's a ways off.

@AndyAyersMS
Copy link
Member Author

Formatting failure looks like some kind of CI glitch.

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Apr 13, 2023

Formatting failure looks like some kind of CI glitch.

Looks like this is hitting other PRs too, opened #84780

@AndyAyersMS AndyAyersMS merged commit 20de7f0 into dotnet:main Apr 13, 2023
122 of 125 checks passed
@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2023

@AndyAyersMS I'm not sure if that is my PR but I'm seeing asserts in PGO pipelines like this https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-84772-merge-ca986f3aaa67420399/System.Collections.Tests/1/console.b3c01543.log?helixlogtype=result could those be caused by this PR?

@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Apr 13, 2023

Yes it could very well be -- can you open an issue?

#84798

@dotnet dotnet locked as resolved and limited conversation to collaborators May 14, 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.

None yet

2 participants