Avoid an allocation on every GetOrCreateComInterfaceForObject call - #132481
Avoid an allocation on every GetOrCreateComInterfaceForObject call#132481Sergio0694 wants to merge 3 commits into
GetOrCreateComInterfaceForObject call#132481Conversation
GetOrCreateComInterfaceForObject passes the state its ConditionalWeakTable factory needs as an anonymous type, which is a class, so every call allocates one. The factory only runs when a wrapper has to be created, and almost every call finds one that already exists, so that allocation is garbage produced by nearly every transition of a managed object into native code. The state is a struct now, which GetOrAdd supports as its TArg is unconstrained, so the lookup path allocates nothing at all. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/interop-contrib |
jkoritzinsky
left a comment
There was a problem hiding this comment.
It's really disappointing that we can't use anonymous types here TBH. Effectively makes things overly wordy when it really shouldn't be necessary.
LGTM
There was a problem hiding this comment.
Pull request overview
This PR removes a per-call heap allocation in ComWrappers.GetOrCreateComInterfaceForObject by replacing an anonymous-type state object passed to ConditionalWeakTable.GetOrAdd with a small readonly struct state payload.
Changes:
- Replace
new { This = this, flags }withCreateManagedObjectWrapperStateto avoid allocating state on every call. - Update the
GetOrAddvalue-factory to use the new state shape (state.ComWrappers,state.Flags). - Add a private nested state struct (with documentation) to carry
ComWrappersandCreateComInterfaceFlags.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Head branch was pushed to by a user without write access
Co-authored-by: Aaron R Robinson <arobins@microsoft.com>
It could be a tuple instead? |
|
@MattParkerDev we tried using value tuples before in ComWrappers and we reverted it. It roots a bunch of additional cruft that can't be trimmed and was causing something like a 7KB size regression on NAOT for no reason. A custom struct is better. |
ComWrappers.GetOrCreateComInterfaceForObjectpasses the state itsConditionalWeakTable.GetOrAddfactory needs as an anonymous type:Anonymous types are classes, so that's 32 bytes allocated on every call. The factory only runs when the object doesn't have a wrapper yet, and in practice almost every call finds one that already exists, so nearly all of it is garbage that never gets used. This is the path every managed object takes on its way into native code, so it adds up.
GetOrAdd<TArg>doesn't constrainTArg(it even allows ref structs), so a small struct works here and costs nothing to pass:No behaviour change, no API change.
Benchmarks
Release runtime, x64, two alternating passes per build. The allocation numbers were identical across runs.
The lookup path is allocation free now. The first row gains more than the second because the garbage was also costing GC pressure and cache misses, not just the allocation itself. The creation row is within noise on time, since it's dominated by building the wrapper, but it still drops the 32 bytes.
Testing
ComWrappersTests,ComWrappersTestsBuiltInComDisabled,GcRestrictedCalloutReversePInvoke,GlobalInstanceMarshallingTests,GlobalInstanceMarshallingTestsBuiltInComDisabled,GlobalInstanceTrackerSupportTests_TargetWindowsandWeakReferenceTestall pass against the modified CoreLib, built Checked.Note
Parts of this pull request description were generated with GitHub Copilot. All benchmark numbers in it were measured locally.