-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mono hot reload reorg and beginning (disabled) add method support #61853
Merged
lambdageek
merged 20 commits into
dotnet:main
from
lambdageek:hot-reload-add-method-wip-v2
Nov 30, 2021
Merged
Mono hot reload reorg and beginning (disabled) add method support #61853
lambdageek
merged 20 commits into
dotnet:main
from
lambdageek:hot-reload-add-method-wip-v2
Nov 30, 2021
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
Doesn't do anything yet. And needs more sanity checking (roslyn won't give us virtual or abstract methods, but we should check).
Returns the number of rows in a metadata table, taking into account metadata update deltas
The params are added with a subsequent enclog "add param" function.
It "works" in that calling methods appears to work (ie direct token references seem to do the right thing). Primarily this is because the param table additions are not that interesting. All the good stuff is in the method signature (which is just in the blob heap). Presumably anything that actually needs parameter attributes, or anything that uses reflection to look at the parameters, will break.
Allows calling non-public methods, which are now assigned the correct parent
Lambdas that only capture `this` (and not local variables or arguments) compile to a private instance method in the enclosing class. So it is enough to support EnC deltas that add methods.
This is the foundation for a new approach for metadata lookups. Instead of using an immutable model (each lookup traverses every delta to find the most up to date version of each table row), we are going to create a complete updated table for each generation and only do the lookup in the latest exposed generation directly. (This is essentially the CoreCLR model). This commit is just the first foundations: we allocate the tables and copy over the previous generations' rows and zero out any rows that will be inserted. Delta applications and lookups have not been updated yet. As a slight optimization, tables that don't have modified or added rows are not copied from the base image. If a generation modifies or adds rows, from that point forward, each subsequent generation will copy the table. We could be a bit more thrifty with copying, but it will complicate lookups. Also eventually we will try to deallocate the pools for generations that are older than no thread needs anymore. Metadata heaps are still looked up in each delta directly - heap combining does not seem necessary yet.
Implement method name wildcard matching for method descriptions Globbing doesn't work because we don't have g_pattern_match_simple in eglib. But a plain '*' wildcard does work.
leave suppressed columns unchanged
The row index doesn't change in the new scheme, pass it by value
It's only used internally by the update logic
don't compute it from the dmeta image
Instead of storing a list of delta images and then relying on delta_info_lookup to find the DeltaInfo, just store them directly on the baseline info. This changes the cleanup responsibilities a bit. Now we destroy the DeltaInfo when we are iterate through the delta infos when we close the baseline image, instead of when we remove the delta_image_to_info hashtable entry.
Just want to get the cleanups enabled for now
lambdageek
requested review from
imhameed,
marek-safar,
SamMonoRT,
thaystg and
vargaz
as code owners
November 19, 2021 21:39
dotnet-issue-labeler
bot
added
the
area-EnC-mono
Hot Reload for WebAssembly, iOS/Android, etc
label
Nov 19, 2021
vargaz
approved these changes
Nov 19, 2021
/azp run runtime-staging |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-staging |
Azure Pipelines successfully started running 1 pipeline(s). |
lambdageek
added
the
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Nov 29, 2021
Goal for today is to run the DebuggerTestSuite by hand to verify that these changes didn't break the debugger |
lambdageek
removed
the
NO-MERGE
The PR is not ready for merge yet (see discussion for detailed reasons)
label
Nov 29, 2021
This was referenced Nov 30, 2021
ghost
locked as resolved and limited conversation to collaborators
Dec 30, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR does two things:
Details of the delta changes.
DeltaInfo
(representing a summary of a particular update to a single assembly) to contain a collection ofmutants
- the fully modified tables with all the newly added rows and all the new modifications. So lookups are fast now - we just go to the latest generation that is visible to a particular thread and look at its mutants tables. The downside is increased memory use. So we allocate the mutants from memory pools owned by eachDeltaInfo
. Right now we never dealloc the pools, but in the future we could. We already iterate over all the threads during our stop-the-world phase - we can record the earliest generation still needed by every thread and delete the pools for all earlier copies. In practice this means we only keep 3 sets of tables: the mmaped baseline tables, the newest mutants, and the mutants from the prior generation for any still-executing methods.BaselineInfo
, we now store a list ofDeltaInfo
structs which eachDeltaInfo
pointing to a delta image. This means that we can avoid repeated hash table lookups to map from a delta image to itsDeltaInfo
every time the runtime queries us for a table row or for a heap element.