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

multiple profiler support #69

Open
asiderop opened this issue Jan 28, 2019 · 15 comments
Open

multiple profiler support #69

asiderop opened this issue Jan 28, 2019 · 15 comments
Labels
enhancement New feature or request

Comments

@asiderop
Copy link

As has been mentioned elsewhere, a generic data file format would allow for converters to be written to support formats other than perf. Has there been any work towards this yet?

Another approach seems to be what it being implemented in the source with "native" support of various profilers, with each format having its own parsing routine. I prefer this second approach, but it seems the interface should be formalized a bit more.

Has a decision been made on which approach to take? I would like to start sending some PRs to provide support for other profilers.

@spiermar
Copy link
Contributor

Yes, working on that problem now actually, since I'm adding support for a few new formats. Nothing formalized yet, but the heatmap seems pretty straight forward, a list of offsets. The flame graph I'm still thinking about the level of abstraction, due to the range filter, but should be similar to the current nflxprofile parser. I already have a perf -> nflxprofile converter, that might replace the current perf parsing code in the future.

The ugly bit at this point is the profile type detection, which is pretty terrible right now. I'm not a big fan of the idea, but might need to go in the file extension direction soon.

If you already have an idea for the interface definition, send a PR to the README file so we can iterate over it.

@spiermar spiermar added the enhancement New feature or request label Jan 28, 2019
@spiermar spiermar self-assigned this Jan 28, 2019
@spiermar
Copy link
Contributor

@asiderop For the first approach, we will start recommending the nflxprofile format. Distributing the .proto definition for use in different languages.
For the second approach, I have a clear idea for the native interface. Just need to slightly refactor the code to be more pluggable and write the documentation.
Native support for new formats will be dealt with in PRs. PRs should include the conversion to an intermediary format and logic to identify the new format.

@kebab-mai-haddi
Copy link

@spiermar , if I get this right, we are thinking of having different formats being converted to one standardized and then applying our parsing, right? This would be the first method that @asiderop mentioned in the first comment.

@spiermar
Copy link
Contributor

spiermar commented Apr 8, 2019

There are 2 parts to this problem. The first, creating a generic format that can be used and create some examples and documentation. This is partially done, with the nflxprofile format. Examples and documentation is still missing. The second part is refactoring the code to be more pluggable, to either accept external plugins or easier to extend via PRs. I see an external plugin interface as an end state, but that will likely require the code to be easier to extend.

  1. Documentation and converter examples for nflxprofile format
  2. Refactor code so it's easier to extend to newer profile types
  3. External plugin interface

@gmarler
Copy link

gmarler commented May 13, 2019

@spiermar, thanks for referencing my request as something this issue could cover. Per your last update above, I'd like to help with any of the listed 3 efforts.

Of course, to add DTrace to the mix, it would be good to know what to produce in the DTrace stack profile output, to have a concrete example to work with for the documentation and converter work/PR.

I'm going to start by doing perf traces and see what that conversion to nflxprofile looks like, but can you recommend a course of action beyond that so that we can make progress on this?

@spiermar
Copy link
Contributor

@gmarler I already have a script to convert perf outputs to nflxprofile. See https://gist.github.com/spiermar/195286aa29d2ebbe6ecde27c1fd7e8b7.

The function takes a line iterator (iter_lines)

@spiermar
Copy link
Contributor

I can probably take care of 1 pretty quickly, with the perf example. 2 is required for 3, so probably need to start there. If you're keen on helping, I would suggest having a look into the view > controller > parser interaction to see how it could be improved, for both heatmap and flame graph endpoints.

@gmarler
Copy link

gmarler commented May 14, 2019

Thanks for the pointer to the gist. Now I have to determine how to best produce DTrace output to make the first pass at a converter easier to write.

A couple of things I'm mulling over at the moment when trying to produce similar output in DTrace to that of perf:

  1. An event in perf has the command (just execname in DTrace), and what looks like a microsecond timestamp in units of fractional seconds. Guessing I can take nanosecond timestamp in DTrace, divide by 1000 to get microsecs (integer) and make the necessary adjustments to use that as delta.
  2. DTrace has an idiom to exclude the idle threads up front, so shouldn't ever have to parse those
  3. Not sure how stacks that contain both a kernel portion and a user portion are to be handled - I've seen other FlameGraphs that put a "gap" between user and kernel portions of a single stack - don't see an example of that in the examples/ dir, but I may just not recognize it for perf output. In DTrace, one normally uses stack() and ustack() to capture and print these out as part of the same "event". Does that sound correct for FlameScope usage, or is that done differently?

The point here is to produce initial DTrace output, confirm I can convert then FlameScope/FlameGraph it, then use a few outputs as test input later on.

@spiermar
Copy link
Contributor

This might help.

@gmarler
Copy link

gmarler commented May 15, 2019

Yes, that's what I started with ;>.

Making progress on the first pass of DTrace convert, but need to clarify a couple of more things:

  1. I just noticed that perf stacks don't have counts like DTrace does - or at least doesn't have them just after the stack. I presume this means that every stack event has an implicit count of 1? Is that what you're calling "hit_count" in a Node?
  2. If that's true, is line 118 in your gist where you're incrementing that? If not, where are you tracking the occurrence count for each stack?

@spiermar
Copy link
Contributor

  1. Yes, that's the main problem, you will need a different DTrace script. Counts only don't work for FlameScope because I need the timestamps to create the heatmap, or at least counts every 20ms window if using 50 rows. Full aggregation of stacks is not enough to create the heatmap.
  2. Yes, that's what I'm doing. I have all the samples, and counting them, but just for the flame graph. For the heatmap, I'll need a count for every square in the heatmap.

@gmarler
Copy link

gmarler commented May 15, 2019

Excellent - now this is starting to make more sense.

For DTrace I'm capturing a microsecond level timestamp (nanosecond timestamp divided by 1000) for each stack as well as counting the number of occurrences, so it should be easy to also divide up the stacks into time windows for the heatmap.

I'm getting more familiar with the code, and will be able to to check what I've learned against anything you document concerning nflxprofile format.

The most useful thing now would be a description of each field in the format. That will lead to further questions as I make my way into the depths.

@spiermar
Copy link
Contributor

@gmarler fields in the nflxprofile format?

@gmarler
Copy link

gmarler commented May 16, 2019

Yes, what I presume you meant by:

  1. Documentation and converter examples for nflxprofile format

Specifically, the purpose/usage of each field defined/listed in nflxprofile.proto:

Many are self evident from other FlameGraph implementations, but not all.

@spiermar
Copy link
Contributor

Yes, plus documentation and examples for converters and code hooks.

@spiermar spiermar removed their assignment Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants