[mono] Componentize hot reload #52866
Merged
Conversation
This file contains hidden or 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
|
Tagging subscribers to this area: @CoffeeFlux Issue DetailsMove all hot reload code to the hot_reload component. Should be no functional change. Although I did take the opportunity to remove one of the EE callbacks that turned out to be unnecessary.
|
lambdageek
force-pushed
the
componentize-hot-reload
branch
from
06b10a6 to
f363a23
Compare
lambdageek
force-pushed
the
componentize-hot-reload
branch
from
af90179 to
e864039
Compare
lambdageek
requested review from
BrzVlad,
CoffeeFlux,
SamMonoRT,
imhameed,
marek-safar,
thaystg and
vargaz
as code owners
lambdageek
changed the title
lambdageek
force-pushed
the
componentize-hot-reload
branch
2 times, most recently
from
8a2147c to
4b1dd43
Compare
This was referenced Jun 9, 2021
lateralusX
approved these changes
Jun 11, 2021
Member
lateralusX
left a comment
lateralusX
left a comment
There was a problem hiding this comment.
Looked primarily on the component implementation, not that deeply into the bulk of moved code. Couple of smaller things, otherwise, LGTM.
For the fast path data, we will leave a struct in the runtime with the values and pass its address to the component to fill in.
They're unused now
Use separate baseline and delta structs for tracking updates
Move the delta_image and method_table_update data into BaselineInfo and DeltaInfo. Keep a single `MonoImage:has_updates` flag to switch over to the slow path through the component.
Also setup the BaselineInfo and DeltaInfo earlier before the first string heap lookup
instead of hotreload-delta-gen CLI tool
if the hot reload component is stubbed out, return empty capabilities. This is a step toward removing FEATURE_METADATA_UPDATE and ENABLE_METADATA_UPDATE defines and solely using component infrastrucutre for controlling behavior
lambdageek
force-pushed
the
componentize-hot-reload
branch
from
b5dd6de to
a1a11b5
Compare
… var isn't set. Match CoreCLR behavior. Also makes our testsuite actually run the tests (we check for capabilities before running setting DOTNET_MODIFIABLE_ASSEMBLIES using the remote executor)
lambdageek
force-pushed
the
componentize-hot-reload
branch
from
9b9dfc0 to
1b90e05
Compare
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Move all hot reload code to the hot_reload component.
There are some functional changes:
delta_image,method_table_updatefields) moved formMonoImagetoBaselineInfoandDeltaInfostructs in the component. So the locking is different now.metadata_update_initEE callback was unused, so it's been removed.metadata_update_publishedruntime callback is always defined, and its work is guarded by a runtime check of thehas_updatesflag.GetApplyUpdateCapabilitiesmethod now calls an icall to find out from the hot reload component if the runtime was started in a way that supports hot reload and returns capabilities appropriately.There are also some changes to the console sample: