Skip to content

Share implementation of ComWrappers between CoreCLR and NativeAOT #113907

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 42 commits into from
May 10, 2025

Conversation

jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented Mar 26, 2025

Share the managed implementation of ComWrappers from NativeAOT with CoreCLR.

Remove all parts of CoreCLR's native implementation that don't need to run during GC and provide a manually-managed implementation of the logic that must run during GC.

Reimplement DAC's ComWrappers support on top of the ConditionalWeakTables instead of using the entries in the syncblock.

Memory improvements for a synthetic benchmark of exposing 1 million objects to COM and consuming 1 million objects from COM:

Main Peak Working Set: 1,027,820 KiB
PR Peak Working Set: 779,712 KiB

Move portions of ComWrappers.NativeAot.cs that aren't run during GC into ComWrappers.Common.cs

Convert CoreCLR to use the managed ComWrappers implementation wherever possible (everywhere except for during GC)

Move wrapper ID to be a CoreCLR-only concept (as it's only needed for diagnostics support)
…he diagnostics-only table with the debugger support feature switch that already exists.
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 shares the managed implementation of COM wrapper support between CoreCLR and NativeAOT by refactoring the TrackerObjectManager and associated interop and GC routines. Key changes include:

  • Refactoring TrackerObjectManager and related interop functions to use partial classes and updated pointer/boolean conventions.
  • Updating GC and COM interop APIs (such as new lowMemoryPressure parameters and revised iterator signatures) to align with the shared managed implementation.
  • Adjusting project file entries and DAC/debug support to accommodate the consolidated COM wrapper logic.

Reviewed Changes

Copilot reviewed 59 out of 59 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TrackerObjectManager.NativeAot.cs Refactored tracking callbacks and added new API methods for reference path handling.
ComAwareWeakReference.NativeAot.cs Introduced a native-AOT specific implementation for weak reference conversion.
GC.NativeAot.cs Updated GC.Collect overloads to incorporate a lowMemoryPressure parameter.
System.Private.CoreLib.csproj Modified compile includes to remove outdated entries and add new interop source files.
trackerobjectmanager.cpp Revised COM tracker support code, updating iterator and callback patterns to use bool and modern C++ conventions.
InteropLib files (interoplib.cpp, interoplibimports.h, interoplibabi.h, interoplib.h, comwrappers.hpp) Modernized API signatures, constants, and pointer arithmetic, ensuring compatibility with new managed implementations.
DAC and debug-related files Updated DAC support for COM wrappers and interop facilities to match the shared implementation.
TrackerObjectManager.CoreCLR.cs & ComWrappers.CoreCLR.cs Ported and updated COM wrapper support for CoreCLR with new library import patterns.
Comments suppressed due to low confidence (3)

src/coreclr/interop/inc/interoplibimports.h:25

  • Ensure that changing the IteratorNext function signature from returning an HRESULT to a bool does not hide failure details; verify that all consumers are updated to properly handle a false return value.
bool IteratorNext(_In_ RuntimeCallContext* runtimeContext, _Outptr_result_maybenull_ void** trackerTarget, _Outptr_result_maybenull_ InteropLib::OBJECTHANDLE* proxyObject) noexcept;

src/coreclr/interop/trackerobjectmanager.cpp:32

  • Validate that replacing a NativeObjectWrapperContext pointer with an OBJECTHANDLE in the FindDependentWrappersCallback constructor preserves the intended context semantics and that the adjusted assertion correctly verifies source validity.
FindDependentWrappersCallback(_In_ OBJECTHANDLE sourceHandle, _In_ RuntimeCallContext* runtimeCallCxt)

src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs:242

  • Confirm that passing the lowMemoryPressure flag to _Collect is appropriately handled by RuntimeImports.RhCollect and that this change does not introduce unintended performance overhead in low-memory scenarios.
_Collect(generation, (int)iInternalModes, lowMemoryPressure);

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!

@jkoritzinsky
Copy link
Member Author

/ba-g networking timeout

@jkoritzinsky jkoritzinsky merged commit fd8933a into dotnet:main May 10, 2025
138 of 144 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants