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

Replace pthread_self with pthread_threadid_np on macOS #33

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

SpriteOvO
Copy link
Owner

@SpriteOvO SpriteOvO commented Jul 10, 2023

This PR changes the implementation of fn get_current_tid() on macOS.

The previous used pthread_self() returns an opaque type pthread_t with a large value. Instead, pthread_threadid_np returns a system-wide unique integral ID of thread in the location specified by thread_id.

Note that pthread_threadid_np doesn't seem to exist before macOS 10.6 as mentioned from C++ spdlog comment, we can use pthread_mach_thread_np(pthread_self()) for earlier versions. But considering it's a fairly early version (wikipedia says its initial release date is August 28, 2009), I don't have a condition to test it. So ignoring this for now until a user reports it.

I ran the 3 ways of getting TID on macOS from GitHub Action and this is results:

pthread_self: 4567832064
pthread_threadid_np: 9037
pthread_mach_thread_np(pthread_self()): 259

Obviously, pthread_self returns a large value that doesn't quite match our perception of common TID. pthread_threadid_np and pthread_mach_thread_np both look good, but semantically I prefer the former, whose function name implies what we're doing.


Other projects also use pthread_threadid_np to get TID on macOS:

@SpriteOvO SpriteOvO requested a review from Lancern July 10, 2023 13:07
@SpriteOvO SpriteOvO changed the base branch from main to main-dev July 10, 2023 13:08
Copy link
Collaborator

@Lancern Lancern left a comment

Choose a reason for hiding this comment

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

LGTM.

@SpriteOvO SpriteOvO merged commit d7136db into main-dev Jul 12, 2023
@SpriteOvO SpriteOvO deleted the macos-pthread_threadid_np branch July 12, 2023 12:36
@Lancern
Copy link
Collaborator

Lancern commented Jul 12, 2023

Is the thread ID returned from pthread_threadid_np the same as the system thread ID?

@SpriteOvO
Copy link
Owner Author

SpriteOvO commented Jul 12, 2023

Is the thread ID returned from pthread_threadid_np the same as the system thread ID?

@Lancern From what I learned from Google results - macOS doesn't seem to have a definition for TID. There are different debugging tools that use different ways to get a value to use as a TID.

[blog] gettid on Mac OS
[blog] How to get the thread ID correctly?

Refer to this SO answer, pthread_threadid_np produces the same value as in the system tools.

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