Skip to content
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

Remove demangle option from collapsers #144

Merged
merged 2 commits into from
Sep 6, 2019

Conversation

jasonrhansen
Copy link
Collaborator

I think we should be able to remove symbolic_demangle as a dependency to help with #139. Doing so would also remove the following dependencies.

backtrace v0.3.37
backtrace-sys v0.1.31
cc v1.0.41
cpp_demangle v0.2.13
debugid v0.5.3
failure v0.1.5
failure_derive v0.1.5
glob v0.3.0
memmap v0.7.0
msvc-demangler v0.7.0
rustc-demangle v0.1.16
symbolic-common v6.1.4
symbolic-demangle v6.1.4
synstructure v0.10.2
uuid v0.7.4

#133 and #134 added to our collapsers the ability to fix partially demangled Rust symbols with no external dependencies. I originally implemented #132 to work around the same problem, but it required you to pass --no-demangle to perf script, or -xmangled to dtrace since symbolic_demangle doesn't work when the symbols are partially demangled. Now that we are able to take the mostly demangled symbols the profiling tools give us and fix them up from there, the demangle option on the collapsers seems to be unnecessary.

Do you think it makes sense to remove?

@jonhoo
Copy link
Owner

jonhoo commented Sep 5, 2019

So, my only concern with this is if using inferno to produce flamegraphs for non-Rust code. The code we wrote is specific to Rust code (I believe)?

@jasonrhansen
Copy link
Collaborator Author

I believe dtrace, perf, and sample do a good job demangling C++ symbols. I don't know about Swift symbols, which is the other language symbolic_demangle can handle.

From what I can tell, Rust symbols are being handled by these tools as if they were C++ when demangling, which gets them close, but we have to fix them up from there.

@jonhoo
Copy link
Owner

jonhoo commented Sep 5, 2019

Oh, I see what you mean -- we're still relying on the underlying tools to do most of the demangling, and then just fixing it up ourselves. Yeah, I think I'm good with that then!

`rustup component add rustfmt` is failing on FreeBSD.
It doesn't look like we're using it anyway.
@jasonrhansen
Copy link
Collaborator Author

I'm not able to see the details on Azure Pipelines to see what failed. If I click on the "View more details" link it says "Build not found." Any suggestions?

@jonhoo
Copy link
Owner

jonhoo commented Sep 5, 2019

Hmm, I think you're running into xd009642/tarpaulin#190, which I've also seen in the past. You can see the build pipeline here: https://dev.azure.com/jonhoo/jonhoo/_build/results?buildId=201&view=results. I'll try a re-run, which may invalidate that link. You should always be able to see builds through https://dev.azure.com/jonhoo/jonhoo/_build?definitionId=10&_a=summary

@jasonrhansen
Copy link
Collaborator Author

Strange, now I'm able to get to everything just fine. Earlier I couldn't see any of the builds and I tried in multiple browsers. It's too bad tarpaulin keeps segfaulting.

@jonhoo jonhoo merged commit af3cb2a into jonhoo:master Sep 6, 2019
@jasonrhansen jasonrhansen deleted the remove-demangle branch September 6, 2019 14:52
@jonhoo
Copy link
Owner

jonhoo commented Sep 11, 2019

Released in 0.9.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants