Skip to content

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

Closed
lambdageek opened this issue Dec 7, 2020 · 51 comments
Closed

Add runtime API to load Hot Reload deltas #45689

lambdageek opened this issue Dec 7, 2020 · 51 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@lambdageek
Copy link
Member

lambdageek commented Dec 7, 2020

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

namespace System.Reflection.Metadata
{
    public static partial class AssemblyExtensions
    {
        /// <summary>
        /// Hot reload update API
        /// 
        /// Applies an update to the given assembly using the metadata, IL and PDB deltas. Currently executing
        /// methods will continue to use the existing IL. New executions of modified methods will use the new
        /// IL. The supported changes are runtime specific - different .NET runtimes may have different
        /// limitations. The runtime makes no guarantees if the delta includes unsupported changes.
        /// </summary>
        /// <param name="assembly">The assembly to update</param>
        /// <param name="metadataDelta">The metadata changes</param>
        /// <param name="ilDelta">The IL changes</param>
        /// <param name="pdbDelta">The PDB changes. Current not supported on .NET Core</param>
        /// <exception cref="ArgumentNullException">if assembly parameter is null</exception>
        /// <exception cref="...">others TBD</exception>
        public static void ApplyUpdate(Assembly assembly, ReadOnlySpan<byte> metadataDelta, ReadOnlySpan<byte> ilDelta, ReadOnlySpan<byte> pdbDelta = default);
    }
}

Usage Examples

A framework implementing a code reload injector would use the API to deliver deltas to the runtime:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Reflection.Metadata;
using System.Threading;
using System.Threading.Tasks;

public abstract class AbstractHotReloadFramework
{
     // Asynchronously receive sets of related changes from some external source
    public abstract IAsyncEnumerable<IEnumerable<AssemblyLoadContext.HotReloadDelta> ReceiveChangesAsync(CancellationToken ct = default);

    public abstract Task QueueRefresh();
	
    public async Task Loop(CancellationToken ct = default)
    {
        await foreach (var change in ReceiveChangesAsync(ct)) {
	    if (ct.IsCancellationRequested)
		break;
            AssemblyExtensions.ApplyUpdate(change.Assembly, change.MetadataDelta, change.ILDelta, change.PDBDelta);
	    var _t = Task.Run (() => QueueRefresh());
	}
    }
}

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 AssemblyLoadContexts. For the later, it assumes that the caller can find all the Assembly instances of the updated code across all the current AssemblyLoadContexts.

namespace System.Runtime.Loader {
public partial class AssemblyLoadContext {
/// <summary>
/// Contains a hot reload assembly delta
/// </summary>
public struct HotReloadDelta
{
    /// <summary>
    /// The assembly to update
    /// </summary>
    public Assembly Assembly { get; }

    /// <summary>
    /// The metadata changes
    /// </summary>
    public ReadOnlyMemory<byte> MetadataDelta { get; }

    /// <summary>
    /// The IL changes
    /// </summary>
    public ReadOnlyMemory<byte> ILDelta { get; }

    /// <summary>
    /// The PDB changes
    /// </summary>
    public ReadOnlyMemory<byte> PDBDelta { get; }

    public HotReloadDelta(Assembly assembly, ReadOnlyMemory<byte> metadataDelta, Memory<byte> ilDelta, ReadOnlyMemory<byte> pdbDelta)
    {
        Assembly = assembly;
        MetadataDelta = metadataDelta;
        ILDelta = ilDelta;
        PDBDelta = pdbDelta;
    }
}

/// <summary>
/// Hot reload update API
/// 
/// Applies an update to the given assembly using the metadata, IL and PDB. Currently executing
/// methods will continue to use the existing IL. New executions of modified methods will use 
/// the new IL. The supported changes are runtime specific - different .NET runtimes may have 
/// different limitations. The runtime makes no guarantees if the delta has unsupported changes.
/// </summary>
/// <param name="deltas">list of deltas</param>
/// <returns>true for success and false for failure of any update. Some of the updates could have been completed.</returns>
public static bool ApplyHotReloadUpdate(IEnumerable<HotReloadDelta> deltas);
}
}
  • If we want to support a transactional model where deltas that affect multiple assemblies are applied simultaneously, a more complex design may be needed.

        using System;
        using System.Collections.Generic;
        using System.Reflection;
        using System.Runtime.Loader;
    
    namespace System.Runtime.CompilerServices
    {
        public partial class RuntimeHelpers
        {
    	[Flags]
    	public enum HotReloadSupportedFlags : byte
    	{
    	    None = 0,
    	    /// Method body replacement supported
    	    CodeReload = 1,
    	    /// Metadata additions supported
    	    AddedTypes = 2,
    	    /// Metadata changes supported
    	    ModifiedTypes = 4,
    
    	    /// Running methods are updated in place
    	    OnStackReplacement = 128,
    	};
    		
    	///
    	/// Initializes hot reload in the runtime
    	/// Returns a HotReloadInjector object to the first caller.
    	///
    	/// Returns null to every subsequent caller
    	///
    	/// Throws NotSupportedException if this runtime does not support hot reload
    	public static HotReloadInjector EnableHotReload()
    	    => throw new NotImplementedException();
    
    
    	/// Returns a value indicating whether the runtime supports hot reload
    	public static HotReloadSupportedFlags IsHotReloadSupported
    	{
    	    get => throw new NotImplementedException();
    	}
    
        }
    
        public sealed partial class HotReloadInjector
        {
    	/// Begin a hot reload transaction
    	///
    	/// Throws InvalidOperationException if there is already a transaction in progress.
    	///
    	/// If preserveDiagnostics is true, then a call to
    	/// GetCommitFailureDiagnostics following a TryCommitTransaction call
    	/// will return some diagnostic messages about the failure.
    	public HotReloadTransaction BeginHotReload (AssemblyLoadContext alc = default, bool preserveDiagnostics = false)
    	    => throw new NotImplementedException();
    
        }
    
        public sealed partial class HotReloadTransaction : IDisposable
        {
    	/// Add a metadata delta for the given assembly to the transaction.
    	///
    	/// Throws ArgumentException if the assembly is from the wrong ALC.
    	public void AddDelta(Assembly assm, byte[] dmeta, byte[] dil, byte[] dpdb = null)
    	    => throw new NotImplementedException();
    
    	/// Cancel the transaction, discard all the accumulated deltas
    	///
    	/// Throws InvalidOperationException if the transaction is already
    	/// canceled or committed.
    	public void CancelTransaction()
    	    => throw new NotImplementedException();
    
    	/// Apply the accumulated deltas.
    	///
    	/// Returns true on success, or false if the transaction couldn't be
    	/// applied but the runtime is left in a consistent state.
    	///
    	/// If a transaction cannot be applied, and the runtime cannot be left
    	/// in a consistent state, an ExecutionEngineException is thrown.  It
    	/// is recommended that the application shutdown as soon as possible.
    	public bool TryCommitTransaction()
    	    => throw new NotImplementedException();
    
    	/// If TryCommitTransaction() failed and preserveDiagnostics was true when the transaction was started, return a list of diagnostics about the failure.
    	/// Otherwise returns null.
    	public IReadOnlyList<string> GetCommitFailureDiagnostics()
    	    => throw new NotImplementedException();
    
    	~HotReloadTransaction() => Dispose(false);
    
    	/// If the transaction hasn't been committed, it is canceled.
    	public void Dispose()
    	{
    	    Dispose(true);
    	    GC.SuppressFinalize(this);
    	}
    
    	// true if transaction is not committed, canceled, or disposed
    	private bool Active {get; set;}
    
    	private void Dispose(bool disposing)
    	{
    	    if (Active)
    		    CancelTransaction();
    	}
        }
    }
    
  • 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.

  • IL Linker 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.
  • Managed events for delta application. Give framework authors a way to be notified about the changes in a delta (for example a list of stateless change-aware types).
  • A hosting API addition to notify the runtime that a hot reload session will be started and that certain assemblies may receive deltas.

Risks

Unknown at this time.

@lambdageek lambdageek added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.CompilerServices labels Dec 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 7, 2020
@lambdageek
Copy link
Member Author

lambdageek commented Dec 7, 2020

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)

@tommcdon
Copy link
Member

tommcdon commented Dec 7, 2020

Adding @davmason @hoyosjs

@jkotas
Copy link
Member

jkotas commented Dec 7, 2020

My thoughts and questions:

  • What is the set of assemblies that EnableHotReload applies to? I think we need a way to indicate the set of assemblies that the HotReload is enabled for, and this handshake needs to happen before the hot-reloadable assembly gets loaded.

  • There is a lot more granularity for what is supported vs. not supported. For example, CoreCLR has limitations for applying deltas for generics or for special fields like thread statics. I think it is hard to express it via managed API. It may be best for the delta producer to ask about the runtime name and version that it is producing the delta for, and apply the restrictions as appropriate.

  • How do the transactions deal with the changes to the code that is running? I think that arbitrary patching of code in flight is full of reliability corner cases. The transactions may improve it a bit; but the whole thing has to be super complex to actually solve the reliability problem in satisfactory way. I think we should drop the transactions, from the first iterations at least, and just support updating one assembly at a time.

  • The Apply should take Stream or ReadOnlySpan<byte>. byte[] is not the best option performance-wise.

  • What is the level of diagnostic that you expect to get via IReadOnlyList<string>? For EnC in CoreCLR, it has been generally the responsibility of the delta producer that the delta is right. We have not produced very detailed diagnostic. I do not believe that it is even possible for the runtime to produce good actionable diagnostic when the delta is wrong in most cases.

@tmat
Copy link
Member

tmat commented Dec 7, 2020

What is the set of assemblies that EnableHotReload applies to? I think we need a way to indicate the set of assemblies that the HotReload is enabled for, and this handshake needs to happen before the hot-reloadable assembly gets loaded.

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?
I don't think we can choose specific assemblies. We don't know up front which projects is the user gonna make changes in. I think hot reload should be enabled/disabled for all assemblies and the entire lifetime of an app.

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).

@lambdageek
Copy link
Member Author

My thoughts and questions:

Thanks Jan!

  • What is the set of assemblies that EnableHotReload applies to? I think we need a way to indicate the set of assemblies that the HotReload is enabled for, and this handshake needs to happen before the hot-reloadable assembly gets loaded.

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.

  • There is a lot more granularity for what is supported vs. not supported. For example, CoreCLR has limitations for applying deltas for generics or for special fields like thread statics. I think it is hard to express it via managed API. It may be best for the delta producer to ask about the runtime name and version that it is producing the delta for, and apply the restrictions as appropriate.

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 IsHotReloadSupported, perhaps also an on-stack replacement check since that implies a difference in meaning of delta application.

  • How do the transactions deal with the changes to the code that is running? I think that arbitrary patching of code in flight is full of reliability corner cases. The transactions may improve it a bit; but the whole thing has to be super complex to actually solve the reliability problem in satisfactory way. I think we should drop the transactions, from the first iterations at least, and just support updating one assembly at a time.

👍

  • The Apply should take Stream or ReadOnlySpan<byte>. byte[] is not the best option performance-wise.

👍

  • What is the level of diagnostic that you expect to get via IReadOnlyList<string>? For EnC in CoreCLR, it has been generally the responsibility of the delta producer that the delta is right. We have not produced very detailed diagnostic. I do not believe that it is even possible for the runtime to produce good actionable diagnostic when the delta is wrong in most cases.

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.

@tmat
Copy link
Member

tmat commented Dec 7, 2020

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'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").

@lambdageek
Copy link
Member Author

lambdageek commented Dec 7, 2020

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?

That's true.

I don't think we can choose specific assemblies. We don't know up front which projects is the user gonna make changes in. I think hot reload should be enabled/disabled for all assemblies and the entire lifetime of an app.

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.

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.

Fair enough. I'm ok with leaving the validation to the dev-side tooling.

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).

Ok.

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'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").

Yes, the events would be part of a different API. But I think a runtime feature test should be defined so that
frameworks can wrap code related to hot reload in a feature test. Then the IL Linker could prune all that code in non-dev builds of the frameworks.

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);

@jkotas
Copy link
Member

jkotas commented Dec 8, 2020

I don't think we can choose specific assemblies. We don't know up front which projects is the user gonna make changes in.

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.

@tmat
Copy link
Member

tmat commented Dec 8, 2020

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.

@gregg-miskelly How does the debugger decide which module to enable EnC for? Is it the same logic as JMC?

@tmat
Copy link
Member

tmat commented Dec 8, 2020

@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.

@lambdageek
Copy link
Member Author

The difference between these two is how the assembly is built - Release/Debug.

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.

@lambdageek
Copy link
Member Author

Updated the proposal to only focus on the "Apply delta" function, and only one assembly at a time.

Question:

Is using a System.Reflection.Assembly object a good way to identify the assembly to be modified? The wire transport will use some other way to identify the assembly - an assembly name probably, or an MVID, maybe both. Should the API just use that directly? Using an Assembly is also problematic if the same assembly is loaded in multiple ALCs - it will be up to the workload to iterate over the ALCs and make multiple API calls.

@gregg-miskelly
Copy link
Contributor

How does the debugger decide which module to enable EnC for?

The VS debugger will check the flags on System.Diagnostics.DebuggableAttribute. If 0x04 is set, then we try to enable EnC.

@tmat
Copy link
Member

tmat commented Dec 8, 2020

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.

Do you mean per assembly though or for all debug assemblies loaded to the app?

@tmat
Copy link
Member

tmat commented Dec 8, 2020

s using a System.Reflection.Assembly object a good way to identify the assembly to be modified?

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.

@jkotas
Copy link
Member

jkotas commented Dec 8, 2020

Do you mean per assembly though or for all debug assemblies loaded to the app?

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.

