add support for bpftrace #56

bcmyers · 2019-02-22T00:49:40Z

Hopefully this does the trick. Git is still quite confusing to me. Thanks for the help!

src/bin/collapse-bpftrace.rs

src/collapse/bpftrace.rs

bcmyers · 2019-02-22T03:15:38Z

I'll investigate why Windows is failing. I now have a Virtualbox setup for Windows after all that num-format system locale business...

src/collapse/mod.rs

.gitattributes

src/bin/collapse-perf.rs

src/collapse/bpftrace.rs

src/collapse/mod.rs

src/collapse/perf.rs

bcmyers · 2019-02-22T19:15:15Z

Think I got all the comments. Let me know what you think.

src/collapse/bpftrace.rs

src/collapse/mod.rs

src/collapse/bpftrace.rs

jonhoo · 2019-02-22T20:45:56Z

Ah, @brendangregg just responded in brendangregg/FlameGraph#201 and pointed out that they're actually adding direct support for the perf output format in bpftrace (in bpftrace/bpftrace#438), so maybe we simply shouldn't make this change? I think it'd still be worthwhile to keep the trait changes though, as that'll make it much easier to adopt more formats later!

bcmyers · 2019-02-22T20:50:23Z

Ok, so pull out all the bpftrace-related stuff, but keep our trait / architecture / documentation stuff? (means I don't have to re-write the parser :))

Apparently perl scripts won't just run in travis' windows environment; so instead of running stackcollapse-bfptrace.pl in the test, let's just use it's pre-computed output.

Since hopefully inferno will eventually have as many binary applications as the original perl version (one for each "frontend" such as perf and bpftrace), this is an initial attempt at creating an abstraction that will make writing all those very similar applications easier. Also - Unrelated to the above, this commit incorporates a suggested change to the test in bptrace.rs.

…dows problem by telling git not to mess with the line endings of our test data

* Documentation * Rename `collapse_with` to `collapse_file` * Make sure git leaves the line-endings of all our data files alone * Prefer `use super::foo` to `use crate::foo` * Keep separate modules for each `Frontend` implementation * Rename `PerfOptions` back to `Options` * Remane `Perf` constructor to `with_options` (and change arg back to `opt`) * Eliminate `Bpftrace::new` * Other minor changes

bcmyers · 2019-02-22T21:11:51Z

I hope I just did that right. I still don't understand git very well.

jonhoo · 2019-02-22T21:43:34Z

Looks perfect! I'm not sure you needed to do a force-push or rebase, but it's fine either way :) Let's see what Travis says, and then merge. Sorry for all the effort you had to go through 😅

codecov-io · 2019-02-22T22:06:58Z

Codecov Report

Merging #56 into master will decrease coverage by 0.69%.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##           master      #56     +/-   ##
=========================================
- Coverage   76.48%   75.78%   -0.7%     
=========================================
  Files          13       14      +1     
  Lines        1199     1210     +11     
=========================================
  Hits          917      917             
- Misses        282      293     +11

Impacted Files	Coverage Δ
src/flamegraph/mod.rs	`82.6% <ø> (ø)`	⬆️
src/bin/collapse-perf.rs	`0% <0%> (ø)`	⬆️
src/collapse/mod.rs	`0% <0%> (ø)`
tests/collapse-perf.rs	`93.87% <100%> (+0.12%)`	⬆️
src/collapse/perf.rs	`81.37% <76.47%> (-1.2%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81b1caa...0ef871c. Read the comment docs.