-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Folds CCW VTables for root COM interfaces #116289
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
Conversation
This commit allows the CCW VTables for interfaces that don't inherit to be folded by ILC to reduce binary size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables CCW VTables for root COM interfaces to be folded by the Native IL compiler (ILC), reducing the binary size.
- Adds a NativeAOT test project and test case to validate that CCW VTables are preinitialized without a static constructor.
- Introduces
FixedAddressValueTypeAttribute
into the source generator and integrates a new VTable struct generation step. - Exposes
GenerateUnmanagedFunctionPointerTypeForMethod
and updatesComInterfaceGenerator
to emit and reference the VTable struct.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
System.Runtime.InteropServices/tests/TrimmingTests/*.proj | Adds CCWPreinitializationNativeAot.cs to the test project for NativeAOT validation |
System.Runtime.InteropServices/tests/TrimmingTests/CCWPreinitializationNativeAot.cs | Implements a test to check that no type initializer (.cctor ) exists on the COM wrapper |
gen/Microsoft.Interop.SourceGeneration/TypeNames.cs | Adds constants and name syntax for FixedAddressValueTypeAttribute |
gen/ComInterfaceGenerator/VirtualMethodPointerStubGenerator.cs | Changes access modifier of GenerateUnmanagedFunctionPointerTypeForMethod |
gen/ComInterfaceGenerator/ComInterfaceGenerator.cs | Adds new VTable struct generation, updates code emission, and suppresses extra warnings |
Comments suppressed due to low confidence (3)
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VirtualMethodPointerStubGenerator.cs:260
- Consider using an internal access modifier instead of public for
GenerateUnmanagedFunctionPointerTypeForMethod
to limit its exposure to only the generator assembly.
public static FunctionPointerTypeSyntax GenerateUnmanagedFunctionPointerTypeForMethod(
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs:554
- [nitpick] The struct name "InterfaceImplementationVtable" uses inconsistent casing for "Vtable" compared to the step name "GenerateNativeToManagedVTableStruct"—consider renaming to
InterfaceImplementationVTable
for consistency.
private static readonly StructDeclarationSyntax InterfaceImplementationVtableTemplate = StructDeclaration("InterfaceImplementationVtable")
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs:823
- The comment references the old property name
VirtualMethodTableManagedImplementation
, but the new property is namedManagedVirtualMethodTable
. Please update the comment to match the current code.
// public static void** VirtualMethodTableManagedImplementation => (void**)System.Runtime.CompilerServices.Unsafe.AsPointer(ref Unsafe.AsRef(in InterfaceImplementation.Vtable));
Tagging subscribers to this area: @dotnet/interop-contrib |
@dongle-the-gadget Please provide an example of what the savings will look like. For example, what will be the savings in say the CsWinRT core library? |
It would also be helpful to see a before/after of the source generated code that will be emitted for the scenario in question. |
Sorry, I misremembered this part. This PR was meant to reduce the initialization cost of startup, not binary size.
Here's a diff, with some formatting differences and namespaces removed file unsafe class InterfaceInformation
{
- private static void** _vtable;
- public static void** ManagedVirtualMethodTable => _vtable != null ? _vtable : (_vtable = InterfaceImplementation.CreateManagedVirtualFunctionTable());
+ public static void** ManagedVirtualMethodTable => (void**)Unsafe.AsPointer(ref Unsafe.AsRef(in InterfaceImplementation.Vtable));
}
+ file unsafe struct InterfaceImplementationVtable
+ {
+ public delegate* unmanaged[MemberFunction]<void*, Guid*, void**, int> QueryInterface_0;
+ public delegate* unmanaged[MemberFunction]<void*, uint> AddRef_1;
+ public delegate* unmanaged[MemberFunction]<void*, uint> Release_2;
+ public delegate* unmanaged[MemberFunction]<ComWrappers.ComInterfaceDispatch*, int*, int> Method_3;
+ }
file unsafe partial interface InterfaceImplementation
{
+ [FixedAddressValueType]
+ public static readonly InterfaceImplementationVtable Vtable;
- internal static void** CreateManagedVirtualFunctionTable()
+ static InterfaceImplementation()
{
- void** vtable = (void**)RuntimeHelpers.AllocateTypeAssociatedMemory(typeof(global::IComInterface), sizeof(void*) * 4);
{
- nint v0, v1, v2;
- ComWrappers.GetIUnknownImpl(out v0, out v1, out v2);
- vtable[0] = (void*)v0;
- vtable[1] = (void*)v1;
- vtable[2] = (void*)v2;
+ ComWrappers.GetIUnknownImpl(
+ out *(nint*)&((InterfaceImplementationVtable*)Unsafe.AsPointer(ref Vtable))->QueryInterface_0,
+ out *(nint*)&((InterfaceImplementationVtable*)Unsafe.AsPointer(ref Vtable))->AddRef_1,
+ out *(nint*)&((InterfaceImplementationVtable*)Unsafe.AsPointer(ref Vtable))->Release_2);
}
{
- vtable[3] = (void*)(delegate* unmanaged[MemberFunction]<ComWrappers.ComInterfaceDispatch*, int*, int> )&ABI_Method;
+ Vtable.Method_3 = &ABI_Method;
}
- return vtable;
}
} |
@dongle-the-gadget you can simplify: (void**)Unsafe.AsPointer(ref Unsafe.AsRef(in InterfaceImplementation.Vtable)); To just: (void**)Unsafe.AsPointer(in InterfaceImplementation.Vtable); We changed the parameter to |
@Sergio0694 done! |
@dongle-the-gadget is there anything you need? Just reviews, or were you blocked on anything? |
The functionality + preinitialization test is complete so I think only reviews. |
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@jkoritzinsky Please give this a once over. I'm satisfied.
I don't understand why this is occurring. @dongle-the-gadget Can you take a look? |
@AaronRobinsonMSFT looks like it's related to dotnet/roslyn#77528 |
I have a theory of what's happening:
|
@dongle-the-gadget Thanks for the excellent analysis, much appreciated. Okay, let's add back the suppression. We should be good after that. |
Head branch was pushed to by a user without write access
/ba-g Unrelated timeouts |
Thank you @dongle-the-gadget!! |
This commit allows the CCW VTables for interfaces that don't inherit to be folded by ILC.