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 posix thread name #3657

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Conversation

bnason-nf
Copy link
Contributor

No description provided.

@yamt
Copy link
Collaborator

yamt commented Jul 23, 2024

if you want to name internal threads, i guess it would be more useful and flexible to make the thread functions set their names by themselves.

@bnason-nf
Copy link
Contributor Author

What I'm seeing right now is that all the WAMR threads inherit the name of the thread they were spawned from. This is confusing when debugging, so my goal with this change is to avoid that specific confusion. Do we want to do more than that? I see WAMR using threads for the public wasm_runtime_spawn_thread() API, for JIT, for debugging, and for thread-mgr. Are you suggesting that we add a public API to set the thread name (or modify the existing API), and to give the JIT/debug/thread-mgr distinctive names?

@wenyongh
Copy link
Contributor

@bnason-nf seems that you are setting the thread name for debug purpose, how about wrapping the code with BH_DEBUG macro?

#if BH_DEBUG != 0
#if defined __APPLE__
    pthread_setname_np("wamr");
#else
    pthread_setname_np(pthread_self(), "wamr");
#endif
#endif

@yamt how do you think?

@bnason-nf
Copy link
Contributor Author

@bnason-nf seems that you are setting the thread name for debug purpose, how about wrapping the code with BH_DEBUG macro?

Done.

Copy link
Contributor

@wenyongh wenyongh left a comment

Choose a reason for hiding this comment

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

LGTM

@TianlongLiang
Copy link
Collaborator

LGTM

@yamt
Copy link
Collaborator

yamt commented Jul 26, 2024

Are you suggesting that we add a public API to set the thread name (or modify the existing API), and to give the JIT/debug/thread-mgr distinctive names?

yes.

@yamt
Copy link
Collaborator

yamt commented Jul 26, 2024

@bnason-nf seems that you are setting the thread name for debug purpose, how about wrapping the code with BH_DEBUG macro?

#if BH_DEBUG != 0
#if defined __APPLE__
    pthread_setname_np("wamr");
#else
    pthread_setname_np(pthread_self(), "wamr");
#endif
#endif

@yamt how do you think?

i have no opinion on whether it should be conditional on BH_DEBUG or not.

@wenyongh
Copy link
Contributor

@bnason-nf seems that you are setting the thread name for debug purpose, how about wrapping the code with BH_DEBUG macro?

#if BH_DEBUG != 0
#if defined __APPLE__
    pthread_setname_np("wamr");
#else
    pthread_setname_np(pthread_self(), "wamr");
#endif
#endif

@yamt how do you think?

i have no opinion on whether it should be conditional on BH_DEBUG or not.

OK, maybe we can merge this PR to meet the debug requirement first and then add a public API to set the thread name in the future? @bnason-nf what's your opinion?

@bnason-nf
Copy link
Contributor Author

That sounds fine to me.

@wenyongh wenyongh merged commit a9cd8ba into bytecodealliance:main Jul 29, 2024
379 checks passed
@bnason-nf bnason-nf deleted the posix-thread-name branch July 30, 2024 16:19
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.

4 participants