-
Notifications
You must be signed in to change notification settings - Fork 5.1k
JIT: constrain max IL size for OSR inlining #117816
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
Conversation
If we inline too aggressively at intermediate tiers, we can lose profile data that would be beneficial at the final tier. Extend the recent fixes made for limiting max IL size at Tier1+Instr to include OSR as well. This one is a arguably bit more delicate as OSR may well be the final tier, but there is no way to know that. We are just returning to the .NET 9 behavior here. Fixes dotnet#117717. May also fix some of the other regressions that have not yet been analyzed in depth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR constrains the maximum IL size for OSR (On-Stack Replacement) inlining to prevent overly aggressive inlining that could result in loss of valuable profile data for final tier compilation. The change extends existing Tier1+Instrumented compilation limits to also apply to OSR scenarios, reverting to .NET 9 behavior.
Key changes:
- Extends IL size constraints from Tier1+Instrumented scenarios to also include OSR compilation
- Updates logging messages to reflect the new OSR consideration
- Adds explicit logging for both constrained and boosted scenarios
@EgorBo PTAL |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
SPMI won't give us the whole picture here, but clearly we're doing quite a bit less inlining in OSR methods. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM too
If we inline too aggressively at intermediate tiers, we can lose profile data that would be beneficial at the final tier.
Extend the recent fixes made for limiting max IL size at Tier1+Instr to include OSR as well. This one is a arguably bit more delicate as OSR may well be the final tier, but there is no way to know that.
We are just returning to the .NET 9 behavior here.
Fixes #117717. May also fix some of the other regressions that have not yet been analyzed in depth.