-
Notifications
You must be signed in to change notification settings - Fork 118
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
Support multiple --skip-after
#231
Conversation
perf is stopping short on some of my backtraces (I think it's related to complex C++ code in the stack), which fragments the relevant functions in the flamegraph. `--skip-after` works great to avoid this, but I want to do it for multiple functions.
84744d9
to
2fb9aa4
Compare
Codecov Report
@@ Coverage Diff @@
## master #231 +/- ##
==========================================
- Coverage 87.01% 86.85% -0.16%
==========================================
Files 17 17
Lines 2379 2396 +17
==========================================
+ Hits 2070 2081 +11
- Misses 309 315 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, and definitely sounds useful! Could you add a test with multiple skip-afters though? Or just update the current one to have multiple filters?
Done. I went with adding a new one to make sure all the branches are covered (I assume that's what codecov is saying? I haven't dug into what numbers it's reporting). |
src/collapse/perf.rs
Outdated
// without `skip_after` some collapsed lines would look like: | ||
// go;[unknown];x_cgo_notify_runtime_init_done;runtime.main;main.init;... | ||
if line.contains("main.init") { | ||
assert!(line.contains("main.init;")); // we removed the frames above "main.init" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, reading this test again. I think this should be line.starts_with
, not line.contains
. Otherwise it doesn't seem to test much?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Fixed them both, and confirmed that the tests fail if I remove things from skip_after
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks!
This follows jonhoo/inferno#231 Now we also support multiple --skip-after arguments. Update docu to reflect this. A semver bump will be required because FlamegraphOptions is public.
This follows jonhoo/inferno#231 Now we also support multiple --skip-after arguments. Update docu to reflect this. A semver bump will be required because FlamegraphOptions is public.
This follows jonhoo/inferno#231 Now we also support multiple --skip-after arguments. Update docu to reflect this. A semver bump will be required because FlamegraphOptions is public.
perf is stopping short on some of my backtraces (I think it's related to
complex C++ code in the stack), which fragments the relevant functions
in the flamegraph.
--skip-after
works great to avoid this, but I wantto do it for multiple functions.