Add background type preloading based on multicorejit #52595
Conversation
gbalykov
force-pushed
the
multicorejit-unification-btp
branch
from
f801eff to
e0d3216
Compare
|
cc @mangod9 |
src/coreclr/vm/multicorejit.cpp
Outdated
|
|
||
| if (m_pMulticoreJitRecorder != NULL) | ||
| { | ||
| m_pMulticoreJitRecorder->RecordMethodJit(pMethod, true, false); |
There was a problem hiding this comment.
I think the dojit parameter should be true here, in the sense we are recording a load of pregenerated method code, but the profile may be played back in a situation where that method is not pregenerated. The closest alternative would be to JIT the method, in effect the profile only records methods whose code should be loaded or jitted. Also, if the player does not try to compile the method, it would not actually load the pregenerated code, so it would only load the types relevant to where the method is, and would not load any other types used by the method. It may also help to parallelize the R2R method code loads.
I suggest the following:
- Remove the
dojitparameters and treat it as true - Remove the
NO_JIT_TAGflag - Remove
RecordMethodLoadsince thedojitparameter is the only differentiating factor - Rename
RecordMethodJitand equivalent toRecordMethodLoadOrJitor similar
src/coreclr/vm/multicorejitimpl.h
Outdated
| @@ -45,7 +46,7 @@ const int MAX_WALKBACK = 128; | |||
|
|
|||
| enum | |||
| { | |||
| MULTICOREJIT_PROFILE_VERSION = 102, | |||
| MULTICOREJIT_PROFILE_VERSION = 103, | |||
There was a problem hiding this comment.
This shouldn't be necessary if the NO_JIT_TAG flag is removed (based on my other comment), if I understood correctly that was the only change to the storage format
src/coreclr/vm/prestub.cpp
Outdated
| { | ||
| if (MulticoreJitManager::IsMethodSupported(this)) | ||
| { | ||
| mcJitManager.RecordMethodLoad(this); // Tell multi-core JIT manager to record method on successful load from R2R |
There was a problem hiding this comment.
In my branch I have moved this code to after a successful SetNativeCode() in the caller such that there would not be duplicate recordings of the method during multi-threaded races
|
With the current changes, I believe the pregenerated methods would not be loaded by MCJ and instead would be jitted. The vast majority of methods would fall into that category, so a lot more would be unnecessarily jitted compared to before. After the fixes above and after incorporating my branch, those methods should be loaded and tiered up appropriately and not jitted up-front. I'm curious to see if that makes any difference to the startup time in your app. |
…back R2R'ed code - Renamed `*wasTier0Jit` to `wasTier0` to include pregenerated code that is loaded, which is considered tier 0 - Added a subset of `FinalizeOptimizationTierForTier0Jit` as `FinalizeOptimizationTierForTier0Load` to set the `wasTier0` flag so that those methods would be call-counted for tiering when MCJ-loaded code is used, and updated the relevant code - Moved recording of loaded method code to after a successful `SetNativeCode()` to prevent duplicate recordings of the same method in multi-threaded races - Enabled `MulticoreJitPrepareCodeConfig` to look up pregenerated code to parallelize that work
gbalykov
force-pushed
the
multicorejit-unification-btp
branch
from
e0d3216 to
2d7eede
Compare
|
@kouvel thanks for taking a look at this! Indeed, invocation of
I've fixed other changes as you suggested. |
|
Thanks! |
ghost
locked as resolved and limited conversation to collaborators
This is a second part of #48326 change, which enables handling of methods loaded from r2r images. Background thread of multicorejit now not only jits methods but also loads methods from R2R images. This allows to load types in background thread.
This is required as part of #45748 change (specifically, #45748 (comment)), goal of which is to enable background type preloading using multicorejit.
On tizen armel, on xamarin calculator app next improvement is achieved:
cc @alpencolt