Open jonhoo opened 5 years ago
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 write_frame
is there since the T
s might not be strings, e.g. in my case they wouln't be printable by themselves as they contain IDs to interned strings, hence the customizable writer.
@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 add_stack_trace
method isn't meant to be called only once per a unique stack trace, but once every sample, so it can be internally collated and processed in whatever way you'd see fit. (I guess add_sample
would be a better name for it; sorry, it was late and I was tired.)
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 Clone
?
Actually, thinking some more about it, that interface looks more like what you'd pass to collapse
than to flamegraph
. collapse
would then collapse the samples into their counts, and then pass that to flamegraph
. Keeping that interface generic I think will be tricky. One alternative would be to include the collapsed count in add_stack_trace
, and then you could totally have it be the interface to flamegraph
.
Yes, it does look like an interface to collapse
, and that's deliberate. The distinction between collapsing the stack traces and generating the flamegraph is somewhat artificial I think - it definitely makes sense for standalone scripts since you want multiple collapse
-like scripts to normalize the input (which can be of various different formats) into a common format and one flamegraph
script to generate the desired output, however for an API you'd ideally want to just throw a bunch of stack traces and have the library massage it in whatever way it deems necessary.
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
Clone
?
You don't need to make the iterator Clone
; it'd be enough to mark the T
with Clone
(since those are either going to be integers, or &str
, which are both cheap to copy), and then you could whatever with those, e.g. push those frames to a Vec
, and then you can iterate over as many times as you'd like (and you can cache the Vec
inside of FlameBuilder
, so it'd be essentially free).
I was more thinking if we don't want to (potentially re-)allocate a Vec
for each stack. I'd be totally fine making what you propose the interface to collapse, and then the same but with counts the interface for flamegraph, and then also provide an interface that's the same as the one to collapse but that also does the flamegraphing (which is then what you'd use).
I'd be totally fine making what you propose the interface to collapse, and then the same but with counts the interface for flamegraph, and then also provide an interface that's the same as the one to collapse but that also does the flamegraphing (which is then what you'd use).
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 add_sample
call differs depending on whether you're just trying to collapse or also wanting to produce a flame graph, so I suspect we'll want them to be separate. I also don't think that'll come at too much of a cost, since collapsing will need a separate "pass" anyway.
I was thinking about something similar but would like to suggest taking it one step further:
1) parse (reads the data and emits stacks) but doesn't combine - data gets streamed. 2) collapses stack traces (takes data from 1 and combines them) 3) sort 4) render
Now, where I'm going with this is the ability to create a live view from this. we could have cargo flamegraph --serve
that opens a port and lets you see the flame graph building up live in the browser as your system runs.
And I imagine the same interface, as it is more split apart will also work for other tools or am I missing something?
@koute would something like sketched out in #98 work for noperf?
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)?