internal runtime API to return hot reload capabilities

[lambdageek]

MonoVM and CoreCLR will both support hot reload in .NET 6. However the actual set of supported (non-"rude") edits are different between the runtimes. In future releases the capabilities of both runtimes will evolve. Part of #44806 and #45629

Related roslyn issue: dotnet/roslyn#49010

In a hot reload editing session, the runtime and the Roslyn compiler (mediated by a communication mechanism such as dotnet watch or the Visual Studio debugger) will need to agree on the supported edits.

One approach would be to build into Roslyn the knowledge of the capabilities of each runtime based on the reported runtime version. However in this case it would be difficult to mix compiler and runtime versions. Also if the runtime wants to disable an edit in a servicing release, it would require closely coupling servicing releases of (potentially several) Roslyn versions and the runtime.

Another approach is to expose a new API implemented by the runtime that returns a list of the supported capabilities:

 namespace System.Reflection.Metadata
{
    partial class AssemblyExtensions
    {
        /// Returns a description of the supported System.Reflection.Metadata.AssemblyExtensions.ApplyUpdate edits.
        /// Returns string.Empty if applying updates is not supported by this runtime.
        internal static string GetApplyUpdateCapabilities ();
    }
}

The specific format of the return value would be implementation defined and in practice it would be a contract between the runtimes and Roslyn.

Example of capabilities description

(This is for illustration only and not part of the API definition)

For example the string might be a space separated sequence of tokens from the following set: Baseline, RuntimeEdits, AddDefinitionToExistingType, NewTypeDefinition. The proposal is to represent .NET 5 CoreCLR capabilities as Baseline plus everything that .NET 5 CoreCLR can do minus what .NET 6 MonoVM can do. That is, .NET 5 MonoVM would be represented by the empty set of capabilities, .NET 5 CoreCLR as Baseline plus everything that CoreCLR can do that .NET6 Mono cannot. In future releases, as MonoVM catches up it will start returning AddDefinitionToExistingType (for example). In future releases when one or both runtimes add a new capability, it will be added to the enumeration. In summary:

• .NET 5 CoreCLR, .NET Framework: "Baseline AddDefinitionToExistingType NewTypeDefinition " (and ... TBD based on Mono capabilities )
• .NET 6 CoreCLR: .NET 5 + "RuntimeEdits"
• .NET 5 MonoVM: ""
• .NET 6 MonoVM: "Baseline RuntimeEdits"

[dotnet-issue-labeler]

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.

[lambdageek]

/cc @tmat @mikem8361 @tommcdon

[jkotas]

We would define negative capabilities for Mono,

That looks weird. I think we should define this just in term of the possitive capabilities.

Is there a reason why not just use a flags enum for this?

[jkotas]

Potential alternative design is to avoid introducing a new API for this and just use the existing RuntimeFeature.IsSupported API. e.g. RuntimeFeature.IsSupported("AddFieldDefinition").

[tmat]

That looks weird. I think we should define this just in term of the possitive capabilities.

