-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Hosting API to enable hot reload at startup #47274
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @vitek-karas, @agocke Issue DetailsWhen running without a debugger attached, we need a way to tell the runtime that certain assemblies will be modified with EnC deltas. For Mono we need to do this early enough that we set the right interpreter optimization flags before we execute any managed code from the affected assemblies. The information given to the runtime should probably include the set of assemblies that will be modified (as opposed to informing the runtime that any assembly at all could potentially be modified). There are two approaches we could take:
My loosely held opinion is that an environment variable is better.
|
It assumes that the app in EnC mode is launched by a controller that sets this environment variable for you. Is it a safe assumption to make?
|
Passing additional properties to the runtime through In this sense it would be similar to I guess it depends on the needs in this case:
If we only need to support this in cases where we build the app with hot reload in mind (similar to "Debug" configuration), then using the |
Hmm.. using |
Do we pass along the |
My understanding is that at this time the mobile embedders don't bundle the json files (or otherwise make the content available). And also apparently they're not using I'm back to mildly preferring to use an environment variable. Update: we added work items for XI and XA to make sure they call |
I think so. Hot Reload would most likely need a controller that is responsible for launching the app. (That seems the easiest way to guarantee that the controller has a consistent view of the source code, the managed assemblies and queued up EnC deltas) |
Will we use managed APIs to actually do the hot-reload editing? |
Still TBD - it's what I'm prototyping in Mono, but ultimately we'll have to align on a design with @tommcdon.
I can think of reasons why that won't work right now for Mono's hot reload, but I have a harder time thinking of why this wouldn't work in principle.
Assuming we choose the right execution engine (interp, not JIT or AOT), the information that hot reload needs is: don't do inlining of any methods that could change. Which in practice means don't inline methods from a modifiable assembly.
So a startup hook out to work to tell the runtime what the modifiable assemblies are.
Currently it's all-or-nothing in mono - we turn off all inlining, because there's no flag for modifiable assemblies yet. But it seems reasonable to add such a flag and it shouldn't slow down the interpreter fast path unreasonably.
Setting up a communication channel through a startup hook, may work, but I'm not certain. The "main assembly" on Xamarin.iOS is the Xamarin Objective C runtime. So setting up a communication channel would have to be done without using any managed iOS functionality. I'm not sure if that's an onerous limitation. Probably it's ok. I think XA works similarly. I suppose worst case the managed startup hook could call into a native library for the bulk of the work. A managed startup hook would work on wasm, I think. Mono and all three of those hosts don't have startup hook support right now, so that would need to be done. Xamarin and wasm will require tooling work to bundle the startup hook assemblies into the application bundles (and maybe AOT them for ios). |
Slightly different way to think about it (.. my way 😉):
|
I do like @vitek-karas idea of driving the whole thing via the existing startup hook mechanism. |
Ok, let's table this for now. I'll make a workitem for myself to add startup hook support for Mono, and then take a closer look at using a hook for wasm and xamarin hot reload. Thanks! |
I don't think the set of assemblies is known at startup time. It is known for each assembly just before it loads, but not necessarily at the startup. |
It's the set of assemblies that are built from the sources in the current solution. Isn't it? An over-approximation is fine - we want the set of assemblies that could be changing. |
It gets complicated. The IDE does not currently tell the debugger what assemblies are editable. The current EnC impl in CLR does not need this information at startup. If I am not mistaken, the debugger determines whether EnC should be enabled for an assembly and notifies the CLR as/before the assembly is loaded. |
I don't think Mono needs this at startup either (other than perhaps a global "there will be hot reload in this run" flag). Just before/while the assembly is loaded should be fine for Mono, too. How does the debugger make the determination of which assemblies are editable? Hot reload tooling would need to do something similar @tommcdon |
It checks an attribute on the assembly. The compiler stamps assemblies built in DEBUG configuration with certain attributes. |
DEBUG build is required for EnC to work at C#/VB compiler level. |
As tmat correctly pointed out there is a flag on the assembly. In coreclr we also can set COMPLUS_ForceEnc. |
What about if we by default to enabling hot reload/ENC on debug built modules? The DOTNET_MODIFIABLE_ASSEMBLIES env var can be used to opt-out of this default by either specifying no modules (empty list) or a list of hot reloadable modules regardless of how they are built. We will need to investigate the performance impact of enabling ENC for debug modules over just debug modules. Is there any difference in JIT codegen for ENC modules over debug built modules? /cc: @noahfalk, @AndyAyersMS |
Keying on debug build configuration seems like a pretty good heuristic to me. Also do we need to support scenarios where we don't know the full list of assemblies at startup, for example an application that will load plugins? If so that means these unpredictable assemblies can never be included in any opt-out or opt-in environment variable. Whatever the default behavior is for unlisted assemblies is what all plugins would get.
Last I knew, yes there is a difference and it is controlled by the flag CORJIT_FLAG_DEBUG_EnC passed to the JIT when we compile methods in EnC assemblies. My understanding is that these differences force more regularity on stack layout and exception handling clauses which allows the debugger to do the OSR portion of EnC. If you don't need OSR you don't need to specify this flag. @AndyAyersMS probably knows the detail of the codegen differences better than I. |
@lambdageek Is it acceptable for Mono to always enable editability for any assembly compiled as debug by default? Would it mean that assemblies compiled as debug have to always use interpreter on Mono (for now at least)? If we were to enable all assemblies compiled as debug to be editable by default, 3rd party tools will find creative ways how to use it. For example, I can imagine diagnostic tools that act as profilers will try to use it instead of the current unmanaged profiler APIs that are very inconvenient to use. Are we happy about it, or do we want to lock it down somewhat e.g. making the debug assemblies editable by default only when an environment variable or configuration switch is set? |
I think the optimal situation for Mono is to have some global flag (environment variable or switch) to signal that editability is desired, and after that we can choose the editable assemblies based on the assembly debug attribute. There are two constraints:
|
Then lets define an opt-in env var for debug built assemblies. A binary flag name something like DOTNET_MODIFIABLE_DEBUG_ASSEMBLIES=1. I think an environment variable is better instead of a host property because it makes it a lot easier for dotnet-watcher (or other "controller") to enable it. The DOTNET_MODIFIABLE_ASSEMBLIES env var may also still be useful to specify assemblies that are not debug built. Not sure yet if the work to do this is worth it. @pranavkm (who is working on dotnet-watch) may have some feedback on that. |
DOTNET_ENABLE_MODIFIABLE_DEBUG_ASSEMBLIES might be a better name. And DOTNET_MODIFIABLE_ASSEMBLIES would be useful for testing to always enable the test assembly or assemblies regardless of how they are built. |
Can we have just one env variable that can be set to different values? E.g. |
Ok, how about just the DOTNET_MODIFIABLE_ASSEMBLIES env var with two special case-insensitive keywords "debug" and "all" and they can be used with module paths seperated by ";". Just debug built assemblies: All assemblies (except "system", "dynamic", etc.): Debug built assemblies and a test assembly: I'm not sure we need "all" because we already have |
This is undocumented CoreCLR-specific switch. We want something that is documented and that works the same way on both CoreCLR and Mono. I do not think we would want to add
Are we going to actually use this for anything in near future? I sounds like unnecessary complication to me. |
We should also clarify how |
We may want it for testing to enable just a test assembly regardless of how it is built. Hopefully @pranavkm can tell us if specifying individual modules will be useful for dotnet-watch. |
Are we going to support hot-reload e2e for the release build configuration? If not, we should rather always build the test assemblies for this as debug. |
dotnet-watch is aware of the set of projects / assemblies it's building. It would be fairly easy for us to specify the list of assemblies being built as a value to the env variable if that makes a difference. That said, in the ordinary case specifying |
I don't know if the JIT_FLAG_DEBUG_EnC (set if the module is editable) disables inlining or not. It does prepare for on stack replacement which isn't needed for hot reload but may be needed on Windows for a smooth transition from hot reload edits to debugging/ENC. Unless it has noticeable performance impact and since it is opt-in now, I say the "All" option sets the jit ENC flag just like the COMPlus_ForceENC. To keep things simple and consistent I think the "All" option should do this for Windows, Linux and MacOS, etc. |
It does not.
I think we should just enable the |
Does this env var need to go through an official API review? Should I mark it api-ready-for-review? /cc: @stephentoub |
Today, no. Though we're getting to a point with our environment variables where I do think we should start looking at them with a view to consistency, and potentially adding a tiny more amount of process around them, not to block them but just to help with that consistency. @terrajobst, thoughts? Related: #47283 |
@lambdageek, have you added this env var to mono yet? |
@mikem8361 Nope, got pulled away to work on a couple other things. Hope to return to it later this week. |
Implements #47274 for Mono. 1. `DOTNET_MODIFIABLE_ASSEMBLIES` environment variable must be set to `debug` for hot reload to be enabled (in addition to the interpreter being enabled) 2. Assemblies must be compiled in Debug mode - we check `DebuggableAttribute` for the `DisableOptimizations` bit. If an assembly doesn't have the bit set, it can't be reloaded. 3. In the interpreter - don't try to inline when hot reload is enabled and the caller or callee has the DisableOptimizations bit set. * [mbr] Check DOTNET_MODIFIABLE_ASSEMBLIES for hot reload support If it's set to "debug" (case insensitive) then hot reload is enabled for assemblies compiled in debug mode. Otherwise hot reload is disabled * [mbr] Disable inlining for DebuggerAttribue assemblies with the optimizer disabled flag * [mbr] Update samples * [mbr] Throw InvalidOperationException is hot reload is not supported On a per-assembly basis * [mbr] Stub implementations if !ENABLE_METADATA_UPDATE
This is now done for MonoVM and CoreCLR |
Related to #44806 and #45629.
When running without a debugger attached, we need a way to tell the runtime that certain assemblies will be modified with EnC deltas. For Mono we need to do this early enough that we set the right interpreter optimization flags before we execute any managed code from the affected assemblies.
The information given to the runtime should probably include the set of assemblies that will be modified (as opposed to informing the runtime that any assembly at all could potentially be modified).
The current proposal:
Pass an environment variable
DOTNET_MODIFIABLE_ASSEMBLIES
with the case-insensitive keyword "debug" to enable all debug built assemblies for hot reload. This can be expanded on with other keywords like "all" or a comma separated list of assemblies when there are clear scenarios for the options.Alternative approaches:
MODIFIABLE_ASSEMBLIES
property tocoreclr_initialize
that includes a list of assemblies that the runtime should expect to be modified. The presence of the property would indicate that hot reload support should be enabled. We could pass the property via the.runtimeconfig.dev.json
file without any modifications to the (desktop) host. Mobile and wasm workloads would need to adjust their hosts.DOTNET_MODIFIABLE_ASSEMBLIES
with the same information. On mobile and wasm workloads, the hosts wouldn't need to be adjusted, MonoVM would consult the environment variable itself. On other workloads the host could interpret the environment variable or leave it to the runtimes.The text was updated successfully, but these errors were encountered: