Partition the ComWrappers RCW cache into per-processor buckets - #132033
Partition the ComWrappers RCW cache into per-processor buckets#132033Sergio0694 wants to merge 2 commits into
Conversation
The RCW cache is consulted on essentially every native to managed transition, and the finalizer thread concurrently takes write locks on it to remove entries for collected RCWs. With a single dictionary behind a single reader-writer lock, all of that traffic serializes on one lock, which shows up as the dominant cost in WinRT interop profiles. Split the cache into independent buckets, each holding the same dictionary and reader-writer lock as before, and select the bucket for a given COM instance by hashing its pointer. The number of buckets matches the processor count (rounded up to a power of two), the same default concurrency level used by ConcurrentDictionary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The cached handles always point to a NativeObjectWrapper, so a strongly typed weak handle expresses that directly. It skips the type check on every read, allocates through GCHandle.InternalAlloc without revalidating the handle type, and reads the target once instead of twice when checking whether it was collected. 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. |
There was a problem hiding this comment.
Pull request overview
This PR improves ComWrappers RCW identity cache scalability by partitioning the per-instance cache into multiple per-processor buckets (each with its own lock and dictionary) and by switching cached entries to use a strongly-typed WeakGCHandle<NativeObjectWrapper>.
Changes:
- Partition the RCW cache into multiple buckets selected via a pointer hash to reduce lock contention on hot-path cache hits.
- Replace
GCHandle-based cache entries withWeakGCHandle<NativeObjectWrapper>for more direct and cheaper weak-handle operations. - Update the
ComWrappersNoLockAroundQueryInterfaceregression test to reliably exercise the same cache bucket/lock and to assert the cross-thread call completes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/tests/Interop/COM/ComWrappers/API/Program.cs | Updates the regression test to reuse the same COM identity pointer under the new bucketed cache and adds a completion assertion. |
| src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs | Implements the bucketed RCW cache design and moves cache entries to WeakGCHandle<NativeObjectWrapper>. |
|
Tagging subscribers to this area: @dotnet/interop-contrib |
| int bucketCount = (int)BitOperations.RoundUpToPowerOf2((uint)Environment.ProcessorCount); | ||
| Bucket[] buckets = new Bucket[bucketCount]; |
There was a problem hiding this comment.
| int bucketCount = (int)BitOperations.RoundUpToPowerOf2((uint)Environment.ProcessorCount); | |
| Bucket[] buckets = new Bucket[bucketCount]; | |
| uint bucketCount = BitOperations.RoundUpToPowerOf2((uint)Environment.ProcessorCount); | |
| Bucket[] buckets = new Bucket[bucketCount]; |
Cast not needed?
| // COM instances are heap allocated, so they're always at least pointer aligned (and 16-byte aligned | ||
| // in practice). That means their low bits are constant and can't be used to select a bucket directly. | ||
| // Multiplying by a large odd constant (2^64 divided by the golden ratio) mixes every input bit into | ||
| // the high half of the product, which is then masked to produce the index. The whole sequence lowers | ||
| // to a multiply, a shift and a mask, which is negligible next to the lookup that follows. | ||
| ulong hash = (ulong)(nuint)comPointer * 0x9E3779B97F4A7C15; | ||
| uint index = (uint)(hash >> 32) & (uint)(buckets.Length - 1); | ||
|
|
||
| // Return the bucket by reference, so that it's addressed in place in the array rather than copied | ||
| return ref buckets[index]; |
There was a problem hiding this comment.
This logic seems like it could use some asserts instead of cryptic outofrange. EDIT: noticed the & here now
| // COM instances are heap allocated, so they're always at least pointer aligned (and 16-byte aligned | ||
| // in practice). That means their low bits are constant and can't be used to select a bucket directly. | ||
| // Multiplying by a large odd constant (2^64 divided by the golden ratio) mixes every input bit into |
There was a problem hiding this comment.
Make sure this doesnt break with pointer tagging since it can be used with GC and native allocators.
This comment has been minimized.
This comment has been minimized.
| /// important because the cache is consulted on essentially every transition from native to managed code, and | ||
| /// because the finalizer thread concurrently takes write locks to remove entries for collected RCWs. | ||
| /// </remarks> | ||
| private readonly struct RcwCache |
There was a problem hiding this comment.
Align the design with ConcurrentDictionary/ConcurrentUnifier with a class exterior and inner Container struct?
|
@EgorBot --filter Interop.ComWrappersTests.* |
| _lock.EnterReadLock(); | ||
| try | ||
| private readonly ReaderWriterLockSlim _lock; | ||
| private readonly Dictionary<IntPtr, WeakGCHandle<NativeObjectWrapper>> _cache; |
There was a problem hiding this comment.
Using a dictionary within each bucket feels like we may be leaving perf on the table.
A few ideas:
- Use a custom hash comparer that has an inverse distribution and to the bucketing one (to avoid collisions re-colliding)
- Use ConcurrentUnifier instead of our own type
- Use an array of KeyValuePair like a regular dictionary and handle bucket resizing.
- Have a "single pair or dictionary type" to optimize for the "single bucket entry" case.
Motivation
ComWrapperskeeps a per-instance RCW (Runtime Callable Wrapper) identity cache, mapping a COM identity pointer to theNativeObjectWrappertracking the managed proxy for it. Until now that cache was a singleDictionary<IntPtr, GCHandle>behind a singleReaderWriterLockSlim.That single lock is a process-wide serialization point on a very hot path:
TryGetOrCreateObjectForComInstanceInternal→FindProxyForComInstance. In steady state this is a genuine cache hit, not a miss.NativeObjectWrapper.Release→RcwCache.Remove).So application threads doing read lookups continuously contend with the finalizer thread doing writes. Profiling CsWinRT 3.0 on NativeAOT showed this as the single largest source of overhead in WinRT interop:
RhSpinWait19.1% andntdll!RtlpEnterCriticalSectionContended11.4% exclusive on the main thread — i.e. the main thread mostly waiting.ReaderWriterLockSlim.TryEnterReadLockCore/ExitReadLock9.1% combined andRcwCache.FindProxyForComInstance6.4% in another benchmark.Even with no writers at all,
ReaderWriterLockSlim.EnterReadLockstill has to CAS a shared counter, so concurrent readers alone bounce a single cache line between cores.What this changes
1. Partition the cache into buckets (
a50b8c7)RcwCachebecomes a thin facade over aBucket[], where eachBucketholds exactly the sameDictionary+ReaderWriterLockSlim+ logic as the old single cache. The bucket for a given COM instance is chosen by hashing its pointer, so unrelated COM objects no longer contend.The bucket count is
BitOperations.RoundUpToPowerOf2(Environment.ProcessorCount)— matching the default concurrency levelConcurrentDictionaryuses, and rounded to a power of two so the index is a mask rather than a division.Some details worth calling out:
0x9E3779B97F4A7C15and take the high half — which lowers to a multiply, a shift and anand. Distribution was validated two ways over 200k keys, because the two input classes have different correct expectations:n/k(χ² between 0.01 and 0.52 for k = 16/64/128). These keys are an arithmetic progression, not a random sample, so χ² ~ χ²(k−1) does not apply here — a multiply-shift hash stratifies an arithmetic progression almost perfectly, and χ² ≈ 0 is the desired outcome rather than a suspicious one. The contrast is the point: the exact same keys under raw masking give χ² ≈ 3×10⁶–1.3×10⁷, i.e. every object in one bucket.RcwCacheandBucketarereadonly structs.RcwCacheis inlined into theComWrappersobject andBucketinstances live inline in the array, which avoids two pointer chases on the lookup path and lets several buckets share a cache line.GetBucketreturnsref readonly Bucketso the 16-byte struct is never copied. There is no false sharing to worry about: the bucket fields are only ever written during construction, and all mutable state lives in the referenced lock and dictionary.RemoveAllno longer removes as one atomic batch, since wrappers can now span buckets. Its only caller is apartment/context teardown inTrackerObjectManager, and the cache is only ever observed one entry at a time, so nothing could depend on that atomicity.2. Use
WeakGCHandle<NativeObjectWrapper>(2074c0d)The cached handles always point to a
NativeObjectWrapper, so a strongly typed weak handle expresses that directly. It skips the type check on every read, allocates throughGCHandle.InternalAllocwithout revalidating the handle type, and reads the target once instead of twice when checking whether it was collected (which also removes a benign race where the two reads could disagree).Benchmark results
All numbers measured locally on a 32-core Windows x64 machine (Hyper-V), Release runtime, comparing
mainagainst this branch.Official CsWinRT benchmarks (
ProjectedConstructionPerf)Run against both CsWinRT 2.3.0-prerelease and 3.0.0-preview on the same runtime build, twice per configuration. Mean of 2 runs, in µs:
Allocations are unchanged throughout.
WithStringbarely moves because it is dominated by HSTRING marshalling.Sustained construction (10k instances per invocation)
A loop variant that constructs 10,000 projected objects per invocation, so RCWs from earlier iterations are being finalized while the loop is still running. The projected classes call
GC.AddMemoryPressure, which makes the GC run frequently. This is the scenario the change targets most directly. Mean of 2 runs:The GC counters show the mechanism. Gen2 collections per 1000 operations:
Gen0 and Gen1 track the same way. With a single lock, the finalizer thread stalls acquiring the write lock, so dead RCWs sit in the finalization queue long enough to be promoted instead of dying in gen0. Partitioning lets the finalizer drain faster, so fewer objects survive — a reduction in GC count on top of the reduction in lock waiting.
CsWinRT 3.0 benefits less here precisely because it already produces roughly 3x less GC pressure than 2.x (0.074 vs 0.218 gen2 collections), so there was less finalizer traffic to contend with in the first place.
Microbenchmark: lookup throughput
Steady-state cache hits through
ComWrappers.GetOrCreateObjectForComInstance, with rooted RCWs (source in the collapsed section below):Up to 13.6x faster under contention. Note the ~60 ns floor is dominated by the
QueryInterfacetransition in the harness, so the cache-specific deltas are understated in relative terms.Tradeoffs
Two costs, both intentional and called out explicitly:
1. Slightly slower uncontended lookups with a large working set. 1–3% for small working sets, growing to ~11% with 1024 live RCWs. This is cache footprint: each lookup now touches one of N locks and N dictionaries rather than always the same one. Reducing the bucket count would trade some of the contention win to shrink this.
2. More memory per
ComWrappersinstance. Measured 576 B → 6,536 B on a 32-core machine (oneReaderWriterLockSlim+ oneDictionaryper bucket, ~186 B/bucket). This scales withProcessorCount, so it is larger on bigger machines. Most processes have very fewComWrappersinstances, but that is worth confirming for this to be a good default. Allocating buckets lazily would avoid paying it for CCW-only orUniqueInstanceusage that never touches the RCW cache, at the cost of a branch on the lookup path.Both the bucket count and eager-vs-lazy allocation are easy to adjust — happy to tune based on feedback.
Testing
System.Runtime.InteropServices.TestsandComInterfaceGenerator.Tests) against a Checked CoreLib, soDebug.Assertis active — including the assert inRegisterWrapperForObjectthat verifies the RCW cache maps the identity to the expected proxy, which directly validates bucket selection.src/tests/Interop/COM/ComWrappersruntime tests pass. ThreeGlobalInstancetests fail identically onmainand on this branch in my environment (8007007E, COM class factory registration), so they are pre-existing and unrelated.ComWrappersNoLockAroundQueryInterfacewas updated. That test is a regression test asserting no cache lock is held across aQueryInterfacecallback. It made its nested cross-thread call using a different COM pointer, which with partitioning would only map to the same bucket by chance (~1/32), so it would have quietly stopped catching the regression it exists for. It now reuses the same identity pointer, which guarantees the same bucket, and additionally exercises the "lost the race" path inRegisterObjectForComInstance. It also now asserts the timedThread.Joinactually succeeded rather than ignoring the result (a timeout previously still passed). The join result is recorded and asserted by the caller rather than inside the callback, since that callback is invoked through the COM ABI and a managed exception cannot propagate through it.Standalone benchmark source (no WinRT, runs cross-platform)
Note
Parts of this pull request description were generated with GitHub Copilot. All benchmark numbers in it were measured locally and are reproducible with the sources linked above.