Enable fan-in heuristics at high opt, reduce method size threshold from 50 to 30 for all opt levels #22834
Conversation
Signed-off-by: Younes Manton <[email protected]>
Allow smaller methods to be considered. Signed-off-by: Younes Manton <[email protected]>
|
FYI @vijaysun-omr. I've opened this as a draft for the time being so that I can evaluate some other alternatives. Namely whether enabling fan-in at |
|
Jenkins test sanity all jdk21 |
|
@nbhuiyan I would appreciate your review |
|
fyi @mpirvu mostly for your awareness |
|
Test_openjdk21_j9_sanity.openjdk_aarch64_linux_Personal — Build UNSTABLE Looks like an existing problem. A similar failure (a different test in the same group of tests, same exception backtrace) from a couple of days ago: https://openj9-jenkins.osuosl.org/job/Test_openjdk21_j9_sanity.openjdk_aarch64_linux_Personal/290/ Build_JDK21_x86-64_windows_Personal — Build FAILED Failed due to infra issues before it did anything useful. |
|
Given the nature of the change (cross platform heuristic) and the known/not real issues in the build, I am going to merge this based on all the other platforms passing |
| */ | ||
| if (comp()->getMethodHotness() > warm) | ||
| static bool disableFanInAtHighOpt = feGetEnv("TR_disableFanInAtHighOpt") != NULL; | ||
| if (disableFanInAtHighOpt && comp()->getMethodHotness() > warm) |
There was a problem hiding this comment.
Was || intended instead of && here? I would have expected that the behavior (when TR_disableFanInAtHighOpt is not present) would not change.
There was a problem hiding this comment.
We do want to change the default behaviour and revert to the old behaviour when the env var is set.
The env var isn't there to control the fan-in heuristic in general (that can be done via -Xjit), it's there to quickly revert just this part of the change in case it causes perf regressions or functional issues in the near future. Thanks for double checking it.
There was a problem hiding this comment.
But when the environment variable is not set, disableFanInAtHighOpt == false and the if condition is false.
There was a problem hiding this comment.
Right, that causes control to skip the return and execute the rest of the routine as intended.
Before:
- Methods compiled at
<=warmdon't return here and execute the rest of the routine, and are thereby subject to the fan-in heuristic. - Methods compiled at
>warmreturn here and aren't subjected to fan-in heuristic.
After:
- By default no methods return here, they all execute the rest of the routine and are subjected to the fan-in heuristic.
- If the env var is set, methods compiled at
>warmreturn, are not subject to fan-in; methods<=warmdon't return, are subject to fan-in as before.
There was a problem hiding this comment.
@keithc-ca That is intentional from my understanding. Now at higher opts, unless TR_disableFanInAtHighOpt is set, we do not do an early return from the method adjustFanInSizeInWeighCallSite. The behaviour prior to this change was that at opt levels > warm, adjustFanInSizeInWeighCallSite was disabled. This PR now allows things to proceed at higher opt levels, with the option to disable it at higher opt levels.
There was a problem hiding this comment.
Ok, thanks for the explanations.
4 participants
The commits in this PR enable the JIT compiler's fan-in heuristics at
>warmopt levels and reduce the method size threshold from 50 bytecodes to 30. Methods that are smaller than the threshold are not considered by the fan-in heuristic. By reducing the threshold we allow more methods to be considered by the heuristic, which tries to prevent inlining of methods that are called from a large number of call sites (i.e. that have a high fan-in). This change shows positive effects on certain benchmarks that have small methods that are called from thousands of call sites.The patches also add the
TR_disableFanInAtHighOptenv var which can be set to prevent the fan-in heuristic from being used at>warm.(The method size threshold can already be controlled via the
-Xjit:iprofilerFaninMethodMinSize=option.)