perf(runtime): reduce callback tracking contention

[ReubenBond]

Summary

• Uses a 128-stripe callback dictionary for the silo runtime, keyed by the correlation ID generated by its singleton MessageFactory.
• Keeps the external client on its correlation-ID ConcurrentDictionary, preserving that path's lower lookup and mutation cost.
• Uses System.Threading.Lock on .NET 9+ for short striped critical sections and monitor locking on .NET 8.
• Uses pooled, value-only stripe snapshots and stateful predicates for allocation-free timeout, shutdown, and request-count scans.
• Closes request admission before shutdown sweeps.

Rationale

MessageFactory combines a fixed per-host nonce with a process-wide atomic counter. For one InsideRuntimeClient, this produces a unique correlation ID for every request, and responses preserve that identity. The GrainId formerly included in the callback key was therefore redundant: removing it avoids synthesized tuple-key hashing, reduces dictionary entry size, and leaves callback routing semantics unchanged.

Performance validation

Measurements compared main (3c46ba6ea95f5d7867aeca42ee91fa6b7b6a6771) with PR head (9c37228ff60455ab3d2150d56149e98b8a16d959) using BenchmarkDotNet 0.15.8 on .NET 10.0.11 / SDK 10.0.400, Windows 11 build 26200, AMD EPYC 7763 VM with 16 physical / 32 logical cores. Final runs used 15 measured iterations and symmetric invocation counts. The branch is currently rebased onto main (ef3e57d313830a330b178fdef857a26e0391548d) at head f7c990e2c59a07984258fc3900a3bec4786ad4be; the intervening changes do not alter the measured callback mutation, lookup, or scan implementation. The final follow-up removes the captured predicate allocation from request counting and has focused zero-allocation coverage.

Workload	main	PR	Ratio	Allocation
Silo register + complete, consecutive, 1 thread	136.06 ms	46.99 ms	0.35x	80 B/callback → 0 B
Silo register + complete, stride-128, 1 thread	130.23 ms	47.43 ms	0.36x	80 B/callback → 0 B
Silo register + complete, consecutive, 32 threads	31.56 ms	31.73 ms	1.01x	80 B/callback → 0 B
Silo register + complete, stride-128, 32 threads	31.27 ms	31.24 ms	1.00x	80 B/callback → 0 B
Silo status lookup, 1 thread	6.688 ms	6.128 ms	0.92x	unchanged
Silo timeout/shutdown scan, 8,192 entries	82.29 μs	52.43 μs	0.64x	0 B
Silo predicate count, 8,192 entries	82.43 μs	10.17 μs	0.12x	0 B
Silo count, 8,192 entries	9.525 μs	0.759 μs	0.08x	0 B

Mutation batches contain 1,048,576 callback lifecycles; status batches contain 262,144 lookups. Status lookup is statistically equivalent at the 5% threshold with an 8% lower mean. A direct comparison with the previous owner-key PR head (963842c61) found the ID-only key 46% faster for single-thread mutation and 31% faster for single-thread status lookup, with statistically equivalent 32-thread saturated throughput.

pvanalyze confirms the cause: the previous owner-key implementation spent 10.5% of sampled CPU in synthesized CallbackKey.GetHashCode during the single-thread mutation workload and completed 205 batches in 20 seconds. The ID-only implementation removed that frame and completed 441 batches. Under the synthetic 32-worker workload, fixed stripes still produce more contention events than a grown ConcurrentDictionary, while measured throughput remains neutral and callback allocation falls to zero. Increasing to 256 stripes reduced contention events but did not improve throughput and degraded scan performance, so the implementation retains 128 stripes.

Microsoft Reviewers: Open in CodeFlow

Stripe count validation

A scratch BenchmarkDotNet sweep compared 8, 16, 32, 64, 128, and 256 stripes using the callback mutation, lookup, scan, allocation, and retained-memory workloads. The 15-iteration finalists were 32, 64, and 128 stripes. Although 64 stripes nominally matched 128-stripe saturated throughput, repeated runs showed multimodal slowdown and materially more lock contention. Compared with 64 stripes, 128 retains approximately 10 KB more memory while empty, retains approximately 20 KB less memory at 8,192 populated entries, and adds only approximately 5.5 us to a periodic full scan. Increasing to 256 stripes provided no throughput benefit and further increased empty memory and scan cost. The implementation therefore retains 128 stripes for stable saturation behavior and contention headroom.

[Copilot]

Pull request overview

This PR reduces contention in callback tracking by replacing the existing callback dictionaries with a striped implementation and by embedding a stripe selector into CorrelationId values generated for outgoing requests.

Changes:

• Introduce StripedCallbackDictionary<TValue> for callback tracking with per-stripe locking.
• Update inside/outside runtime clients to use the striped dictionary keyed by CorrelationId.
• Update correlation id generation to encode stripe bits into the upper bits of the id (and adjust CorrelationId.GetHashCode()).

Show a summary per file

File	Description
src/Orleans.Runtime/Core/InsideRuntimeClient.cs	Switch callback tracking to StripedCallbackDictionary and simplify keying/removal to CorrelationId.
src/Orleans.Core/Runtime/OutsideRuntimeClient.cs	Switch client-side callback tracking to StripedCallbackDictionary and update request counting.
src/Orleans.Core/Messaging/StripedCallbackDictionary.cs	Add new striped dictionary implementation (per-stripe lock + snapshot enumeration).
src/Orleans.Core/Messaging/MessageFactory.cs	Encode stripe index into generated CorrelationId values.
src/Orleans.Core/Messaging/CorrelationId.cs	Update hash code implementation for CorrelationId.

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 3

[Copilot]

Review details

Suppressed comments (1)

src/Orleans.Core/Messaging/MessageFactory.cs:53

• CorrelationId generation now embeds stripe bits (via StripedCallbackDictionary.CreateCorrelationId/GetCurrentThreadStripeIndex), but there are no targeted unit tests validating stripe encoding/decoding and basic striped-dictionary semantics (add/get/remove across stripes). Given this change is intended to reduce contention, adding focused concurrency/unit tests would help prevent regressions.

        private CorrelationId GetNextCorrelationId()
        {
            var id = _seed ^ Interlocked.Increment(ref _nextId);
            var stripeIndex = StripedCallbackDictionary<object>.GetCurrentThreadStripeIndex();
            return StripedCallbackDictionary<object>.CreateCorrelationId(unchecked((long)id), stripeIndex);

• Files reviewed: 5/5 changed files
• Comments generated: 5
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (3)

src/Orleans.Core/Runtime/OutsideRuntimeClient.cs:304

• SendRequest no longer checks for client shutdown in-flight (the previous _isStopping gating was removed). StopAsync completes callbacks and then disposes the callback expiry timer; if SendRequest registers a callback after StopAsync begins, that callback may never be completed (no host-shutdown sweep for it, and no expiry monitor after the timer is disposed), potentially hanging user awaits during shutdown/disconnect.

            if (!oneWay)
            {
                var callbackData = new CallbackData(this.sharedCallbackData, context, message, _applicationRequestInstruments);
                callbackData.SubscribeForCancellation(cancellationToken);
                callbacks.TryAdd(message.Id, callbackData);
            }
            else
            {
                context?.Complete();
            }

            LogSendingMessage(logger, message);
            MessageCenter.SendMessage(message);
        }

src/Orleans.Runtime/Core/InsideRuntimeClient.cs:180

• SendRequest no longer has a shutdown/admission gate (the previous _isStopping checks were removed). That means a request can register a callback (callbacks.TryAdd) while OnRuntimeInitializeStop is running or after the shutdown sweep has started. Since OnRuntimeInitializeStop then disposes the callback expiry timer, any callbacks added after the sweep can remain uncompleted indefinitely (no timeout processing, and no host-shutdown completion), leading to hung grain calls during shutdown.

            var oneWay = (options & InvokeMethodOptions.OneWay) != 0;
            if (!oneWay)
            {
                Debug.Assert(context is not null);

                // Register a callback for the request.
                var callbackData = new CallbackData(sharedData, context, message, _applicationRequestInstruments);
                callbacks.TryAdd(message.Id, callbackData);
                callbackData.SubscribeForCancellation(cancellationToken);

src/Orleans.Core/Runtime/OutsideRuntimeClient.cs:428

• Dispose() no longer faults/completes outstanding callbacks (BreakOutstandingMessages was removed). If the client is disposed without StopAsync (or if Dispose runs during a failed startup/reset), in-flight requests can remain permanently incomplete because the expiry monitor is stopped and no host-shutdown completion is triggered.

        public void Dispose()
        {
            if (this.disposing) return;
            this.disposing = true;

            Utils.SafeExecute(() => this.callbackTimer.Dispose());

            Utils.SafeExecute(() => MessageCenter?.Dispose());

            GC.SuppressFinalize(this);
            disposed = true;
        }

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (8)

Previously missed (3) — in code that hasn't changed since the last review.

src/Orleans.Runtime/Core/InsideRuntimeClient.cs:21

• Avoid disabling nullable analysis for this file. The repo enables nullable and treats nullable warnings as errors (Directory.Build.props:27-38), so #nullable disable here is masking mismatches (eg, SendRequest takes IResponseCompletionSource while IRuntimeClient.SendRequest is IResponseCompletionSource?) and null-state issues (eg, RuntimeContext.Current is compared to null). Prefer keeping nullable enabled and fixing annotations/initialization (eg, use ? for truly nullable fields, null! for DI-initialized fields, or align method signatures with the interface).

This issue also appears in the following locations of the same file:

• line 123
• line 500

#nullable disable

src/Orleans.Core/Runtime/OutsideRuntimeClient.cs:19

• Avoid disabling nullable analysis for this file. The repo enables nullable and treats nullable warnings as errors (Directory.Build.props:27-38), so #nullable disable is masking null-state issues and contract mismatches (eg, IRuntimeClient.SendRequest allows IResponseCompletionSource? but this implementation takes non-nullable and still uses context?.Complete()). Prefer keeping nullable enabled and fixing annotations/initialization instead of turning analysis off for the entire file.

This issue also appears in the following locations of the same file:

• line 158
• line 263
• line 417

#nullable disable

src/Orleans.Core/Messaging/StripedCallbackDictionary.cs:266

• ReturnSnapshot always returns pooled arrays with clearArray: true. This forces clearing the entire rented array even when the element type contains no references, adding avoidable overhead in hot-path enumeration (eg, callback expiry scans).

                ArrayPool<KeyValuePair<CorrelationId, TValue>>.Shared.Return(snapshot, clearArray: true);

src/Orleans.Runtime/Core/InsideRuntimeClient.cs:503

• OnRuntimeInitializeStop calls callback.OnHostShutdown() without exception handling. If a callback implementation throws, shutdown can be interrupted, leaving other callbacks uncompleted and skipping timer disposal/await.

            foreach (var (_, callback) in callbacks)
            {
                callback.OnHostShutdown();
            }

src/Orleans.Core/Runtime/OutsideRuntimeClient.cs:161

• StopAsync calls callback.OnHostShutdown() without exception handling. If a callback throws, shutdown can be interrupted, leaving other callbacks uncompleted and skipping timer disposal/await/MessageCenter stop.

            foreach (var (_, callback) in callbacks)
            {
                callback.OnHostShutdown();
            }

src/Orleans.Core/Runtime/OutsideRuntimeClient.cs:424

• Dispose() no longer completes/faults outstanding callbacks (it only disposes the timer and MessageCenter). If Dispose is called without StopAsync, any in-flight requests can hang indefinitely waiting on their callbacks.

        public void Dispose()
        {
            if (this.disposing) return;
            this.disposing = true;

            Utils.SafeExecute(() => this.callbackTimer.Dispose());

            Utils.SafeExecute(() => MessageCenter?.Dispose());

src/Orleans.Core/Runtime/OutsideRuntimeClient.cs:266

• SendRequest can run concurrently with StopAsync because StopAsync does not close admission (it doesn't set disposing/disposed until later via ConstructorReset/Dispose). That allows new callbacks to be added while shutdown is completing existing callbacks and stopping MessageCenter, which can leave those new callbacks permanently incomplete.

        public void SendRequest(GrainReference target, IInvokable request, IResponseCompletionSource context, InvokeMethodOptions options)
        {
            ThrowIfDisposed();
            var cancellationToken = request.GetCancellationToken();

src/Orleans.Runtime/Core/InsideRuntimeClient.cs:131

• SendRequest registers callbacks without any shutdown/admission guard. During silo shutdown, OnRuntimeInitializeStop completes existing callbacks, but concurrent new SendRequest calls can still add callbacks after the sweep, potentially leaving them incomplete when messaging is stopping.

        public void SendRequest(
            GrainReference target,
            IInvokable request,
            IResponseCompletionSource context,
            InvokeMethodOptions options)
        {
            var cancellationToken = request.GetCancellationToken();
            cancellationToken.ThrowIfCancellationRequested();

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Lite

[ReubenBond]

The retried .NET CI workflow confirms PR-specific lifecycle regressions across Windows, Linux, and macOS on both TFMs. LateRejectionAfterSiloDisposalDoesNotResolveServices fails consistently, StoppedClient_WithInFlightCall_FaultsInsteadOfHanging times out after 60 seconds, and KilledSilo_WithInFlightOutboundCall_DisposesPromptly times out after 90 seconds. These failures recur on retry and need a callback/shutdown lifecycle fix rather than another rerun. Run: https://github.com/dotnet/orleans/actions/runs/32134600035

[Copilot]

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Copilot review overview

Review tier: Lite
Findings: 1 · 1

New issues introduced by this change (1)

Severity	Finding
	src/​Orleans.Runtime/​Core/​InsideRuntimeClient.cs — The PR description says the silo callback table retains the existing (GrainId, CorrelationId)…

Pre-existing issues (1)

Severity	Finding
	src/​Orleans.Core/​Messaging/​StripedCallbackDictionary.cs — System.Threading.Lock usage in this repo is typically guarded by NET10_0_OR_GREATER (e.g.,… View comment

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/Orleans.Core/Messaging/StripedCallbackDictionary.cs:171

• ForEach() returns pooled arrays with clearArray=true for reference-containing TValue, which clears the entire rented buffer (often larger than snapshotCount). Clearing only the populated range avoids unnecessary work while still preventing reference retention before returning to the pool.

                if (snapshot is not null)
                {
                    ArrayPool<TValue>.Shared.Return(
                        snapshot,
                        clearArray: RuntimeHelpers.IsReferenceOrContainsReferences<TValue>());
                }

[Copilot]

Copilot review overview

Review tier: Lite
Findings: 1 · 1 · 1

New issues introduced by this change (1)

Severity	Finding
	src/​Orleans.Runtime/​Core/​InsideRuntimeClient.cs — GetRunningRequestsCount currently allocates due to the capturing lambda passed to CountWhere…

Pre-existing issues (2)

Severity	Finding
	src/​Orleans.Core/​Messaging/​StripedCallbackDictionary.cs — System.Threading.Lock usage in this repo is typically guarded by NET10_0_OR_GREATER (e.g.,… View comment
	src/​Orleans.Runtime/​Core/​InsideRuntimeClient.cs — The PR description says the silo callback table retains the existing (GrainId, CorrelationId)… View comment

Suppressed comments (1)

src/Orleans.Core/Messaging/StripedCallbackDictionary.cs:182

• System.Threading.Lock is guarded by #if NET9_0_OR_GREATER, but other usages in this repo (e.g. LocalReminderService.cs) guard it with NET10_0_OR_GREATER. Using NET10_0_OR_GREATER here would better match the target frameworks (net8.0;net10.0) and avoids a potential compile break if net9.0 is added later but System.Threading.Lock is not available there.

#if NET9_0_OR_GREATER
        public readonly System.Threading.Lock Lock = new();
#else
        public readonly object Lock = new();
#endif

[Copilot]

Copilot review overview

Review tier: Lite
Findings: None

Issues resolved since last review (3)

Severity	Finding
	src/​Orleans.Runtime/​Core/​InsideRuntimeClient.cs — GetRunningRequestsCount currently allocates due to the capturing lambda passed to CountWhere… View resolved comment
	src/​Orleans.Runtime/​Core/​InsideRuntimeClient.cs — The PR description says the silo callback table retains the existing (GrainId, CorrelationId)… View resolved comment
	src/​Orleans.Core/​Messaging/​StripedCallbackDictionary.cs — System.Threading.Lock usage in this repo is typically guarded by NET10_0_OR_GREATER (e.g.,… View resolved comment

[Copilot]

Copilot review overview

Review tier: Lite
Findings: None