Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for writing the Brendan Gregg's flamegraph format #2768

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

exyi
Copy link

@exyi exyi commented Nov 27, 2021

It's a bit frustrating to do CPU profiling with dotnet-trace on Linux as neither SpeedScope or Chromium can collapse the same stack traces together.
Simplest approach I figured out is to add support to dotnet-trace for writing a report which https://github.com/brendangregg/FlameGraph can understand.

Example flamegraph produced with this tool: https://exyi.cz/fdotnet-example.svg

It works really well with CPU sampling, not sure how to make it work to profile allocations (or other more advanced events)

@exyi exyi requested a review from a team as a code owner November 27, 2021 10:51
@davmason
Copy link
Member

davmason commented Dec 1, 2021

Hi @exyi,

Whenever we add support for new formats it becomes something we have to support long term, so before we decide to add it or not I would like to make sure it is something that makes sense to support in the tool long term, and that there are no better options.

To be clear, I am not voting yes or no to this PR, just saying that we should have a conversation first. @josalem should be part of that conversation and he is currently out of the office. We should wait for him before moving forward on this.

Thanks!

@exyi
Copy link
Author

exyi commented Dec 12, 2021

No problem, I just needed this for myself because I didn't found any alternatives so I could as well make a PR. If you figure out a better way to get flamegraphs without using Windows, I will happily switch my workflow :)

@josalem
Copy link
Contributor

josalem commented Jan 3, 2022

Thanks @exyi! I'm back in the office for a bit, so I'll look at this.

I'm wondering if we might be better served doing the entire conversion in app, i.e., the output is the SVG of the flamegraph. I need to ponder what would be the better format to support.

Do you know off the top of your head whether Brendan's tool supports any other input formats or if there are any other tools that can consume the CollapsedStacks format? If we are going to add a supported format, I would hope that it covers as many bases as possible.

@exyi
Copy link
Author

exyi commented Jan 3, 2022

Do you know off the top of your head whether Brendan's tool supports any other input formats

Oh yes, it can convert from a number of different formats: https://github.com/brendangregg/FlameGraph#2-fold-stacks, however none of these is supported by dotnet trace and producing the collapsed stacks seems to be the simplest option (plus it reduces one step in the processing)

other tools that can consume the CollapsedStacks format?

I don't know. However, going through this simple text format makes it really easy to do processing on the data using basic unix tools. For example, I'm often interested in stacks containing certain method, or I want to "trim" them to always start at the method of interest. I know tools like PerfView can do this too in the GUI, but it's not worth maintaining a Windows VM just for this.

@exyi
Copy link
Author

exyi commented Jan 3, 2022

One more thing, it seems that most tools have the conversion script in the FlameGraph repository, so it would maybe make more sense to write this contribute nettrace convertor to his repo. However, nettrace seems to be fairly complex format and there is already nice infrastructure for C#. I also could not find any specification for the nettrace format :| So in case you don't want it here, I can try to implement it in Perl (:facepalm:) if you can provide me with some kind of spec.

@noahfalk
Copy link
Member

noahfalk commented Jan 3, 2022

I also could not find any specification for the nettrace format

Didn't get a chance to look into the broader issue at this point, but on this part we have a description of the format here:
https://github.com/microsoft/perfview/blob/main/src/TraceEvent/EventPipe/EventPipeFormat.md

If anyone wanted to read the format in C# TraceEvent nuget package can do that. I think there have also been some implementations written in Go, such as the OpenTelemetry collector.

@josalem
Copy link
Contributor

josalem commented Jan 3, 2022

One more thing, it seems that most tools have the conversion script in the FlameGraph repository, so it would maybe make more sense to write this contribute nettrace convertor to his repo. However, nettrace seems to be fairly complex format and there is already nice infrastructure for C#. I also could not find any specification for the nettrace format :| So in case you don't want it here, I can try to implement it in Perl (🤦) if you can provide me with some kind of spec.

I noticed the same thing. nettrace is a binary format defined in this document: https://github.com/microsoft/perfview/blob/main/src/TraceEvent/EventPipe/EventPipeFormat.md. I'm honestly not sure how hard it would be to write a reader in perl as I've never used it for manipulating a binary file. Based on this Perl doc it seems like it might be straightforward so long as you only pull out stacks.

There is a level of processing that happens to the nettrace file before the conversion logic you are affecting happens. The CPU time of each sample is calculated using a series of heuristics. This could certainly be recreated in Perl as well.

As Noah mentioned, there have been other tooling that have implemented parsers for nettrace.

@noahfalk
Copy link
Member

I am closing old PRs that have had no progress in a long time. You are welcome to resume the work any time and open a new PR.

@noahfalk noahfalk closed this Jul 18, 2022
@exyi
Copy link
Author

exyi commented Jul 20, 2022

I'm waiting for feedback from your side. I'll happily resume the work, if you need something from my side

@tommcdon tommcdon reopened this Jul 20, 2022
@tommcdon
Copy link
Member

I'm waiting for feedback from your side. I'll happily resume the work, if you need something from my side

Thanks for letting us know! I've re-opened the PR and we apologize for closing it while the action item was on our side to review the PR. We are busy with .NET 7 runtime issues at the moment and so it may take some time before we review this.

@noahfalk
Copy link
Member

@exyi - sorry, that was my bad here. I had incorrectly assumed since the last responses were from John that it wasn't waiting on us. John unforetunately left our team recently but perhaps we can get @davmason to take a look instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants