-
Notifications
You must be signed in to change notification settings - Fork 115
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
Tidy up library interface for flamegraph (and collapse?) #30
Comments
Sounds good to me; I should be able to produce such iterator easily. I wonder though if in the long run an interface which has collapsing integrated wouldn't be better - the extra integration could probably me made more performant, and it would be potentially simpler to use. Something like this maybe: struct FlameBuilder<T> {}
impl FlameBuilder<T> where T: PartialEq + Eq + PartialOrd + Ord + ... {
fn new() -> Self {}
fn add_stack_trace<I>(&mut self, I)
where I: IntoIterator<Item = T> {}
fn generate<F>(self, output: F, write_frame: W) -> Result<()>
where F: Write, W: FnMut(&T, &mut F) -> Result<()> {}
} The |
@koute sadly the algorithm for producing the flamegraph actually requires walking through all the frames present in each stack sample. That's why I proposed the above: we need both the count for each stack sample present, and the frames of that stack sample to be able to detect and merge shared parent frames. |
@jonhoo The API in my post does allow you to walk through all the frames though. (: The |
Oh, I see. Hmm, I still don't think that's enough, as we actually need to walk each stack multiple times (twice, specifically) to do the merging. Maybe if we require that the iterator also be Actually, thinking some more about it, that interface looks more like what you'd pass to |
Yes, it does look like an interface to
You don't need to make the iterator |
I was more thinking if we don't want to (potentially re-)allocate a |
So, basically, we'd have something like this for the "collapse or directly generate a flamegraph": struct FlameBuilder<T> {}
impl FlameBuilder<T> where T: PartialEq + Eq + PartialOrd + Ord + Clone {
fn new() -> Self {}
fn add_sample<I>(&mut self, I)
where I: IntoIterator<Item = T> {}
fn collapse<F>(self, output: F, write_frame: W) -> Result<()>
where F: Write, W: FnMut(&T, &mut F) -> Result<()> {}
fn generate_flamegraph<F>(self, output: F, write_frame: W) -> Result<()>
where F: Write, W: FnMut(&T, &mut F) -> Result<()> {}
} and another (different) interface just for generating a flamegraph from already collapsed frames? |
Almost. The stuff the code has to do in response to an |
I was thinking about something similar but would like to suggest taking it one step further:
Now, where I'm going with this is the ability to create a live view from this. we could have And I imagine the same interface, as it is more split apart will also work for other tools or am I missing something? |
I'm looking at using inferno to generate flame charts, has there been any progress on a programmatic api? |
Not beyond the discussion you see in this issue, no. But if you want to take a stab at writing up a PR, I'd be happy to review it! |
I've poked around the code a bit, but I'm not really sure how things go from trace file to flamegraph, is all the data that's involved a number of samples and some strings? Shouldn't some sort of duration or something be involved too? (Disclaimer: I'm not familiar with the ins and outs of flamegraphs whatsoever so forgive me if I'm missing something) |
Sorry to be so slow to get back to you — I'm very short on spare time these days it seems. Maybe @jasonrhansen can give you some pointers? |
If we want other projects, like @koute's nperf, to start using inferno to draw their flame graphs (and perhaps even to do the collapsing), we need to provide an external interface that isn't just a purely line-based textual one like the ones used internally between
perf script
,stackcollapse-perf
, andflamegraph.pl
. Specifically, we'll want an interface that is typed, and where all (most?) of the parsing is done by the caller. I'm imagining something like:from_samples
would require that the input is given in sorted order (see #28), and then write a flame graph output towriter
. The existing code that does parsing would call from_samples with an iterator created from the lines iterator, mapped something like this:@koute: what do you think of an interface roughly like the above? You think that's an iterator that
nperf
could easily produce? What would be a good interface forcollapse
(if that's even something you might consider using)?The text was updated successfully, but these errors were encountered: