Add progress reporting during heap enumeration#5763
Open
danmoseley wants to merge 3 commits intodotnet:mainfrom
Open
Add progress reporting during heap enumeration#5763danmoseley wants to merge 3 commits intodotnet:mainfrom
danmoseley wants to merge 3 commits intodotnet:mainfrom
Conversation
Commands like verifyheap and dumpheap -stat walk the entire managed heap but produce zero output during the walk, which on large dumps (e.g., 19 GB, 248M objects) can mean 7+ minutes of silence indistinguishable from a hang. Add a ProgressCallback property to HeapWithFilters that fires periodically (default every 10 seconds) during EnumerateFilteredObjects(), reporting bytes scanned vs. total heap size as a percentage. Enable it in: - VerifyHeapCommand (always, since it never streams output) - DumpHeapCommand (when -stat is used, since output is deferred) Extract timing logic into a ProgressReporter helper class with unit tests. Fixes dotnet#5760 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
One thing not mentioned above -- while a user may know to wait without feedback, the Copilot CLI (at least for me) did not. So this may be more intersting to fix than before. I'm assuming that changing the console output here is not considered a breaking change... |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds periodic progress reporting during managed heap enumeration to avoid long “silent” periods in verifyheap and dumpheap -stat, using a time-gated reporter and a callback hook in the heap enumeration pipeline.
Changes:
- Added
ProgressReporterhelper and integrated it intoHeapWithFilters.EnumerateFilteredObjects()via an optionalProgressCallback. - Enabled progress reporting for
verifyheap(always) anddumpheap -stat(when-statis used). - Added a new unit test project with
ProgressReporterTests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Diagnostics.ExtensionCommands/HeapWithFilters.cs | Adds progress callback plumbing and reporting during object enumeration. |
| src/Microsoft.Diagnostics.ExtensionCommands/ProgressReporter.cs | Introduces time-gated progress reporting + message formatting. |
| src/Microsoft.Diagnostics.ExtensionCommands/VerifyHeapCommand.cs | Enables heap-scan progress output for verifyheap. |
| src/Microsoft.Diagnostics.ExtensionCommands/DumpHeapCommand.cs | Enables heap-scan progress output for dumpheap -stat. |
| src/Microsoft.Diagnostics.ExtensionCommands/Microsoft.Diagnostics.ExtensionCommands.csproj | Grants unit tests access to internals via InternalsVisibleTo. |
| src/tests/Microsoft.Diagnostics.ExtensionCommands.UnitTests/Microsoft.Diagnostics.ExtensionCommands.UnitTests.csproj | Adds new unit test project for progress reporting. |
| src/tests/Microsoft.Diagnostics.ExtensionCommands.UnitTests/ProgressReporterTests.cs | Adds unit tests for reporting behavior and formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...nostics.ExtensionCommands.UnitTests/Microsoft.Diagnostics.ExtensionCommands.UnitTests.csproj
Outdated
Show resolved
Hide resolved
src/tests/Microsoft.Diagnostics.ExtensionCommands.UnitTests/ProgressReporterTests.cs
Outdated
Show resolved
Hide resolved
...nostics.ExtensionCommands.UnitTests/Microsoft.Diagnostics.ExtensionCommands.UnitTests.csproj
Outdated
Show resolved
Hide resolved
…d using - Use $(SupportedXUnitTestTargetFrameworks) instead of hard-coded net8.0 - Use $(XUnitVersion) instead of hard-coded xunit 2.9.3 - Remove unused using System.Threading Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
flaky tests-
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add progress reporting during heap enumeration
Commands like
verifyheapanddumpheap -statwalk the entire managed heap but produce zero output during the walk. On large dumps (e.g., 19 GB / 248M objects), this means minutes of complete silence on both stdout and stderr, indistinguishable from a hang.This PR adds periodic progress output (every 10 seconds) during heap enumeration for the two worst offenders:
verifyheap-- 7+ minutes of silence before printing a single linedumpheap -stat-- ~80 seconds of silence before printing the statistics tableExample output
Without the fix, there would be no output at all before the
Statistics:line.Design choices
Output format: Progress lines are written via
Console.WriteLine(), producing one line every ~10 seconds that scrolls up. Two other formats were considered:. . . . .) -- minimal but gives no indication of how far along or how much is left\roverwrite -- cleanest UX in interactive terminals, but produces a single growing line when stdout is redirected to a file, and depends on terminal\rsupportWriteLineis the most portable choice; forverifyheapon this dump it produces ~43 progress lines over 7 minutes, which seems an acceptable trade-off for the information provided.Scope: Only
verifyheap(always) anddumpheap -stat(when-statis used) enable progress. Commands that already stream output continuously (dumpheapwithout-stat,finalizequeue,gchandles) don't need it.gcheapstatanddumpasyncalso have silent walks but use different enumeration paths; they can be addressed in a follow-up.Performance
The progress mechanism adds negligible overhead:
Stopwatch.ElapsedMillisecondsread per valid object (~5ns), compared to the per-object work of reading method tables, sizes, and verifying references (orders of magnitude more expensive)Console.WriteLinecall happens only once every 10 secondsList<ClrSegment>upfront (typically a handful of segments) to avoid double-enumerating the filtered segment listChanges
HeapWithFilters.cs-- AddedProgressCallbackproperty and aProgressIntervalMsconstant. When set,EnumerateFilteredObjects()computes total heap size from segment metadata and periodically invokes the callback.ProgressReporter.cs-- New helper class encapsulating the time-gated progress logic, with a staticFormatProgressMessagemethod.VerifyHeapCommand.cs-- Sets progress callback before heap walk.DumpHeapCommand.cs-- Sets progress callback when-statis used.ProgressReporterTestscovering callback timing, byte tracking, and message formatting.Test data (19.3 GB dump, 248M objects, warm cache)
verifyheapdumpheap -statFixes #5760