Do not check iTable entries after lastITable cache test at warm #22831
Conversation
|
@0xdaryl Could you please review/merge this PR? Thanks FYI: @a7ehuo @BradleyWood @vijaysun-omr This change keeps the improvements in DaCapo-pmd and avoids the regressions in Liberty benchmarks. |
No. Liberty apps have a flat profile where very few methods (if any) reach hot/scorching. Performance is dictated by what happens in warm bodies. |
| generateX86MemoryReference(scratchReg, fej9->getOffsetOfInterfaceClassFromITableField(), cg()), | ||
| interfaceClassReg, cg()); | ||
| } | ||
| static bool enableITableIterationsAfterLastITableCacheCheckAtWarm = feGetEnv("TR_EnableITableIterationsAfterLastITableCacheCheckAtWarm") != NULL; |
There was a problem hiding this comment.
Is there a reason why this "enable" option is an env var and the "disable" option is an Option? That is, why not make them both Options?
There was a problem hiding this comment.
There is no strong reason for the env var. It wasn't at all present in the first implementation of this solution. Then, I wanted to cover all basis by offering the user a way out and realized I have to open another omr PR which needs to propagate to openj9. The env var was a compromise to speed this PR up (FYI: the iTable code increases the JITServer CPU consumption a lot due to the many messages it generates).
I will create an omr PR.
There was a problem hiding this comment.
Dependency created in eclipse-omr/omr#8009
|
Is this not done on Power ? @zl-wang |
this was done already on Power, but we had done it in a better way in my opinion (done it in PicBuilder naturally without taking up extra codeCache). |
mpirvu
force-pushed
the
jitserver-cpuimprov
branch
from
757b3b7 to
8cbc7a9
Compare
mpirvu
added
the
depends:omr
|
OMR with the option change has promoted. Please fix your commit and PR title--it is a bit long. Perhaps just: "Do not check iTable entries after lastITable cache test" |
PR eclipse-openj9#22216 introduced a change in the code generator for x86 to check a few entries from the iTable if the lastITable cache test fails. While this change improves the score of some benchmarks (like pmd from DaCapo) it may regress others (Daytrader8 1%, AcmeAirEE8 0.8%, Liberty startup 2%). This commit allows the iTable entries check only for compilations at `hot` or above opt levels. To allow the iTable entries check at opt levels lower than `hot` the user needs to use the following Xjit/Xaot command line option: `enableITableIterationsAfterLastITableCacheCheckAtWarm` To disable the iTable entries check at all opt levels, the user needs to use the following Xjit/Xaot option: `disableITableIterationsAfterLastITableCacheCheck` Depends on: eclipse-omr/omr#8009 Signed-off-by: Marius Pirvu <[email protected]>
mpirvu
changed the title
mpirvu
force-pushed
the
jitserver-cpuimprov
branch
from
8cbc7a9 to
afaad94
Compare
|
I reduced the length of the PR/commit title. |
|
Jenkins test sanity xlinux,win,zlinux jdk21 |
| static bool disableITableIterationsAfterLastITableCacheCheck = feGetEnv("TR_DisableITableIterationsAfterLastITableCacheCheck") != NULL; | ||
|
|
||
| if (disableITableIterationsAfterLastITableCacheCheck) | ||
| if (comp->getOption(TR_DisableITableIterationsAfterLastITableCacheCheck)) |
There was a problem hiding this comment.
My apologies, I should have asked this earlier. Is throttling this optimization for the warm opt level applicable to Z as well?
No, it does not apply on Z. Experiments need to be performed on that platform before taking a decision. |
5 participants
PR #22216 introduced a change in the code generator for x86 to check a few entries from the iTable if the lastITable cache test fails. While this change improves the score of some benchmarks (like pmd from DaCapo) it may regress others (Daytrader8 1%, AcmeAirEE8 0.8%, Liberty startup 2%).
This commit allows the iTable entries check only for compilations at
hotor above opt levels.To allow the iTable entries check at opt levels lower than
hotthe user needs to use the following Xjit/Xaot command line option:enableITableIterationsAfterLastITableCacheCheckAtWarmTo disable the iTable entries check at all opt levels, the user needs to use the following Xjit/Xaot option:
disableITableIterationsAfterLastITableCacheCheckDepends on: eclipse-omr/omr#8009