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

Allow to rename thread in RealtimePublisher #176

Closed
roncapat opened this issue Oct 29, 2024 · 4 comments · Fixed by #228
Closed

Allow to rename thread in RealtimePublisher #176

roncapat opened this issue Oct 29, 2024 · 4 comments · Fixed by #228

Comments

@roncapat
Copy link
Contributor

At least in Linux, it is possible to rename a thread, via calls like ptrhead_setname_np.

For application featuring many RealtimePublishers and many other threads, diving into perf record outputs (e.g. via hotspot tool) is difficult because by default all threads have the same name (First 15 characters of the process name), and thread renaming is a powerful tool to discriminate at a glance which thread does what.

To allow this, either the implementation of RealtimePublisher shall permit to obtain the native handle (see this) or may enable for Linux only an additional method that uses pthread_setname_np.

I can submit a PR depending on maintainers feedback of course 👍🏻

@christophfroehlich
Copy link
Contributor

@saikishor what do you think?

@firesurfer
Copy link
Contributor

As I just spent some time with that class: I would recommend against both suggestions and make an alternative proposal:

We make the thread_ field protected and you can inherit from the RealtimePublisher and add whatever functionality you need.

I think this is the cleanest approach as we keep the RealtimePublisher itself free from platform specific code and avoid exposing internals.

@roncapat
Copy link
Contributor Author

Fine for me, as long as I have a way to access the thread handle :)

@firesurfer
Copy link
Contributor

@roncapat I suggest you create a PR for that ;)

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 a pull request may close this issue.

3 participants