dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

dotnet-gcdump: report verb - #791

Merged
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout
Apr 21, 2020
Merged

dotnet-gcdump: report verb#791
josalem merged 28 commits into
dotnet:masterfrom
mustakimali:dotnet-gcdump-stdout

Conversation

@mustakimali

@mustakimalimustakimali commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

Files produced by dotnet-gcdump can not be viewed without Visual Studio or PerfView.
This PR introduces a new verb to generate report in the stdout. This will
allow viewing the results when a Windows PC isn't available.

  • dotnet gcdump report [-t heapstat] <gcdump_filename> - Reads a previously generated gcdump file and produces a heapstat report in the stdout.
  • dotnet gcdump report -p <processId> [-t heapstat] - Generate a report from a running dotnet process.

Report type (-t) is optional as only only one type of reports are produced. Other type of reports may be added on the future (#809 (comment)). I have not idea about how to get those information from the gcdump file at the moment so .I had to keep them outside of the scope of this PR.

Proposal is here: #809

@dnfclas

dnfclas commented Jan 30, 2020

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@mustakimali
mustakimali marked this pull request as ready for review January 30, 2020 22:46
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 2 times, most recently from bd897fa to 155b9f4CompareJanuary 30, 2020 23:09

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

First round of feedback. I really like this! Have you considered adding the ability to read an existing and print this? I believe with this change, all the necessary logic is available.. We would just need to add a dotnet gcdump print file.gcdump (or some other verb) command to the tool.

That's a great idea, didn't cross my mind. Would you prefer a separate PR for this or this one?

@josalem

Copy link
Copy Markdown
Contributor

Would you prefer a separate PR for this or this one?

I think it's fine to do that in another PR 😃. In preparation for that PR though, I might refactor the printing logic into its own file for reuse in another Command(the S.CommandLine API that equates to verbs).

@mustakimalimustakimali changed the title dotnet-gcdump: Allow writing the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutFeb 1, 2020
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/CollectCommandHandler.cs Outdated
@mustakimali
mustakimaliforce-pushed the dotnet-gcdump-stdout branch 3 times, most recently from 39b8ff7 to c9889ccCompareFebruary 1, 2020 01:44
@mustakimalimustakimali changed the title dotnet-gcdump: Allow reading a .gcdump file and print the data into stdoutdotnet-gcdump: Allow reading a .gcdump file and print the data into stdout cross-platformFeb 1, 2020
josalem
josalem previously approved these changes Feb 3, 2020

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to try running this locally a few times to try some scenarios out. I'm going to approve it for now, and merge it once I've had a chance to play with it a bit 😄

@josalem

Copy link
Copy Markdown
Contributor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Naming is hard! Let's settle this on another PR first. Sure I'll do that. 👍

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintCommandHandler.cs Outdated
@mustakimali

mustakimali commented Feb 7, 2020

Copy link
Copy Markdown
ContributorAuthor

As a heads up, I just chatted with @noahfalk and we want to do a little more thinking to make sure the naming for the verb and option make sense. If you have some extra cycles, could you do another PR that adds the verb and option to the gcdump spec (under docs/design-docs). That will give us a good spot to discuss the appropriate names. The code here looks good, we just need to settle on the names.

Thank you, I've had a chance to create the separate PR to include the verb option. Let me know if that works.

#809

@noahfalk

Copy link
Copy Markdown
Member

@josalem - I cleared your previous review to make it apparent that the work has been ongoing and I assume an updated review would be necessary once it finishes.

@noahfalk
noahfalk dismissed josalem’s stale reviewFebruary 26, 2020 19:15

Review is weeks old and work has been ongoing

@mustakimali
mustakimali requested a review from josalemApril 5, 2020 23:27

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/PrintReportHelper.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
Comment threadsrc/Tools/dotnet-gcdump/CommandLine/ReportCommandHandler.cs Outdated
mustakimaliand others added 4 commits April 13, 2020 15:17
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
Co-Authored-By: John Salem <josalem@microsoft.com>
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

I've noticed there seem to always bee a set of entries in the reports that look like this:

 -1 0 System.Threading.Tasks.VoidTaskResult [System.Private.CoreLib.dll]
-1 0 System.UInt16 [System.Private.CoreLib.dll]
-1 0 System.UInt32 [System.Private.CoreLib.dll]
-1 0 System.Char [System.Private.CoreLib.dll]
-1 0 System.IntPtr [System.Private.CoreLib.dll]
-1 0 System.Diagnostics.Tracing.EventSource [System.Private.CoreLib.dll]
-1 0 System.Boolean [System.Private.CoreLib.dll]
-1 0 System.IO.TextReader [System.Private.CoreLib.dll]
-1 0 System.Text.Encoding [System.Private.CoreLib.dll]
-1 0 System.IO.TextWriter [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Object> [System.Private.CoreLib.dll]
-1 0 System.SByte [System.Private.CoreLib.dll]
-1 0 System.Byte [System.Private.CoreLib.dll]
-1 0 System.Globalization.CalendarId [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureInfo> [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.IntPtr> [System.Private.CoreLib.dll]
-1 0 bucket [System.Private.CoreLib.dll]
-1 0 Entry<System.String,System.Globalization.CultureData> [System.Private.CoreLib.dll]
-1 0 Entry<System.RuntimeType,System.RuntimeType> [System.Private.CoreLib.dll]
-1 0 System.Reflection.CustomAttributeRecord [System.Private.CoreLib.dll]
-1 0 System.UInt64 [System.Private.CoreLib.dll]
-1 0 System.Text.StringOrCharArray [System.Console.dll]
-1 0 System.ConsoleKeyInfo [System.Console.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo>[] [System.Private.CoreLib.dll]
-1 0 Entry<System.Text.StringOrCharArray,System.ConsoleKeyInfo> [System.Private.CoreLib.dll]

I don't think this is necessarily something wrong with your code, but I think the fact that there are 0 count, -1 size entries is worth investigating. I'll take a look at this offline and see what I can dig up. These might be types that have been created but there aren't any "current" instances in the heap once the gcdump was finished.

Other than that, just a couple nits for this round 😄

Should I just ignore those lines from the report?

@josalem

Copy link
Copy Markdown
Contributor

Sorry for the delay! I'm currently investigating what these entries are. They appear to be type events that came across without there being any instances of the types in the GC heap. This is strange, though, since the list of types that get sent during collection should only contain the list of types that the GC saw when it was enumerating the heap. This is something that I think is worth investigating, but shouldn't necessarily block this PR. For now, let's hide these 0 count entries from the report and we can review the results of that.

@josalem

Copy link
Copy Markdown
Contributor

I just ran some simple experiments and found the following: These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI. This leads me to believe that these are innocuous. I'll dig through the PerfView code at some point in the future to see if there are any notes about what these entries mean. For now, feel free to make the reports ignore 0 count entries 😄

@mustakimali

Copy link
Copy Markdown
ContributorAuthor

These entries show up in reports made from gcdumps collected using PerfView (our control group in this case), but not in the PerfView UI.

Thanks for looking into this 🙇 , so this means Perfview UI had to do the same at some point.
I have updated the PR with the changes (the output is clean now).

@josalemjosalem left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long! Thanks for all the effort on this feature! 🚀

CC @noahfalk@sywhang@tommcdon

@josalem
josalem merged commit e6a3e4d into dotnet:masterApr 21, 2020
@mustakimali

Copy link
Copy Markdown
ContributorAuthor

Sorry this took so long! Thanks for all the effort on this feature! rocket

CC @noahfalk@sywhang@tommcdon

Thank you for helping me along the way with all the useful suggestions 👍

@mustakimali
mustakimali deleted the dotnet-gcdump-stdout branch April 21, 2020 22:57
@colotiline

Copy link
Copy Markdown

Do you have plans when report will be released?

@noahfalk

Copy link
Copy Markdown
Member

@mikem8361 was looking into releasing a new version of our dotnet-* diagnostic tools relatively soon I think? I'll let him follow up with any additional info on timeline if he has it.

If you are eager to get a version before we officially release it syncing and building this repo is hopefully straightforward too. The resulting dotnet-gcdump app is a standard .Net Core console app that can be copied to where you want it.

@mikem8361

Copy link
Copy Markdown
Contributor

I'm hoping to release the official build early next week. If you can't wait you can install the latest build this way:

dotnet tool uninstall -g dotnet-gcdump
dotnet tool install -g dotnet-gcdump --add-source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet5/nuget/v3/index.json

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jan 19, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@mustakimali@dnfclas@josalem@noahfalk@davidfowl@colotiline@mikem8361@jonsequitur