-
Notifications
You must be signed in to change notification settings - Fork 5k
Add runtime API to load Hot Reload deltas #45689
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
Comments
cc @tommcdon @davidwrighton @tmat @LyalinDotCom very very alpha draft API proposal. (Going a bit beyond what's in #44806 at the moment: the prototype assumes single-assembly updates at the moment) |
My thoughts and questions:
|
I'm not sure why we need to enable hot reload at all while the app is running. Shouldn't it rather be something that gets enabled when the app is starting? Agree with the other points. It should be up to the caller of these APIs to validate that the changes can be applied and not pass in deltas that can't. The validation needs to be aware of what edits are ok for specific versions of specific runtimes. These APIs should in turn always succeed unless a bug in the runtime is hit. Today EnC can't recover if the runtime is not able to apply an edit. All bets are off at this point - the app may crash, corrupt data, etc. We don't expect the runtime to ever fail to apply delta (unless a bug is hit). |
Thanks Jan!
I was thinking this might be an all-or-nothing once per process-lifetime operation that toggles the runtime into hot reload mode. Would the set of assemblies be useful information? I guess we could continue using more aggressive optimizations in the assemblies that won't be changing. I think in that case it might be better to use some host properties rather than a managed API to indicate the changeable assemblies.
This is not meant for delta producers (ie something running on the developer's machine), but rather for app frameworks (ie code running inside the app that is being developed). Delta producers should be aware of the workload that is being targeted and of the capabilities of the underlying runtime. App frameworks may want to check for hot reload support to register for some workload-specific events to react to deltas (for example by forcing a repaint, or discarding cached state etc). I agree the enum is not really comprehensive. It's probably enough to just have a boolean feature flag
👍
👍
I'm ok with dropping this. The only reliable recourse if a delta application fails is to stop accepting new deltas and advise the developer to restart the app. |
I'd expect that to be part of a different API - the one that defines the events that the debugger (or other initiator of hot reload) triggers after the change is applied. And this API would only apply for type replacements (of types marked "reloadable"). |
That's true.
We know it's not going to be framework code or System.Private.CoreLib, so we could potentially continue to more aggressively optimize those assemblies.
Fair enough. I'm ok with leaving the validation to the dev-side tooling.
Ok.
Yes, the events would be part of a different API. But I think a runtime feature test should be defined so that But I'm ok to define that feature flag in a separate API proposal. I think I will retool this proposal to just expose the single managed entry point, without a success flag or the diagnostics array. Basically: public static void AddAndApplyHotReload (Assembly assm, ReadOnlySpan<byte> dmeta, ReadOnlySpan<byte> dil, ReadOnlySpan<byte> dpdb = null); |
If you tell the runtime to be prepared for arbitrary edits of everything, it will result in severe degredation of the code quality everywhere that is unlikely to be acceptable experience. Note that it is how debuggers work today: EnC is not enabled for everything, it is only enabled for modules that the debugger tells the runtime to enable it for. I would like to see this API to be usable for an eventual implementation of standalone Reflection.Emit. Reflection.Emit is essentially append-only assembly editing. |
@gregg-miskelly How does the debugger decide which module to enable EnC for? Is it the same logic as JMC? |
@jkotas It makes sense to distinguish runtime assemblies and assemblies coming from user built projects: disable EnC for the former and enable for the latter. The difference between these two is how the assembly is built - Release/Debug. But that's given by attributes on the assembly, so there doesn't need to be any API. |
I don't think we want to degrade performance for all Debug configuration assemblies. There still needs to be some explicit directive to the runtime to expect a hot reload session, in my opinion. |
Updated the proposal to only focus on the "Apply delta" function, and only one assembly at a time. Question: Is using a |
The VS debugger will check the flags on System.Diagnostics.DebuggableAttribute. If 0x04 is set, then we try to enable EnC. |
Do you mean per assembly though or for all debug assemblies loaded to the app? |
Good point - an assembly can be loaded multiple times and we definitely want to update all the loaded instances of the assembly and also apply the changes to all instances loaded in future. The component that calls Hot Reload APIs needs to apply the changes to all instances of the assembly that are currently loaded and listen to module load events for the process. When the same assembly is loaded again it needs to apply all deltas that have been applied before to this new instance. VS debugger currently handles this. |
This should be per assembly. I do not think that we want to couple the policy for applicability of deltas with debuggability. I expect that this feature will be useful for number of other diagnostic (e.g. better profiler ReJIT) and non-diagnostic scenarios. We may consider to have an extra flags that describe the edits that you plan to use with the given assembly, for example: AddTypes, AddMethods, AddInstanceFields, AddStaticFields, ModifyMethodBodies. The runtime can then avoid the overhead associated with being prepared for arbitrary deltas.
I think so.
Agree. This is not a problem that the runtime part should worry about.
I think this should live under
We do not need convenience overload that omits the PDBs. This is niche API. People using this API should not have a problem with passing an extra null argument if necessary. |
Wrt. how the hot reload gets enables: The right mechanism for the inner dev loop scenario may be forced by the end-to-end workflow. Do we have that figured out and written down somewhere? |
I agree that coupling it on the runtime level isn't necessary and shouldn't be done to support more scenarios. |
@LyalinDotCom is on point for what the end-to-end user experience is here. Dmitry? |
I think we should discuss the possibility of making this API out-of-proc via the diagnostic server IPC. It is available now on both coreclr and mono. The controller/project watcher code wouldn't have to be injected into the app. It may make it easier for VS to implement this for CTRL-F5 because it could leverage the ENC project rebuilding/delta generation it already has and send the edits over to the target app process. Even @SteveSandersonMS's WebAssembly prototype/demo puts the project watcher/delta builder in separate process and sends the edits to the WebAssembly and calls the inproc mono API. Relying on the diagnostic server connection may have issues I haven't thought about for some end-to-end scenarios. It make may also take a little more work to implement than the inproc API. There would still be a need for an inproc API to for app model frameworks to registered for code change callbacks to flush any caches/refresh UI. |
This API proposal wasn't meant to say anything about project watching, or generating deltas. It's just about applying deltas to the runtime. Where they come from is out of scope. The design I had in mind is somehting like:
The thinking was that it's easier to write a hot reload listener in C# than in C. But since the diagnostic server already exists, I suppose that's moot.
Certainly project watching / delta generation have to be out of proc. For Xamarin or WebAssembly the source code and VS are not on the same device as the running app.
I think so too. What I am proposing is just the API for applying deltas, not how they're transported from the development environment to the running app.
One scenario where this won't work is WebAssembly. (AFAIK we haven't committed to implementing the changes necessary to make the diagnostic server single-threaded / JS eventloop callback based and to use WebSocket for transport). I think diagnostic server IPC will probably work for the Xamarin scenarios if we use the reverse connection mode (I'm not sure if that's the right term) to start the diagnostic server session: the mobile device would connect back to the developer machine. |
+1 on keeping the connection/transport concepts out of scope here. They will have to be different for each application framework and deployment scenario. Only the IDEs/frameworks can bake in assumptions about how this has to work in their specific scenarios. |
I would like to purpose an update to the hot reload API to allow an atomic update of multiple assemblies deltas. This is for multi-assembly edits (i.e. adding a new method in Assembly A and invoking in Assembly B) and allows atomic updates of the same code across all AssemblyLoadContext's. For the later, it assumes that the caller can find all the Assembly instances of the updated code across all the current AssemblyLoadContext's.
|
If everybody is in agreement, I'm going to change this proposal to the above one that Jan suggested in the AssemblyExtensions namespace/class, throws instead of returns bool, and is for a single assembly at a time. |
Looping in @davidwrighton, @terrajobst |
This totally makes sense, and the first implies the second, given that one of the supported developer experiences is a mode where we guarantee not to restart the app without asking for permission. Attempting certain edit types may kill the runtime forcing a restart, so we must know in advance not to attempt those edit types speculatively. However this does put responsibility for knowing which edit types are compatible with which (runtime,version) pairs. Where will that go - inside Roslyn? Have we got a definite acknowledgement from the relevant team that they are taking on responsibility for maintaining this?
What's meant by "only applies for reloadable types"? Does that just mean all types that can ever get edited? I think the app framework needs to know about all edits that are being applied, since any of them may require a UI refresh. |
namespace System.Reflection.Metadata
{
public static partial class AssemblyExtensions
{
public static void ApplyUpdate(Assembly assembly, ReadOnlySpan<byte> metadataDelta, ReadOnlySpan<byte> ilDelta, ReadOnlySpan<byte> pdbDelta = default);
}
} |
Issue: dotnet#45689 Currently Windows only. Fail hot reload API if debugging.
Also add ApplyUpdateSdb that takes byte[] arguments for use with mono/debugger-libs as its awkward to call a ROS<byte> method from the debugger. Contributes to dotnet#45689
) Add hot reload apply changes API: AssemblyExtensions.ApplyUpdate Issue: #45689 Currently Windows only. Fail hot reload API if debugging. Added some simple invalid parameter testing to the new ApplyUpdate API.
Also add ApplyUpdateSdb that takes byte[] arguments for use with mono/debugger-libs as its awkward to call a ROS<byte> method from the debugger. Contributes to dotnet#45689
* [mono] Implement public hot reload API Also add ApplyUpdateSdb that takes byte[] arguments for use with mono/debugger-libs as its awkward to call a ROS<byte> method from the debugger. Contributes to #45689 * Update src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs Co-authored-by: Marek Safar <[email protected]> * Don't allow changes to System.Private.CoreLib * formatting * more formatting * Add a TODO to remove ApplyUpdateSdb * update DeltaHelper * don't use ApplyUpdateSdb in DeltaHelper * throw InvalidOpertionException for corlib changes with hot reload disabled * Run the test on mono on all platoforms; CoreCLR windows only * Use resource string * Address review feedback Co-authored-by: Marek Safar <[email protected]>
Linux/MacOS enabled with PR #48497 |
@lambdageek are we ready to close this issue? I think we have the API implemented on mono (all supported OSs) and coreclr Windows, Linux and MacOS except arm/arm64. |
Background and Motivation
As part of #44806 and #45629 contributing to the overall inner loop performance theme for .NET6, the runtime needs to expose an API to accept and apply hot reload deltas at runtime from workloads and frameworks that want to enable this capability.
Proposed API
Usage Examples
A framework implementing a code reload injector would use the API to deliver deltas to the runtime:
Alternative Designs
An atomic update of multiple assemblies deltas. This is for multi-assembly edits (i.e. adding a new method in Assembly A and invoking in Assembly B) and allows atomic updates of the same code across all
AssemblyLoadContext
s. For the later, it assumes that the caller can find all theAssembly
instances of the updated code across all the currentAssemblyLoadContext
s.If we want to support a transactional model where deltas that affect multiple assemblies are applied simultaneously, a more complex design may be needed.
Instead of a managed API, we could tightly couple the API to an existing side-channel such as the diagnostic server, or the debugger. That would mean that workloads and frameworks wishing to provide hot reload capabilities would need to use a communication channel provided by those services to inject changes.
Instead of a managed API, we could expose a native hosting API function to inject changes. That would mean that workloads and frameworks wishing to provide hot reload capabilities would need to implement at least part of their functionality using native code.
Related API changes, not part of this proposal
There will be separate API proposals for additional aspects of hot reload.
System.Runtime.CompilerServices.RuntimeFeature
properties: Give framework authors a way to include hot reload support, but be able to link it out if the framework is running on a runtime without the capability. Potentially also a property to determine if on-stack replacement is available, since it changes the semantics of delta application.Risks
Unknown at this time.
The text was updated successfully, but these errors were encountered: