perf(runtime): improve callback registry throughput - #10913
ReubenBond wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: None
What changed in this PR
This PR optimizes Orleans runtime callback handling for local request/response flows by carrying a process-local callback identity on Message and completing callbacks directly, avoiding a shared callback-table lookup when the response remains in-process.
Changes:
- Introduces a striped, correlation-id keyed callback table and a
CallbackRegistryto support fast, contention-reduced callback lookup/removal with identity-checked cleanup. - Routes local responses through the
Message.ResponseTargetfast-path while ensuring serialization omits the process-local target (remote/forwarded responses use fallback lookup). - Adds targeted tests for callback registry behavior, striped dictionary correctness/allocation behavior, and process-local serialization semantics.
| File | Description |
|---|---|
| test/Orleans.Runtime.Tests/StripedCallbackDictionaryTests.cs | Adds correctness + allocation-focused tests for the striped callback dictionary. |
| test/Orleans.Runtime.Tests/CallbackRegistryTests.cs | Adds behavioral tests for direct-target completion, fallback lookup, stale target handling, and close behavior. |
| test/Orleans.Runtime.Tests/CallbackDataTests.cs | Updates test helper signature to align with identity-based unregister callback. |
| test/Orleans.Core.Tests/Serialization/MessageSerializerTests.cs | Verifies Message.ResponseTarget is process-local and not serialized across round-trips. |
| src/Orleans.Runtime/Core/InsideRuntimeClient.cs | Switches from ConcurrentDictionary callback tracking to CallbackRegistry and uses direct completion for local responses. |
| src/Orleans.Runtime/Core/CallbackRegistry.cs | Adds the registry which supports direct-target completion + striped fallback lookup/removal. |
| src/Orleans.Core/Runtime/SharedCallbackData.cs | Changes unregister callback signature to accept CallbackData (enabling identity-checked removal). |
| src/Orleans.Core/Runtime/OutsideRuntimeClient.cs | Updates unregister path to identity-checked removal while keeping the existing ConcurrentDictionary-based registry. |
| src/Orleans.Core/Runtime/CallbackData.cs | Updates unregister calls to pass this and adds TryDoCallback returning completion status. |
| src/Orleans.Core/Messaging/StripedCallbackDictionary.cs | Adds a 128-stripe, lock-based dictionary with snapshot enumeration and identity-checked removal. |
| src/Orleans.Core/Messaging/MessageFactory.cs | Propagates ResponseTarget onto responses created from a request. |
| src/Orleans.Core/Messaging/Message.cs | Adds a [NonSerialized] process-local ResponseTarget property on Message. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ReubenBond
changed the title
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Orleans.Runtime/Core/CallbackRegistry.cs — TryCompleteResponse returns false when the callback was found and removed but the callback had… |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Runtime/Core/CallbackRegistry.cs — TryCompleteResponse returns false when the callback was found and removed but the callback had… View resolved comment |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes a core runtime concurrency mechanism in the request/response pipeline (callback lifecycle, shutdown, and races), which warrants final human review despite strong targeted test coverage.
Review tier: Lite
Findings: None
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes core runtime request/response callback lifecycle and shutdown/timeout race behavior in highly concurrent messaging paths, warranting final human verification.
Review tier: Lite
Findings: None
63ea5fa to
40ac09b
Compare
Code coverage
Report-only conclusion: improved. The current-main baseline is commit Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities. The comparison remains report-only while normal line and branch variance is calibrated. Coverage details |
40ac09b to
81ac18b
Compare
|
|
||
| public SharedCallbackData( | ||
| Action<Message> unregister, | ||
| Action<CallbackData> unregister, |
| private readonly ILoggerFactory loggerFactory; | ||
| private readonly SiloMessagingOptions messagingOptions; | ||
| private readonly ConcurrentDictionary<(GrainId, CorrelationId), CallbackData> callbacks; | ||
| private readonly CallbackRegistry callbacks; |
Silo callback tracking currently uses a
ConcurrentDictionary<(GrainId, CorrelationId), CallbackData>, although the singletonMessageFactoryalready provides host-unique correlation IDs. The tuple key adds hashing, entry size, allocation, and contention to every silo-originated request.This change uses a correlation-ID-only 128-stripe callback registry for the silo runtime. It preserves the external client dictionary path and adds the lifecycle guarantees needed by the runtime:
Direct callback identity on
Messagewas evaluated and removed. Matched local workloads found the registry fallback equal or faster, while the extra process-local field shifted CPU into response creation without an independent throughput benefit.Exact-source validation used current
main137d9acc17830f15b13a4eb0058d6cee633cad5e, original PR head88ed24754faf0b6e2a3fb06dff7f0dac706a0378, and final headbc5f49dc8. On a 32-logical-core AMD EPYC 7763 VM with .NET SDK 10.0.400 / runtime 10.0.11, a matched 25-second hosted-local C=100 trace measured:Hosted-local fixed-concurrency, async response-chain, exception-response, one-way, silo-to-silo, and external-client controls returned callback counts to zero. Remote and external-client throughput remained within run-to-run variance.
This PR supersedes the callback-registry implementation in #10062 and should not be merged alongside it. #10886 is independent and additive: the exact
#10886 + #10913stack improved hosted-local fixed C=16/100/500 by approximately 8–9% over #10886 alone, and AdaptivePing improved from 7.30M/s to 7.97M/s (+9.1%).Microsoft Reviewers: Open in CodeFlow