microsoft / CsWinRT

C# language projection for the Windows Runtime
MIT License
550 stars 106 forks source link

Proposal: using the visitor pattern to make CsWinRT projection vtables AOT-safe #1322

Open Sergio0694 opened 1 year ago

Sergio0694 commented 1 year ago

I've been working with CsWinRT more recently, and thinking about how it could be made AOT-safe. One of the main issues is that CsWinRT has to use several methods that need to be generic, but where the type argument is the vtable type of a projection type, which CsWinRT itself doesn't know about (as these types can be in separate projection assemblies). What's more, in some of these cases CsWinRT isn't even in any generic context whatsoever to begin with. That is, in several scenarios all that CsWinRT has is some Type helperType value it retrieved from a projection type (which is looked up through the WindowsRuntimeHelperType attribute, and that's it. So, we need a way to then be able to generate generic code based on this alone.

The idea for this is to leverage the visitor pattern to achieve effectively the equivalent of associated types. That is, vtable types in projection types will act as visitable objects, accepting a visitor exposing a generic method, which they will use to "inject" their own generic context, with the type argument being the vtable type. This allows CsWinRT to pass generic code that will be invoked (à la double dispatch) by the helper types, so it can correctly receive the necessary generic context in order to work.

API proposal

namespace WinRT.Interop;

interface IWinRTHelperTypeProxy
{
    void Accept<T>(ref T visitor)
        where T : IWinRTHelperTypeVisitor;
}

interface IWinRTHelperTypeVisitor
{
    void Visit<TVftbl>()
        where TVftbl : unmanaged, IWinRTHelperType<TVftbl>;
}

interface IWinRTHelperType<TSelf>
    where TSelf : unmanaged, IWinRTHelperType<TSelf>
{
    static abstract IntPtr AbiToProjectionVftablePtr { get; }
    static abstract TSelf AbiToProjectionVftbl { get; }
    static abstract ObjectReferenceValue CreateMarshaler2<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces)] T>(T obj);

    // Any other members here (eg. "ToAbi", "FromAbi", etc.)...
}

The IWinRTHelperTypeProxy interface is implemented by the WinRT helper type that you specify in the attribute over custom types. CsWinRT already looks this type up today. But the difference is that instead of using a boatload of reflection after that, it can now simply check whether that type can be assigned to IWinRTHelperTypeProxy. If it can, it gets an instance from RuntimeHelpers.GetUninitializedObject (which can easily be shared in a static). Then, it uses a visitor object to pass a callback (basically using double dispatch here) that can be invoked from the helper type within a generic context. This allows the CsWinRT code to be invoked with the concrete helper type, which makes it AOT-safe to then do stuff like invoking generic methods with it as a type argument. This should also be backwards compatible: the types match those currently being looked up via reflection, and also in case some projection types don't implement this new proxy interface, then CsWinRT would fallback to existing logic.

Example use

Here's a few examples that go into more detail of how this proposed pattern could be used to replace existing code paths.

Example 1 (click to expand):
Consider this code path: https://github.com/microsoft/CsWinRT/blob/e644f9cca17612adea4d726bef02069eb422f9aa/src/WinRT.Runtime/IWinRTObject.net5.cs#L144 This could instead use this small visitor type: ```csharp internal struct ObjectRefUnknownAsVisitor : IWinRTHelperTypeVisitor { private readonly ObjectReference _objectRef; private IObjectReference? _result; public ObjectRefUnknownAsVisitor(ObjectReference objectRef) => _objectRef = objectRef; [MemberNotNull(nameof(_result))] void IWinRTHelperTypeVisitor.Visit() { _result = _objectRef.As(); } public IObjectReference? GetResult() => _result; } ``` And then just do this: ```csharp ObjectRefUnknownAsVisitor visitor = new(objRef); HelperTypesMap.GetProxy(helperType).Visit(ref visitor); IObjectReference typedObjRef = visitor.GetResult(); ``` Which removes that `MakeGenericMethod` entirely and makes this AOT safe.
Example 2 (click to expand):
Consider this other code path: https://github.com/microsoft/CsWinRT/blob/e644f9cca17612adea4d726bef02069eb422f9aa/src/WinRT.Runtime/SingleInterfaceOptimizedObject.net5.cs#L39 This can also be made AOT-safe in a very similar way: ```csharp internal struct ObjectRefAsVisitor : IWinRTHelperTypeVisitor { private readonly IObjectReference _objectRef; private IObjectReference? _result; public ObjectRefAsVisitor(IObjectReference objectRef) => _objectRef = objectRef; [MemberNotNull(nameof(_result))] void IWinRTHelperTypeVisitor.Visit() { _result = _objectRef.As(); } public IObjectReference? GetResult() => _result; } ``` ```csharp ObjectRefAsVisitor visitor = new(objRef); HelperTypesMap.GetProxy(helperType).Visit(ref visitor); IObjectReference typedObjRef = visitor.GetResult(); ```
Example 3 (click to expand):
This pattern isn't only useful to make things AOT-safe, it can also just remove reflection in general. Consider yet another code path: https://github.com/microsoft/CsWinRT/blob/e644f9cca17612adea4d726bef02069eb422f9aa/src/WinRT.Runtime/Marshalers.cs#L470-L475 This can be rewritten as follows: ```csharp internal struct CreateMarshaler2FuncVisitor : IWinRTHelperTypeVisitor { private readonly Func? _func; [MemberNotNull(nameof(_func))] void IWinRTHelperTypeVisitor.Visit() { _func = TVftbl.CreateMarshaler2; } public Func? GetResult() => _func; } ``` ```csharp CreateMarshaler2FuncVisitor visitor = default; HelperTypesMap.GetProxy(HelperType).Visit(ref visitor); Func marshaler2 = visitor.GetResult(); ```
Example 4 (click to expand):
Related to 3, not including every single case here, but this approach can also help to optimize all various cases where the vtable type is being used by CsWinRT, eg. when computing vtables to prepare the CCWs for custom projection types, where it currently just manually looks up all various fields (eg. "AbiToProjectionVftablePtr") via reflection.

Example projection interface

Here's an example of what a full projection interface type could look like, supporting this proposal:

Projection interface (click to expand):
```csharp [Guid("33D8B971-2ABF-4A2B-8071-1FFCBCBC8124")] [WindowsRuntimeType] [WindowsRuntimeHelperType(typeof(TypeProxy))] public interface IMyInterface { int GetNumber(); private sealed class TypeProxy : IWinRTHelperTypeProxy { void IWinRTHelperTypeProxy.Accept(ref T visitor) { visitor.Visit(); } [Guid("33D8B971-2ABF-4A2B-8071-1FFCBCBC8124")] public unsafe struct Vftbl : IWinRTHelperType { public static readonly IntPtr AbiToProjectionVftablePtr = InitVtbl(); private static IntPtr InitVtbl() { Vftbl* lpVtbl = (Vftbl*)ComWrappersSupport.AllocateVtableMemory(typeof(Vftbl), sizeof(Vftbl)); lpVtbl->IUnknownVftbl = IUnknownVftbl.AbiToProjectionVftbl; lpVtbl->GetNumber = &GetNumberFromAbi; return (IntPtr)lpVtbl; } private IUnknownVftbl IUnknownVftbl; private delegate* unmanaged[Stdcall] GetNumber; static nint IWinRTHelperType.AbiToProjectionVftablePtr => AbiToProjectionVftablePtr; static Vftbl IWinRTHelperType.AbiToProjectionVftbl => *(Vftbl*)AbiToProjectionVftablePtr; static ObjectReferenceValue IWinRTHelperType.CreateMarshaler2<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces)] T>(T obj) { return MarshalInspectable.CreateMarshaler2(obj); } [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })] private static int GetNumberFromAbi(IntPtr thisPtr) { try { return ComWrappersSupport.FindObject(thisPtr).GetNumber(); } catch (Exception e) { ExceptionHelpers.SetErrorInfo(e); return Marshal.GetHRForException(e); } } } } } ```

Notes

I'm not saying this is necessarily the best way to solve this. This is meant to be an idea to hopefully start a conversation from, with the goal of coming up with a solution to make CsWinRT fully AOT-safe in the future. At the very least, it would seem that some form of "associated types" would help getting there, which is the approach I also used here.

cc. @AaronRobinsonMSFT @jkoritzinsky @MichalStrehovsky @agocke @jkotas for thoughts 🙂

jkotas commented 1 year ago

it gets an instance from RuntimeHelpers.GetUninitializedObject

Activator.CreateInstance would be more appropriate here.

AOT-safe

Generic virtual methods are AOT-safe, but not necessarily AOT friendly. The AOT compiler has to generate all possible combinations of the generic instantiations to make them safe. It typically leads to quadratic code size explosion and a lot of dynamically unreachable code. Would it be a problem here?

Sergio0694 commented 1 year ago

"Activator.CreateInstance would be more appropriate here."

Right, I was thinking of RuntimeHelpers.GetUninitializedObject as that's the usual "poor man's static abstracts" approach on .NET Standard 2.0, so you can use it in cases where you don't necessarily have parametless constructors. If we say that the contract here assumes that the helper type proxy must have a public parameterless constructor then yeah I guess Activator.CreateInstance would also be just fine. Out of curiosity, are there any big differences between the two in terms of speed? I thought they were mostly equivalent for the non-generic case? 🤔

"Generic virtual methods are AOT-safe, but not necessarily AOT friendly. The AOT compiler has to generate all possible combinations of the generic instantiations to make them safe. It typically leads to quadratic code size explosion and a lot of dynamically unreachable code."

I assume here that would be a problem for both Accept and Visit? In theory at least for Accept, that could also be a non-generic method, just taking an IWinRTHelperTypeVisitor interface instance. It'd force callers to allocate the small visitor object, but it might not be important for perf given that shouldn't be in a hot-path. I assume that'd at least help?

As for Visit, would you have another idea on how to make that more AOT-friendly here? I don't think we can drop that generic here as that's what makes the whole thing work, but would you know of maybe some other way to structure that to achieve the same but in a more AOT-friendly way?

EDIT: I guess we could also do this, in theory:

abstract class WinRTHelperTypeProxy
{
    abstract void Accept(WinRTHelperTypeVisitor visitor);
}

abstract class WinRTHelperTypeVisitor
{
    abstract void Visit<TVftbl>()
        where TVftbl : unmanaged, IWinRTHelperType<TVftbl>;
}

Basically:

Would this be better and help?


In general, does the overall idea seem reasonable? One of my goals here was mostly to at least get a feel for whether such an approach would be considered viable/ok, even if the exact implementation didn't have to be the final one already 🙂

jkotas commented 1 year ago

If we say that the contract here assumes that the helper type proxy must have a public parameterless constructor

You would just need to make sure that the trimming annotations match. RuntimeHelpers.GetUninitializedObject is annotated with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.PublicConstructors)]. Activator.CreateInstance overloads are annotated the same or a subset for (Public-only or PublicParameterlessConstructor-only).

Out of curiosity, are there any big differences between the two in terms of speed? I thought they were mostly equivalent for the non-generic case?

Parameter-less Activator.CreateInstance is signfificantly faster compared to GetUninitializedObject in CoreCLR today. We do not spend a lot on GetUninitializedObject performance. It is not a method that we want to encourage people to use.

In general, does the overall idea seem reasonable?

I believe that general wisdom is that a custom global analysis tool is needed for WinRT support that has performance characteristics on par with .NET Native UWP. The solutions without the custom global analysis tool leave performance on the table - either throughput or in bigger size. At the first glance, this looks like a solution that would lead to large binary sizes.

Sergio0694 commented 1 year ago

"I believe that general wisdom is that a custom global analysis tool is needed for WinRT support that has performance characteristics on par with .NET Native UWP."

I see, that makes sense. Just so I understand - would that be specific for when CsWinRT is used with NativeAOT, or at least would it need a different mode for when NativeAOT is used to fully leverage the fact that no dynamic loading can be done? That is, my understanding was that one of the reasons why MCG is so effective on UWP is that it can also rely on the fact that .NET Native doesn't support loading assemblies (nor creating new types at runtime), so it can fully optimize stuff with a complete program view with the full set of possible types for everything. Wouldn't that not be the case for CsWinRT when used eg. on a normal WinUI 3 app running on CoreCLR, which could very well load some additional .dll-s at runtime containing projected types and whatnot? I guess what I'm curious about is: would this hypothetical global analysis tool have to account for the two possible modes and limit the set of optimizations it can (or can't) do in either case? 🤔

jkotas commented 1 year ago

The global analysis needs to see all code that is included in program. It would not be a global analysis otherwise.

APIs to create new types (e.g. Type.MakeGenericType) and load new assemblies (e.g. Assembly.LoadFile) are not trimming and AOT safe. It is ok for the global analysis to assume that these APIs are not used by the app. If these APIs are used by the trimmed app, you should see a warning. We do not guarantee that things are going to work if there are trim warnings.

jlaanstra commented 1 year ago

NativeAOT can also generate native dlls right? Could a NativeAOT app call into a native dll that then loads another NativeAOT compiled managed dll?

jkoritzinsky commented 1 year ago

In that scenario, there will be two separate NativeAOT runtimes in the process and each separately-compiled NativeAOT component would have its own runtime (and they'd communicate across a non-.NET interop boundary)