-
Notifications
You must be signed in to change notification settings - Fork 129
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
Add collapser for sample on macOS #133
Conversation
|
||
fn write_stack<W: Write>(&self, writer: &mut W) -> io::Result<()> { | ||
if let Some(func) = self.stack.last() { | ||
for symbol in IGNORE_SYMBOLS { |
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.
How sure are we that everyone will always want to ignore these?
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.
Not sure at all. I was just following upstream. Maybe we should add an option for this?
Depending on how the demangling is by default, it could be that it makes sense for us to just do it again? |
For |
I don't know why the |
Seems to fail on FreeBSD too? Is it just really slow? |
It's not slow at all when I run it on my computer. The whole test suite runs in 4 or 5 seconds. |
Hmm, that's very interesting. It seems to run just fine on my box too. The test does use |
The input is 125k, but the input file we use for dtrace is 1.4M and that one doesn't deadlock. |
I'll try restarting it and see if that helps. If not, maybe try adding
and some basic |
@jasonrhansen hmm, nope, looks like it got stuck on the exact same test again :/ |
I haven't yet figured out why the tests are timing out on CI, but I came up with a solution to the poorly demangled Rust symbols. Using |
I couldn't figure out how to get that test to work so I just disabled it for now. |
Codecov Report
@@ Coverage Diff @@
## master #133 +/- ##
=========================================
- Coverage 89.72% 89.52% -0.2%
=========================================
Files 8 10 +2
Lines 983 1146 +163
=========================================
+ Hits 882 1026 +144
- Misses 101 120 +19
Continue to review full report at Codecov.
|
Excellent! Thank you 🎉 |
This diverges a bit in behavior from stackcollapse-sample.awk from upstream. For some reason it was implemented in a way that it outputs a bunch of erroneous stacks with 0 samples. This Rust version doesn't do that. However, the resulting flamegraphs should be the same between the two since those lines with 0 samples will be skipped over when generating the flamegraph.
One unfortunate thing about
sample
is it has the same problem demangling Rust names asDTrace
. At least with DTrace there's a way to disable demangling so we can handle it in our collapser. I didn't add demangling to this collapser because, as far as I can tell, there's no way to disable demangling forsample
.