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: profile updates for finally optimizations #49139

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

AndyAyersMS
Copy link
Member

Update impImportLeave to propagate IBC counts to new blocks.

Fix profile weights during callfinally chain merging.

Scale profile weights for cloned finallys. Choose the continuation path based
on profile weight. Make sure to keep handler entry hot.

Partial fix for #48925.

Update `impImportLeave` to propagate IBC counts to new blocks.

Fix profile weights during callfinally chain merging.

Scale profile weights for cloned finallys. Choose the continuation path based
on profile weight. Make sure to keep handler entry hot.

Partial fix for dotnet#48925.
@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 Mar 4, 2021
@AndyAyersMS
Copy link
Member Author

AndyAyersMS commented Mar 4, 2021

cc @dotnet/jit-contrib

No diffs for non-pgo.

~ 90 methods impacted in SPMI crossgen collection (all in Roslyn methods that have IBC data)

Sample diff. In this method there are 3 exits from the try-finally. The two lexically early ones (BB04 and BB06) have the same continuation; the lexically last exit BB08 has a different continuation. Before the changes in this PR we would reroute flow from BB08 to the clone, but profiling shows that we should prefer the other exit path.

Also note in the before picture the step blocks (BB12-BB16) have no profile counts, and both the original finally BB09 and the cloned finally BB18 have the same count, and the cloned finally is reached via BB08 which has zero profile count.

In the after picture the step blocks have IBC counts (from the change to impImportLeave), the cloned finally is reached via BB04/BB06, and the original finally has no IBC count (it should have count zero, but because of the "handler entry must be hot" logic it is given a nonzero non-IBC weight).

BeforeAfter

@AndyAyersMS
Copy link
Member Author

Net diff is slightly positive...

Total bytes of delta: -519 (-0.57% of base)
    diff is an improvement.
...
91 total files with Code Size differences (59 improved, 32 regressed), 31 unchanged.

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM

BasicBlock* const firstCloneBlock = blockMap[firstBlock];
bool retargetedAllCalls = true;
BasicBlock* currentBlock = firstCallFinallyRangeBlock;
BasicBlock::weight_t retargetedWeight = 0;
Copy link
Member

Choose a reason for hiding this comment

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

0 => BB_ZERO_WEIGHT

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup.

@AndyAyersMS
Copy link
Member Author

Another occurrence of #11063

Unhandled exception. System.InvalidOperationException: Collection was modified after the enumerator was instantiated.

@AndyAyersMS AndyAyersMS merged commit b428758 into dotnet:main Mar 5, 2021
@AndyAyersMS AndyAyersMS deleted the FinallyCloningProfileUpdates branch March 5, 2021 04:09
@AndyAyersMS AndyAyersMS mentioned this pull request Mar 5, 2021
54 tasks
@AndyAyersMS AndyAyersMS self-assigned this Mar 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 4, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
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
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants