Add a notification mechanism for EnC and HotReload

[StephaneDelcroix]

Background and Motivation

One of the key work stream for the success of HotReload is a feedback mechanism so frameworks and 3rd party libraries knows that a type was hot reloaded and can decide to replace a live instance, invalidate a cache, refresh a binding, etc...

The proposed API isn't prescriptive, and other feedback mechanisms can be discussed, but it's quite important that the API is part of the BCL and the feature is supported by the runtime, so all frameworks will benefits from it, and it'll work in all HotReload scenarios (debugger and botnet watch).

This proposal was first mentioned in #45689, 2nd bullet of the 'Related API changes' section

Proposed API

namespace System.Reflection.Metadata
{
    [System.AttributeUsage(System.AttributeTargets.Assembly, AllowMultiple = true)]
    public sealed class  ApplyUpdateNotificationAttribute : Attribute
    {
        /// <summary>
        /// HotReload notification API
        ///
        /// Declares which static method to execute when a HotReload delta is applied. The hot reload agent is responsible
        /// for scanning assemblies at load, and register the notifiers.
        ///
        /// notifier signature should look like: internal static void BeforeUpdate(Type type) and/or AfterUpdate(Type type).
        /// </summary>
        /// <param name="type">The Type on which the notifier is defined</param>
        /// <exception cref="ArgumentNullException">type or notifier are null</exception>
        /// <exception cref="...">type or notifier does not exist, or notifier isn't static, or doesn't have the right signature</exception>
        public ApplyUpdateNotificationAttribute (Type type)   
    }
}

Usage Examples

This is how a UI Framework could reload a view when an HotReload change is applied

