[IPC Protocol][dotnet-gcdump] Add non-lossy dotnet-gcdump mode#5886
[IPC Protocol][dotnet-gcdump] Add non-lossy dotnet-gcdump mode#5886mdh1418 wants to merge 11 commits into
Conversation
CollectTracing6 (0x0207) extends CollectTracing5 with a trailing sessionBufferMode field on the streaming-session payload: 0 = Drop (lossy circular buffer), 1 = Block (non-lossy; producers block until the reader drains). The user_events payload is unchanged. Available in .NET 11.0 and later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new non-lossy (Block) buffering mode for IPC streaming EventPipe sessions (CollectTracing6) and wires it into dotnet-gcdump collect via a new --non-lossy flag to improve reliability when collecting heap snapshots under high event volume.
Changes:
- Extend
Microsoft.Diagnostics.NETCore.Clientto support CollectTracing5/6 payload serialization (event filters + buffering mode) and command selection. - Add a
--non-lossyoption todotnet-gcdump collectand plumb it through toStartEventPipeSessionbuffering configuration. - Update IPC protocol documentation to describe CollectTracing6 and its
sessionBufferModefield.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tools/dotnet-gcdump/DotNetHeapDump/EventPipeDotNetHeapDumper.cs | Adds a nonLossy path and uses EventPipeSessionConfiguration with EventPipeBufferingMode.Block. |
| src/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs | Updates call into TryCollectMemoryGraph for the new signature. |
| src/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs | Adds --non-lossy, propagates it to collection, and prints a helpful error on unsupported runtimes. |
| src/Microsoft.Diagnostics.NETCore.Client/DiagnosticsIpc/IpcCommands.cs | Adds CollectTracing5/6 command IDs. |
| src/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/EventPipeSessionConfiguration.cs | Introduces buffering mode + V5/V6 serialization (including event filters). |
| src/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/EventPipeSession.cs | Selects CollectTracing5/6 based on filters/buffering mode and adds filter detection. |
| src/Microsoft.Diagnostics.NETCore.Client/DiagnosticsClient/EventPipeProvider.cs | Adds per-provider Event ID filtering support via EventPipeProviderEventFilter. |
| documentation/design-docs/ipc-protocol.md | Documents CollectTracing6 and the new sessionBufferMode field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
EventPipeProviderEventFilter exposes the per-provider Event ID allow/deny list that CollectTracing5 introduced (available on .NET 10+). It is opted into via a new EventPipeProvider constructor overload, leaving the original constructor signature intact for binary compatibility. A session whose providers carry a filter is started with CollectTracing5 via SerializeV5, which adds the session-type wire prefix. enable+ids is an allow-list, !enable+ids a deny-list, and !enable+empty (the default for a null filter) allows all events. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EventPipeBufferingMode {Default, Block} selects the session buffering mode, exposed through a new EventPipeSessionConfiguration constructor overload (the original constructor signatures are left intact for binary compatibility). It is serialized by SerializeV6 (the CollectTracing5 streaming payload plus a trailing sessionBufferMode). A session with a non-default buffering mode is started with CollectTracing6 (.NET 11+); Block requests non-lossy collection in which the runtime blocks producers rather than dropping events when the buffer fills.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
collect gains --non-lossy, which starts the GCHeapSnapshot session in Block buffering mode so the runtime blocks producers instead of dropping events when the buffer fills. This produces a complete gcdump on large heaps, at the cost of slower collection, and requires a target runtime that supports CollectTracing6. When the target is too old, collect catches UnsupportedCommandException and reports that non-lossy requires .NET 11+, suggesting a plain (lossy) collection instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Covers which CollectTracing command (2 through 6) is chosen for a given EventPipeSessionConfiguration, and the CollectTracing5/6 payload layout: the IpcStream session-type prefix, the per-provider event filter (allow-list, deny-list, and the null = allow-all encoding), and the trailing buffering mode that distinguishes V6 from V5. EventPipeSession.CreateStartMessage is made internal so the selection logic is testable via InternalsVisibleTo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fd1f00f to
40be140
Compare
noahfalk
left a comment
There was a problem hiding this comment.
LGTM modulo a couple comments inline.
| }; | ||
|
|
||
| private static readonly Option<bool> NonLossyOption = | ||
| new("--non-lossy") |
There was a problem hiding this comment.
I'd suggest we don't offer this as a command-line option. I assume all users will want non-lossy behavior whenever it is supported. So the tool can try CollectTracing6 and if that reports unsupported then fall back.
There was a problem hiding this comment.
If we make it default, should we still have a way to move back to lossy behavior if some workload hits issues/deadlocks with the new non-lossy behavior?
There was a problem hiding this comment.
Since the blocking behavior only occurs when there are no free buffers to acquire, without blocking mode, the events would have been dropped, which makes the GC graph reconstruction unrecoverable.
So having a toggle back to lossy seems only beneficial for the .NET process to not deadlock. I don't think we acquire multiple locks, nor hold the rt_lock across a park/wait.
I think adding an opt-out is cheap, at the risk of the option never being used, but another workaround would just be downgrading dotnet-gcdump version.
I think the larger benefit is for dotnet-trace collect to have this option as an opt-in, and dotnet-gcdump can have it as an opt-out, where each tool passes in different defaults for a bufferingMode arg into their Collect function
…rd overload Rename the lossy default buffering mode from Default to Drop to name the behavior directly. Add an EventPipeSessionConfiguration overload that takes both a rundown keyword and a buffering mode, so callers that pass a custom rundown keyword (dotnet-trace) can also opt into Block buffering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ValidateResponseMessage previously surfaced both the UnknownCommand and InvalidArgument runtime error responses as UnsupportedCommandException. Split them into UnknownCommandException (the runtime does not recognize the command id) and InvalidCommandArgumentException (the command is recognized but its arguments are rejected). Both derive from UnsupportedCommandException, so existing catch blocks are unaffected, letting callers tell "this runtime is too old for the command" apart from an argument-validation failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the --non-lossy flag with a --buffering-mode option that takes Drop or Block (also 0 or 1) and defaults to Block, so gcdump produces a complete heap dump on large heaps out of the box. The option is shared with the report command. The bool nonLossy parameters become EventPipeBufferingMode bufferingMode throughout, and the now-unused bool-rundown buffering-mode configuration overload is removed in favor of the rundown-keyword one. When the target runtime doesn't recognize the CollectTracing6 command used for Block buffering (UnknownCommandException; .NET 11+ only), the session controller warns and falls back to the default lossy buffering so a dump is still collected. A runtime that recognizes the command but rejects the payload (InvalidCommandArgumentException) surfaces normally instead of being silently downgraded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a `--buffering-mode` option (Drop or Block, also 0 or 1) to dotnet-trace collect, defaulting to Drop so the existing lossy behavior is unchanged. Passing Block requests non-lossy collection; if the target runtime is too old to support it (CollectTracing6, .NET 11+), collection fails with a clear message rather than silently falling back. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
An earlier commit added UnknownCommandException and InvalidCommandArgumentException, but the runtime's actual behavior on the EventPipe collect path makes both redundant: - A malformed or unrecognized payload (including an unknown buffering-mode value) is rejected with BadEncoding, never InvalidArgument (which is only produced by the profiler protocol), so InvalidCommandArgumentException was unreachable. - The only UnsupportedCommandException the collect path can produce is UnknownCommand, so a dedicated UnknownCommandException subtype adds nothing. Drop both subtypes: UnknownCommand again throws UnsupportedCommandException, which dotnet-gcdump catches to fall back to lossy buffering and dotnet-trace catches to error (Block) or downgrade rundown. Add a dedicated BadEncodingException - a ServerErrorException that is deliberately NOT an UnsupportedCommandException - with a clearer message, so a rejected payload surfaces as an actionable "unsupported option" error instead of a bare HRESULT and is never swept into the command-downgrade retries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
noahfalk
left a comment
There was a problem hiding this comment.
LGTM modulo a few comments inline
| /// <param name="dsrouter">The dsrouter command to use for collecting the gcdump.</param> | ||
| /// <returns></returns> | ||
| private static async Task<int> Collect(CancellationToken ct, int processId, string output, int timeout, bool verbose, string name, string diagnosticPort, string dsrouter) | ||
| private static async Task<int> Collect(CancellationToken ct, int processId, string output, int timeout, bool verbose, string name, string diagnosticPort, string dsrouter, EventPipeBufferingMode bufferingMode) |
There was a problem hiding this comment.
I'd suggest we don't add this as an option - instead just act as if the caller always specified block mode. Adding more options adds clutter in the help text and in the docs, making the tool seem more complicated. I struggle to think of a situation where a customer would ever be better off specifying the drop mode setting. If I'm being insufficiently imaginative and it turns out to be useful we could always add it later.
(for dotnet-trace the option seems to have more legitimate use so no objection to it there)
There was a problem hiding this comment.
Right, since the performance impact of blocking only occurs if events wouldve otherwise been dropped.
I can drop the option from gcdump.
Also checking that we will keep the behavior to make gcdump automatically retry with drop mode in case block mode fails on older runtimes, since we no longer give users the choice.
dotnet-gcdump always collects with non-lossy (Block) buffering, falling back to the default lossy buffering when the target runtime is too old to support it, so the buffering mode is no longer user-configurable. Remove the --buffering-mode option from collect and report and drop the bufferingMode plumbing; the gcdump session now requests Block directly and the session controller still falls back to Drop on older runtimes. Also clarify the sessionBufferMode Drop wording in the IPC protocol doc and simplify the dotnet-trace Block-unsupported message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Diagnostics counterpart to dotnet/runtime#129457 for fixing #2404
This PR proposes a new CollectTracing command
CollectTracing6that exposes a newsessionBufferModeconfiguration option to specify how an IPC Streaming EventPipe Session using buffers should handle events when buffers are full.0 - Drop (default, pre-existing behavior)
1 - Block (non-lossy, parks producer threads until buffer space is available)
Additionally, this PR introduces serializing for CollectTracing5 (non-UserEvents) and CollectTracing6, and adds a new
--non-lossyoption todotnet-gcdump collectto leverage the new non-lossy mode.