`System.Reflection.Assembly object a good way to identify the assembly to be modified?

I think so.

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.

Agree. This is not a problem that the runtime part should worry about.

System.Runtime.CompilerServices.RuntimeHelpers

I think this should live under System.Runtime.Loader. Either in AssemblyLoadContext or in a new type.

public static bool ApplyHotReloadUpdate (Assembly assm, ReadOnlySpan dmeta, ReadOnlySpan dil);

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.

@jkotas
Copy link
Member

jkotas commented Dec 8, 2020

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?

@tmat
Copy link
Member

tmat commented Dec 8, 2020

This should be per assembly. I do not think that we want to couple the policy for applicability of deltas with debuggability.

I agree that coupling it on the runtime level isn't necessary and shouldn't be done to support more scenarios.
With the current EnC it's coupled on the debugger level. As @gregg-miskelly pointed out the debugger enables EnC based on System.Diagnostics.DebuggableAttribute. I don't see a reason to change that.

@lambdageek lambdageek changed the title [draft] Developer inner-loop reload API Add runtime API to load Hot Reload deltas Dec 8, 2020
@stephentoub
Copy link
Member

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?

@LyalinDotCom is on point for what the end-to-end user experience is here. Dmitry?

@mikem8361
Copy link
Contributor

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.

@lambdageek
Copy link
Member Author

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.

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:

  [developer machine]                        |             [app machine]
  project watcher / delta generator    <- [tcp/ip] ->   network listener (C#) -> apply delta API -> runtime (C)

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.

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.

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.

Using the diagnostic server to apply a delta is a possibility (and probably one that makes a lot of sense for inner devloop scenarios).

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.

Relying on the diagnostic server connection may have issues I haven't thought about for some end-to-end scenarios.

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.

@SteveSandersonMS
Copy link
Member

+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.

@mikem8361
Copy link
Contributor

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.

/// <summary>
/// Contains a hot reload assembly delta
/// </summary>
public struct HotReloadDelta
{
    /// <summary>
    /// The assembly to update
    /// </summary>
    public Assembly Assembly { get; }

    /// <summary>
    /// The metadata changes
    /// </summary>
    public ReadOnlyMemory<byte> MetadataDelta { get; }

    /// <summary>
    /// The IL changes
    /// </summary>
    public ReadOnlyMemory<byte> ILDelta { get; }

    /// <summary>
    /// The PDB changes
    /// </summary>
    public ReadOnlyMemory<byte> PDBDelta { get; }

    public HotReloadDelta(Assembly assembly, ReadOnlyMemory<byte> metadataDelta, Memory<byte> ilDelta, ReadOnlyMemory<byte> pdbDelta)
    {
        Assembly = assembly;
        MetadataDelta = metadataDelta;
        ILDelta = ilDelta;
        PDBDelta = pdbDelta;
    }
}

/// <summary>
/// Hot reload update API
/// 
/// Applies an update to the given assembly using the metadata, IL and PDB. Currently executing
/// methods will continue to use the existing IL. New executions of modified methods will use 
/// the new IL. The supported changes are runtime specific - different .NET runtimes may have 
/// different limitations. The runtime makes no guarantees if the delta unsupported changes.
/// </summary>
/// <param name="deltas">list of deltas</param>
/// <returns>true for success and false for failure of any update. Some of the updates could have been completed.</returns>
public static bool ApplyHotReloadUpdate(IEnumerable<HotReloadDelta> deltas);

@mikem8361
Copy link
Contributor

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.

@mikem8361 mikem8361 removed the untriaged New issue has not been triaged by the area owner label Feb 9, 2021
@mikem8361 mikem8361 added this to the 6.0.0 milestone Feb 9, 2021
@mikem8361 mikem8361 added api-ready-for-review API is ready for review, it is NOT ready for implementation enhancement Product code improvement that does NOT require public API changes/additions and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 9, 2021
@mikem8361
Copy link
Contributor

Looping in @davidwrighton, @terrajobst

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Feb 16, 2021

Today EnC can't recover if the runtime is not able to apply an edit.
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

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?

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'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").

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.

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Feb 16, 2021
@terrajobst
Copy link
Contributor

terrajobst commented Feb 16, 2021

Video

  • It's not a mainline API, but it's logically similar to the existing TryGetRawMetadata()
  • However, we don't believe this should be an extension method because most people would never need it
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);
    }
}

mikem8361 pushed a commit to mikem8361/runtime that referenced this issue Feb 17, 2021
Issue: dotnet#45689

Currently Windows only.

Fail hot reload API if debugging.
lambdageek added a commit to lambdageek/runtime that referenced this issue Feb 18, 2021
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
mikem8361 pushed a commit that referenced this issue Feb 18, 2021
)

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.
lambdageek added a commit to lambdageek/runtime that referenced this issue Feb 19, 2021
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
ghost pushed a commit that referenced this issue Feb 19, 2021
* [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]>
@mikem8361
Copy link
Contributor

Linux/MacOS enabled with PR #48497

@mikem8361
Copy link
Contributor

@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.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests