Change inliner estimate code size heuristic #20904
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vijaysun-omr
force-pushed
the
estimate-changes1
branch
6 times, most recently
from
1e3bb08 to
97abe2e
Compare
|
Performance testing has shown no harmful effects of this change in other cases, and it does clearly help the test case that was estimating an order of magnitude more than it inlined. So I am taking the WIP off. |
vijaysun-omr
changed the title
mpirvu
approved these changes
Jan 17, 2025
| if (inlineAnyway && !calleeHasNonColdCalls) | ||
| { | ||
| _optimisticSize = origOptimisticSize; | ||
| // This resetting is probably needed n this path since we are inlining despite exceeding some condition/threshold |
Do not reset the _analyzedSize value if there was a large callee or there was estimation failure for some other reason. Keep track of all the code we have analyzed (including code that we estimated and then discarded) so that we do not have an undefined amount of estimating on a given top level callee. Introduce an "allowance" factor that gets applied on the size threshold to separate how much we are allowed to analyze and how much we are allowed to bring in as inlined call graph. This commit should limit the amount of code we are allowed to analyze to be a factor no more than 2x of what we we are allowed to bring in to the compiled method via inlining. Added a new env var TR_AnalyzedAllowanceFactor that allows a user to specify the factor by which we multiply the original estimate size threshold to control how much we can analyze even with backtracking. Added a new env var TR_GraceInliningThreshold that controls how big a callee is allowed to be, in order to be inlined even if the call graph size estimate is exceeded. Misc. cleanups: Fixed an inconsistency in how we reset variables during backtracking Renamed _optimisticSize to _analyzedSize since it is less confusing Fixed a typo in a variable name Fixed some white spaces Signed-off-by: Vijay Sundaresan <[email protected]>
vijaysun-omr
force-pushed
the
estimate-changes1
branch
from
97abe2e to
2a2ccc3
Compare
I have added the description, thanks |
|
jenkins test sanity all jdk21 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the inlining optimization, there is a phase named estimate code size, that essentially collects the methods in the call graph of the top level callee that ought to be inlined. This estimation has a size threshold that limits the size of the call graph that will be analyzed, in order to prevent the estimation from taking too much cpu and memory. As per the original implementation, if the estimation looked at a call graph that has bytecodes that add up to more than the size threshold, this would cause the top level callee to not be inlined.
However, there was a "backtracking" mechanism added subsequently (still many years ago), that allowed us to skip over some very large parts of the call graph after estimating it, i.e. skip that part of the call graph from what gets considered for inlining. This allows the estimation to consider some other parts of the call graph that normally won't have been considered, and subsequently cause those other parts of the call graph to potentially get inlined.
This backtracking mechanism during estimation can cause a lot more (in one case we were running, it was an order of magnitude more) code to be estimated, but only a fraction of that code gets inlined in the end. In this PR, we are aiming to cap the amount of code that can be estimated, without hampering how much gets inlined in the end. So, this PR should save on the amount of cpu and memory taken by the estimation logic in some cases with very large call graphs that were also backtracking a lot.
Added a new env var
TR_AnalyzedAllowanceFactorthat allows a user to specify the factor by which we multiply the originalestimate size threshold to control how much we can analyze even with backtracking.
Added a new env var
TR_GraceInliningThresholdthat controls how big a callee is allowed to be, in order to be inlined even if the call graph size estimate exceeds the threshold.