-
Notifications
You must be signed in to change notification settings - Fork 5.2k
JIT: slightly increase the inline budget #118641
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
The inline budget is set by the root method being compiled. If we inline a large method into a small one, we sometimes run out of budget trying to inline the large method's callees. Increase the budget from 20 to 22 to give us a bit more breathing room. Fixes some regressions from dotnet#114996 / dotnet#116054
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 increases the JIT inline budget from 20 to 22 to address performance regressions introduced in previous changes. The inline budget controls how much estimated compile time increase is allowed through method inlining.
- Increases the DEFAULT_INLINE_BUDGET constant from 20 to 22
- Addresses scenarios where inlining large methods into small ones exhausts the budget before the large method's callees can be inlined
- Fixes regressions from previous JIT optimizations
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@EgorBo PTAL Locally on my box and |
Might it be worth special casing this particular scenario, especially for cases like forwarding methods which are common for public entry points? One could imagine that if we have a simple forwarding call or a trivial method that calls a big method then we adjust the budget to the larger. -- I'm sure there's some tuning that could be done to find a good balance on when/why we override the base budget |
Yeah I think we need to revisit this whole area. Another thing we have in mind is to prioritize inlines instead of inlining in IL order and "depth first" like we do now. Our current approach often leaves simple top-level inlines on the table when the budget is reached. Unfortunately this is not a simple change. |
👍. Particularly if we could do something like "below always inline threshold" first, then some mix of AggressiveInlining weighted against IL size, I think we'd see some improvements in the larger edge cases |
|
@EgorBot --arm --intel --filter |
|
@AndyAyersMS to be fair, I'm not sure it's going to work on an already merged PR, I think you actually need |
|
Ah, thanks... in these results I assume PR is main before this PR? |
Yeah, it is something I need to fix, or at least rename to "commit" vs "previous". So yeah, in the results "PR" is actually showing changes before your PR. |
|
Strange bounce back behavior from this change. dotnet/perf-autofiling-issues#61217 |
The inline budget is set by the root method being compiled. If we inline a large method into a small one, we sometimes run out of budget trying to inline the large method's callees.
Increase the budget from 20 to 22 to give us a bit more breathing room.
Fixes some regressions from #114996 / #116054