[assembly:ApplyUpdateNotification(typeof(HotReloadExtensions)]

namespace MyUIFramework
{
    public class View {
        public View Parent {get; set;} // setting the Parent sets the Child on the Parent view

        public View ()
        {
            HotReloadExtensions.RegisterInstance(this);
        }
    }

    internal static class HotReloadExtensions
    {
        // Keep weakRef of created views in a cache
        public static void RegisterInstance(View view) {...}

        // retrieve values from the cache
        static IEnumerable<View> GetViewsOfType(Type type) {...}

        // the notifier, declared by the attribute
        public static void AfterUpdate(Type type)
        {
            foreach (var oldinstance in GetViewsOfType(type)
            {
            }
        }
    }
}

Alternative Designs

1. event: creating an event has the problem of registration, mainly for 3rd party libraries
2. implement this capability at the framework level. repetitive work, and the runtime would still have to implement it's own mechanism to invalide its caches (I'm thinking ReflectionCache, ....)
3. whatever applies the change could also trigger the notification, but that would require injecting some shared code in all platforms supporting EnC/HR

Risks

Unknown at this time

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.

[StephaneDelcroix]

/cc @tommcdon @LyalinDotCom @tmat @joj

[stephentoub]

The runtime is responsible for scanning assemblies at load, and register the notifiers

If it's the runtime (i.e. corelib) that's responsible for notifying, then I think it should just be an event code can subscribe to rather than a dynamic reflection-based mechanism, likely right next to ApplyUpdate, e.g. UpdateApplied. A component can register with the event from a ModuleInitializer if desired.

If we want to use a reflection-based scheme looking for attributes, it doesn't need to be the runtime that does this: it can just be the hot reload component injected into the process.

EnCNotificationAttribute

EnC is an implementation detail on coreclr, e.g. mono uses MBR. We should tie the naming here to the same naming used for the other public surface area, e.g. ApplyUpdate.

Guid notifierId, IEnumerable<Guid> runBefore

Why is this necessary at this level? If there are multiple components that have a relationship, why can't just one of them register and then be responsible for invoking whatever else needs to be invoked in whatever order is important?

that would require injecting some shared code in all platforms supporting EnC/HR

Isn't this already the plan?

[stephentoub]

and the runtime would still have to implement it's own mechanism to invalide its caches

This part still needs to be debated. @jkotas has expressed strong concerns against doing this.

[jkotas]

attributes

Enumerating all types in all assemblies to find items marked with a specific attribute is well know performance anti pattern. What about having assembly-level attribute that specifies the EnC manager(s) in the given assembly? The EnC manager can then just look for the assembly-level attributes to find all eligible participants that is quite cheap.

EDIT: I have misread the proposal. I see that the proposal is assembly level attribute. That's good.

A component can register with the event from a ModuleInitializer if desired.

I like the declarative scheme better than registering upfront. The declarative scheme is more pay-for-play (e.g. wrt linkability) without additional work.

Also, the declarative scheme allows assemblies compiled against netstadard to participate, without re-targeting for .NET 6+. I am not sure whether this is important.

If there are multiple components that have a relationship, why can't just one of them register and then be responsible for invoking whatever else needs to be invoked

How does the lower-level component know whether there is going to be some higher-level component to flush it, or whether it is on its own and it should be responsible for itself? I think there will need to be some sort of ordering applied by the EnC manager.

runtime invalide its caches

I would be fine with CoreLib reflection participating in the updating scheme as long as it is pay-for-play, like the assembly-level attribute that specifies the EnC manager(s) in the given assembly that I have suggested.

Also, do we need an API that returns true when code updates are allowed? This API would return true when the environment variable that enables code updatability is set. I expect that some components will want to switch to a different slower mode of operation to support updateability.

[jkotas]

IEnumerable runBefore

I do not think you can have IEnumerable<Guid> in an attribute.

I think the attribute should just have the local update manager type name. The types of notifications and other details can be inferred from the methods implemented on the update manager.

public static void OnChange(Type oldType, Type newType)

Why does this have oldType and newType? I thought that we are focused on updating types in-place.

[stephentoub]

I would be fine with CoreLib reflection participating in the updating scheme as long as it is pay-for-play, like the assembly-level attribute that specifies the EnC manager(s) in the given assembly that I have suggested.

To make sure I understand, who is doing the clearing/reset here? And your previously expressed concerns about reflection becoming a mutable model are no longer a concern because the assembly is opting in to hot reload via the attribute?

The declarative scheme is more pay-for-play (e.g. wrt linkability) without additional work.

You mean because the linker could be told whether or not to remove the attribute based on build flavor vs needing to be taught if/when it was ok to remove the module initializer? Fair enough.

How does the lower-level component know whether there is going to be some higher-level component to flush it, or whether it is on its own and it should be responsible for itself?

It's a question of scenarios. If it's really the case that you might be using a lower-level component that needs notification and be doing so separately from a higher-level component (that may or may not be used) that also needs notifications and they need to be executed in a particular order, then yeah, that would need to be applied by the invoker. Is that the scenario? To make it concrete, how does that come into play here, e.g. with Xamarin?

the EnC manager

When you say "EnC manager", which component specifically are you referring to? I just want to make sure we're talking about the same thing. Do you mean something in the runtime itself (e.g. code invoked as part of ApplyUpdate), or do you mean the code injected into the process (e.g. the same code that calls ApplyUpdate), or something else?

[jkotas]

To make sure I understand, who is doing the clearing/reset here?

The global update manager (not in CoreLib, comes with SDK) would call local update handler in CoreLib. The local update handler in CoreLib would simply clear the weak handle that points to the reflection cache.

And your previously expressed concerns about reflection becoming a mutable model are no longer a concern because the assembly is opting in to hot reload via the attribute?

I have internalized the fact that all this is best effort, the feature will not work as expected in number of cases and that it is by design.

Is that the scenario? To make it concrete, how does that come into play here, e.g. with Xamarin?

Reflection and Json serializer are example of lower-level / higher-level components with relationship.

EnC manager

I meant the code injected into the process. We need better names for this - what about:

• Update Manager: Global component injected into the process
• Update Handler: Local assembly-specific code that applies the updates

[stephentoub]

The global update manager (not in CoreLib, comes with SDK) would call local update handler in CoreLib

Meaning corelib itself would have an attribute on it like:

[assembly: UpdateAppliedHandler("System.Reflection.Type", "ClearReflectionCaches", "D02B1C93-A6E5-4F07-A45D-2CD02675CF77")] //with whatever attribute name and arguments we decided on

yes?

I have internalized the fact that all this is best effort

Ok 😄

And, yes, there are plenty of things that won't work, automatically. For example, I don't think JsonSerializer will be in a good position to automatically dump all reflection-emitted code it spit out previously, given that it doesn't reference all JsonSerializerOptions instances ever used, and such options objects hold on to reflection-related state. Libraries and app code that may have used JsonSerializer and are holding on to the options bag that references the serialized code will need to (and know to) drop it and create a new one. You could imagine a complicated versioning scheme where every cache is tagged with a version, JsonSerializer updates its notion of valid version on every update, and it only respects caches created with that same version number, or something like that.

Reflection and Json serializer are example of lower-level / higher-level components with relationship.

I presume you mean because, for example, JsonSerializerOptions will need to ditch its s_defaultOptions instance in order to drop any reflection caches it contains? Are all executing threads going to be paused during the invocation of these handlers? If not, I wonder how much ordering really matters, as problems can arise regardless of which cache is cleared first. It's obviously better for the underlying reflection cache to be cleared so that all subsequent accesses to it get up-to-date information, but if JsonSerializer could be executing concurrently with an update being applied, it's going to be operating on stale state until its cache is cleared and executing threads with a reference to the cached data decide to go back to the cache to update.

Regardless, your point is that such orderings may be needed. That suggests we're going to need to document the IDs of anything that anyone might want to rely on for ordering, so that they can include those dependencies.

This example is also one where just ordering the invocations from lower in the assembly reference graph to higher in the graph would suffice. Are there examples where it'd be important for the higher-level component to update first (and where the lower-level component is used with and without the higher-level one such that it needs its own update handler)?

We need better names for this

@tommcdon has been using:

• Hot Reload Agent: the code injected into the process (what you refer to as "update manager")
• Hot Reload Manager: the external launcher component (e.g. dotnet watch) that injects the agent

and we could add to that:

• Hot Reload Update Handler: code invoked by the manager in response to an update being applied

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

One of the key work stream for the success of HotReload is a feedback mechanism so frameworks and 3rd party libraries knows that a type was hot reloaded and can decide to replace a live instance, invalidate a cache, refresh a binding, etc...

The proposed API isn't prescriptive, and other feedback mechanisms can be discussed, but it's quite important that the API is part of the BCL and the feature is supported by the runtime, so all frameworks will benefits from it, and it'll work in all HotReload scenarios (debugger and botnet watch).

This proposal was first mentioned in #45689, 2nd bullet of the 'Related API changes' section

Proposed API

namespace System.Runtime.???
{
[System.AttributeUsage(System.AttributeTargets.Assembly)]
    public sealed class EnCNotificationAttribute : Attribute
    {
        /// <summary>
        /// HotReload notification API
        ///
        /// Declares which static method to execute when a HotReload delta is applied. The runtime is responsible
        /// for scanning assemblies at load, and register the notifiers
        ///
        /// notifier signature should look like: internal static void OnChange(Type oldType, Type newType);
        /// </summary>
        /// <param name="type">The Type on which the notifier is defined</param>
        /// <param name="notifier">The name of the static method to invoke when a delta is applied</param>
        /// <exception cref="ArgumentNullException">type or notifier are null</exception>
        /// <exception cref="...">type or notifier does not exist, or notifier isn't static, or doesn't have the right signature</exception>
        public EnCNotificationAttribute (string type, string notifier)

        /// <summary>
        /// ...
        /// when the runtime loads the containing assembly, it can build a topological graph and execute the notifiers in the right order
        /// </summary>
        /// ...
        /// <param name="notifierId">the Id of the notifier</param>
        /// <param name="runBefore">this notifier should be executed before notifier in runBefore</param>
        /// ...
        /// <exception cref="...">loop detected in the topological map of notifiers</exception>
        public EnCNotificationAttribute (string type, string notifier, Guid notifierId, IEnumerable<Guid> runBefore)
        {
        }
    }
}

Usage Examples

This is how a UI Framework could reload a view when an HotReload change is applied

namespace MyUIFramework
{
    [assembly:EnCNotification("HotReloadExtensions", "OnChange"]

    public class View {
        public View Parent {get; set;} // setting the Parent sets the Child on the Parent view

        public View ()
        {
            HotReloadExtensions.RegisterInstance(this);
        }
    }

    internal static class HotReloadExtensions
    {
        // Keep weakRef of created views in a cache
        public static void RegisterInstance(View view) {...}

        // retrieve values from the cache
        static IEnumerable<View> GetViewsOfType(Type type) {...}

        // the notifier, declared by the attribute
        public static void OnChange(Type oldType, Type newType)
        {
            foreach (var oldinstance in GetViewsOfType(oldType)
            {
                var newInstance = System.Activator.CreateInstance(newType);
                newInstance.Parent = oldInstance.Parent; //a serialisation/deserialisation of the instance is better, Parent is given as an example
            }
        }
    }
}

Alternative Designs

1. event: creating an event has the problem of registration, mainly for 3rd party libraries
2. implement this capability at the framework level. repetitive work, and the runtime would still have to implement it's own mechanism to invalide its caches (I'm thinking ReflectionCache, ....)
3. whatever applies the change could also trigger the notification, but that would require injecting some shared code in all platforms supporting EnC/HR

Risks

Unknown at this time

Author:	StephaneDelcroix
Assignees:	-
Labels:	api-suggestion, area-Diagnostics-coreclr, untriaged
Milestone:	-

[jkotas]

Meaning corelib itself would have an attribute on it like:
[assembly: UpdateAppliedHandler("System.Reflection.Type", "ClearReflectionCaches", "D02B1C93-A6E5-4F07-A45D-2CD02675CF77")]

Yep.

with whatever attribute name and arguments we decided on

I think the attribute should have just the type name. No method name or Guid. All those details can be attached to the update handler type.

You could imagine a complicated versioning scheme where every cache is tagged with a version, JsonSerializer updates its notion of valid version on every update, and it only respects caches created with that same version number, or something like that.

Yep, there is a lot of complicated schemes we can invent here to push this further. Another option can be to have BeforeUpdate and AfterUpdate methods on the update handler, and require the update handler takes a lock in BeforeUpdate and release the lock in AfterUpdate.

ordering the invocations from lower in the assembly reference graph to higher in the graph would suffice.

Yes, this works for assemblies with static dependencies. It does not work for cases where the graph is composed dynamically, via DI or similar architectures. Again, whether this is important is a question for how far we want to go.

[mikem8361]

Couldn't AppDomain.AssemblyLoad be used for the dynamic cases by the Hot Reload Agent?

All those details can be attached to the update handler type.

If the attribute only has the type name, how does the agent know what to call on that type? Is the agent going to make assumptions on the name and parameters of the method or methods? Would defining an interface that the type implements make sense?