The assumption was that there wouldn't be that many negative and the set would be empty for .NET Framework/.NET 5.
Otherwise we would need to hard-code list of everything that .NET Framework, .NET 5 can do into the code that calls this API (since the runtimes do not have the API, the call would fail and the fall back would assume it's an older runtime).

Is there a reason why not just use a flags enum for this?

64 might not be enough bits.

[jkotas]

It would be useful to see the full set we expect to add for .NET 5.

HotReloadCapability

This should not be called HotReload. We have dropped "HotReload" from the apply delta API as well.

[tmat]

It would be useful to see the full set we expect to add for .NET 5.

For .NET 5, it would be an empty set - the same as .NET Framework.
For .NET 6 it would probably also be one item: RuntimeEditsSupported - which would indicate that we can apply changes while the app is running (as opposed to when suspended at breakpoint).

For Mono we are discussing the set here: dotnet/roslyn#49010 (comment).

Something like: RuntimeEditsSupported, AddDefinitionToExistingTypeNotSupported, AddNewTypeDefinitionNotSupported, etc. TBD

[tmat]

Good question. I think that would need to be special-cased as "nothing is supported".

[tmat]

For reference, here are all the current rude edits. They do not map to metadata edits directly, but you can get sense of what is reported.

[jkotas]

For Mono we are discussing the set here: dotnet/roslyn#49010 (comment).

Can you paste it here once the full set of capabilities is determined?

Otherwise we would need to hard-code list of everything that .NET Framework,

It does not sound that bad to me (vs. having to keep explaining till forwever the reason behind the possitive/negative split).

[tmat]

Can you paste it here once the full set of capabilities is determined?

Sure.

In time the negative ones become obsolete as Mono implements more features. The same happens with Roslyn's Rude Edits. We obsolete them as we allow more edits.

The problem with listing everything that should be enabled is also that we'd need to implement each of the positive capability in Roslyn - we can't assume that certain capabilities are always present. With negative ones it's much easier - we only need to revisit reporting rude edits that are affected by something that Mono does not have. We don't need to revisit every single rude edit and condition it based on a set of positive capabilities.

[jkotas]

The problem with listing everything that should be enabled is also that we'd need to implement each of the positive capability in Roslyn - we can't assume that certain capabilities are always present

You do not have to. You can check whether the baseline is available, and disable all edits if it is not.

[tmat]

By "baseline" you mean .NET 5 - Mono 6? We could do that. Seems useless then to list capabilities in two places and subtract them, essentially ignoring them. I guess another way of doing this would be to have a single capability Baseline which would represent this difference (.NET 5 - Mono 6).

[tmat]

So, how about this?

• .NET 5, .NET Framework: { Baseline, AddDefinitionToExistingType, NewTypeDefinition, ... TBD based on Mono capabilities }
• .NET 6: .NET 5 + { RuntimeEdits }
• Mono 5: { }
• Mono 6: { Baseline, RuntimeEdits }

(exact names TBD)

[jkotas]

I assume that .NET 6 would also include everything in .NET 5 bucket. Also, is the list really exactly the same between .NET 5 and .NET Framework? I thought that made some EnC improvements in .NET Core.

Sounds reasonable (exact names TBD).

[tmat]

I assume that .NET 6 would also include everything in .NET 5 bucket.

Yes, fixed.

[tmat]

I thought that made some EnC improvements in .NET Core.

I am not aware of any (so far).

[lambdageek]

Updated the proposal with summary of the above discussion: positive flags; .NET5 CoreCLR - .NET 6 MonoVM as Baseline; changed the API name not to use HotReload.

[jkotas]

How is the hotreload agent going to use this API? I assume that it will just enumerate everything, serializes it and sends it over to the manager to inform the compiler. That makes me think - would it be better just have an API that returns string?

public class RuntimeFeature
{
    public string? GetAssemblyUpdateCapabilities();
}

The string can be space separated list. The exact names are not visible in the API surface so making them pretty is less important. I think the negatives that were in the original proposal would be acceptable in this case (e.g. prefix the name with minus). And you do not have to go through API review and documentation for every EnC improvement.

[tmat]

A string would be fine with me.

[jkotas]

linker can trim out hot reload support from managed libraries.

I do not think we want to attach trimming of the hot reload support to the capabilities string. It should be separate - already tracked by #51159.

[bartonjs]

We feel that rather than string tokenization this should just be a flags enum.

partial class RuntimeFeatures
{
    [Flags]
    public enum AssemblyUpdateFeatures : long
    {
        None = 0,
        RuntimeEdits = 1 << 0,
        AddDefinitionToExistingType = 1 << 1,
        ...
    }

    public static AssemblyUpdateFeatures AssemblyApplyUpdateCapabilities { get; }
}

is what got doodled during the discussion, those names weren't really discussed.

[tmat]

As @jkotas pointed out a string has benefits over enum.

Beside those that Jan mentioned the components consuming this API do not target .NET 6, so there is no real value in making the values strongly typed. We will need to parse the values from strings/int anyways.

[jkotas]

Does this actually need to be a public API? Can this be just an internal API, called via private reflection, just like the hot reload update handlers?

[lambdageek]

In the API review, the feedback I heard was:

1. A string API doesn't absolve us of having to come to agreement about the structure of the capabilities - roslyn and the runtimes still have to use the same representation.
2. Adding a public API even if it's intended for the hot reload agent and roslyn doesn't preclude others from using the API - therefore rather than forcing those folks to use an ad-hoc string parser, we should just give them a bitmask of flags.

---

Does this actually need to be a public API? Can this be just an internal API, called via private reflection, just like the hot reload update handlers?

I don't think there's a technical need for it to be public.

[jkotas]

Then let's just keep it internal and return string. We have a ton of internal surface to support debuggers, so an internal method like this would not be an outlier.

[lambdageek]

If it's going to be internal, I prefer to make it a method, rather than a property (easier to call from reflection), and also move it to System.Reflection.Metadata.AssemblyExtensions so that it's near ApplyUpdate(). Updated the issue description.