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

Add support for custom demanglers #119

Closed
wants to merge 4 commits into from

Conversation

DaniPopes
Copy link
Contributor

Adds support for custom demangler functions with the signature fn(&str, &mut Buffer) -> std::fmt::Result where Buffer is an opaque type that only exposes an implementation of std::fmt::Write.

The default implementation is moved to a standalone function, and the buffer is now cleared before any demangling is done as tracy guarantees:

When a call to ___tracy_demangle is made, previous contents of the string memory
do not need to be preserved.

Follow-up to #102.

tracy-client/src/demangle.rs Outdated Show resolved Hide resolved
// > It is expected that it will be internally reused.
// > When a call to ___tracy_demangle is made, previous contents of the string memory
// > do not need to be preserved.
static mut BUFFER: Buffer = Buffer::new();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work right in MT contexts, will it? Don't we need at least a buffer for each thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think it has to be MT-safe, the cpp implementation uses a global malloc'd pointer that's reused like we do here and it's worked fine

@@ -44,6 +44,9 @@ mod plot;
mod span;
mod state;

#[cfg(feature = "demangle")]
pub mod demangle;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also pub use crate::demangle::register_demangler (see a similar thing done with frame above.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already exported through macro_export, do you want to do this regardless?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Among other reasons it makes use crate::register_demangler work within the crate without surprises...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error[E0432]: unresolved import `crate::demangle::register_demangler`
  --> tracy-client/src/lib.rs:31:9
   |
31 | pub use crate::demangle::register_demangler;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `register_demangler` in `demangle`
   |
   = note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined

tracy-client/src/demangle.rs Outdated Show resolved Hide resolved
@nagisa
Copy link
Owner

nagisa commented Sep 23, 2024

Merged as 24a868a after a rebase.

@nagisa nagisa closed this Sep 23, 2024
@DaniPopes DaniPopes deleted the custom-demangler branch September 23, 2024 20:23
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