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

Set Tracy thread names according to Rust std thread names #38

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

infmagic2047
Copy link

Rust's std library has the concept of named threads, but the names are currently not passed to Tracy.

This PR adds set_thread_name_from_std() in tracy-client for passing the std thread names to Tracy, and does so automatically in tracing-tracy when the first span or event is handled for each thread.

self.set_thread_name(name);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is trivial for downstream code to write themselves; is it really worth having a method for it?

Copy link
Author

Choose a reason for hiding this comment

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

I put the method here to raise some awareness of the Rust thread naming API, as set_thread_name alone may imply thread naming being some Tracy-specific feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could that be solved by pure documentation instead?

@nagisa
Copy link
Owner

nagisa commented May 24, 2022

Rust's std library has the concept of named threads, but the names are currently not passed to Tracy.

That's somewhat unfortunate. Both the standard library and Tracy set up the pthread's view of the thread name (on UNIXes). Tracy however also stores these thread names in a linked list internally for, I'd imagine, faster lookups and will only look for the name there if Tracy is enabled.

This PR adds set_thread_name_from_std() in tracy-client for passing the std thread names to Tracy, and does so automatically in tracing-tracy when the first span or event is handled for each thread.

My concern with automatic setting like this is largely with just the fact that it affects thread-local state in a non-obvious place – if I end up running pthread_setname_np somewhere in my code, I would end up pretty surprised if observability instrumentation would overwrite this somehow.

I think might be a good idea is to first probe Bartosz and see if they would be interested in taking a change to Tracy that would make it attempt fetch a thread name from system in case the linked list maintained by it hasn't an entry yet. If Tracy did so, the thread names set by the Rust standard library would automagically appear in profiles, and so they would in many other cases where people don't want to use tracy functions to set thread names for whatever reason.

@infmagic2047
Copy link
Author

My concern with automatic setting like this is largely with just the fact that it affects thread-local state in a non-obvious place – if I end up running pthread_setname_np somewhere in my code, I would end up pretty surprised if observability instrumentation would overwrite this somehow.

I agree that sliently changing OS thread names is not ideal here, especially because it happens unpredictably. What we are missing here is the ability to tell Tracy about thread names without actually setting OS-level names, but I don't expect this feature getting into Tracy as there isn't a need within C++.

What about making this configurable in TracyLayer? It could be enabled by default as there probably aren't many people who use Rust thread names and also set thread names manually.

I think might be a good idea is to first probe Bartosz and see if they would be interested in taking a change to Tracy that would make it attempt fetch a thread name from system in case the linked list maintained by it hasn't an entry yet. If Tracy did so, the thread names set by the Rust standard library would automagically appear in profiles, and so they would in many other cases where people don't want to use tracy functions to set thread names for whatever reason.

I prefer a solution that does not go through the OS. On Unix systems there is a 16-byte limit on thread names, while Rust and Tracy do not have such limitations, so a roundtrip via OS may lose some information.

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.

3 participants