-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Microbenchmarks where inliner runs out of budget with TieredPGO #85531
Comments
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsSee #84264 for context A number of microbenchmarks—particularly those under
Suggest that we review RWC/SPMI to see how often this thing comes up outside of microbenchmarks. Possible fixes
|
cc @EgorBo |
While looking at dumps for #85349 I've noticed that a lot of autoproperties weren't inlined in System.Text.Json, would it make sense to always inline really small methods like autoproperties and maybe not count them into the inliner budget? |
Can you give a more specific example? Really small methods should already be bypassing budget checks. |
Here, note all the |
These look small in C#, but can you verify they're small in IL? |
Well they're autoproperties, won't those be always compiled as |
I don't know about always, but it looks like these are indeed small:
Now, do you know for sure why these didn't get inlined? |
I guess when we run out of budget (or too many locals?) we give up even on "below ALWAYS_INLINE" stuff |
I haven't checked thoroughly, but the change from the PR made the assignments to delegate typed properties take less of the budget, so that's why I assume it's more budget being available that let it inline more. |
I think it's more likely that we hit the local var limit, and since your change creates fewer local vars, we can inline more. The only way to know for sure is to look at the jit dump or at least the inline summary tree. |
Call graph for
|
Tested the 1P web service we usually test: Default mode: 40 "exceeds budget" decisions, among them in BCL:
(these are root methods where some of their inlinees refused to inlined due to the budge thing) PGO=1: 2350 "exceeds budget" decisions or ~60X more than in the no-PGO mode. I tried to play with various knobs, e.g. limitted the max IL size for PGO mode (it helped but still a lot), tried to play with various benefit multipliers trying to add additional friction for inlining in big methods (or of big inlinees) - didn't help. The only simple thing we can do is just to bump the budget now, proper changes definitely require a lot of re-thinking I changed Tested a couple of apps and didn't notice startup regressions from this change. UPD: I probably overdramatized the numbers, once we hit an "exceed budget" problem we start label all callees with it in the current root, even those we would never inline anyway. So the real number of inlinees we should have inlined but we didn't is way lower. cc @AndyAyersMS |
|
One thing I noticed that methods with |
Moving to 9.0 as it's probably too late to land invasive changes for this |
Moving to 10.0 since any changes here potentially have too risky impact |
See #84264 for context
A number of microbenchmarks—particularly those under
System.Text.Json
—run out of budget when run with TieredPGO.Suggest that we review RWC/SPMI to see how often this thing comes up outside of microbenchmarks.
Possible fixes
AggressiveInlining
inSystem.Text.Json
(if these cases are all json related)The text was updated successfully, but these errors were encountered: