Skip to content

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

Merged
merged 7 commits into from
Jun 17, 2025

Conversation

dongle-the-gadget
Copy link
Contributor

@dongle-the-gadget dongle-the-gadget commented Jun 4, 2025

This commit allows the CCW VTables for interfaces that don't inherit to be folded by ILC.

This commit allows the CCW VTables for interfaces that don't inherit to be folded by ILC to reduce binary size.
@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 04:49
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 4, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 updates ComInterfaceGenerator 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 named ManagedVirtualMethodTable. 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));

Copy link
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@AaronRobinsonMSFT
Copy link
Member

to be folded by ILC to reduce binary size.

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

@AaronRobinsonMSFT
Copy link
Member

It would also be helpful to see a before/after of the source generated code that will be emitted for the scenario in question.

@dongle-the-gadget
Copy link
Contributor Author

dongle-the-gadget commented Jun 4, 2025

Please provide an example of what the savings will look like. For example, what will be the savings in say the CsWinRT core library?

Sorry, I misremembered this part. This PR was meant to reduce the initialization cost of startup, not binary size.

before/after of the source generated code

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

@Sergio0694
Copy link
Contributor

@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 ref readonly in .NET 10 for Unsafe.AsPointer 🙂

@dongle-the-gadget
Copy link
Contributor Author

@Sergio0694 done!

@Sergio0694
Copy link
Contributor

@dongle-the-gadget is there anything you need? Just reviews, or were you blocked on anything?

@dongle-the-gadget
Copy link
Contributor Author

The functionality + preinitialization test is complete so I think only reviews.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a 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.

@jkoritzinsky jkoritzinsky enabled auto-merge (squash) June 13, 2025 18:44
@AaronRobinsonMSFT
Copy link
Member

D:\a_work\1\s\artifacts\obj\SharedTypes\Debug\net10.0\Microsoft.Interop.ComInterfaceGenerator\Microsoft.Interop.ComInterfaceGenerator\SharedTypes.ComInterfaces.IEmpty.cs(29,58): error CS0649: (NETCORE_ENGINEERING_TELEMETRY=Build) Field 'InterfaceImplementation.Vtable' is never assigned to, and will always have its default value

I don't understand why this is occurring. @dongle-the-gadget Can you take a look?

@dongle-the-gadget
Copy link
Contributor Author

@AaronRobinsonMSFT looks like it's related to dotnet/roslyn#77528

@dongle-the-gadget
Copy link
Contributor Author

dongle-the-gadget commented Jun 14, 2025

I have a theory of what's happening:

@AaronRobinsonMSFT
Copy link
Member

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.

auto-merge was automatically disabled June 14, 2025 16:08

Head branch was pushed to by a user without write access

@AaronRobinsonMSFT
Copy link
Member

/ba-g Unrelated timeouts

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit e34a3c4 into dotnet:main Jun 17, 2025
90 of 94 checks passed
@Sergio0694
Copy link
Contributor

Thank you @dongle-the-gadget!!
And thank you Aaron and Jeremy for the reviews 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime.InteropServices community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants