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

src: set a default thread name for workers #4664

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

RafaelGSS
Copy link
Contributor

Hello,

I'm opening this to discuss if that's feasible from libuv point of view. I'm working on nodejs/node#56416 and hardcoding libuv thread names seems something reasonable (I have also talked with @santigimeno previously).

So, I'm opening it here and if I get a green light I can work on tests

Copy link
Member

@santigimeno santigimeno left a comment

Choose a reason for hiding this comment

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

This looks good to me with Saúl suggestion and tests and documentation added

@RafaelGSS RafaelGSS force-pushed the set-libuv-default-thread-names branch from 40806db to 6670a2c Compare January 6, 2025 14:10
@RafaelGSS RafaelGSS marked this pull request as ready for review January 6, 2025 14:11
@RafaelGSS
Copy link
Contributor Author

I'm currently working on tests to make it more reliable than it is nowadays (there's a chance of a thread not being called executed from the thread pool).

@RafaelGSS RafaelGSS force-pushed the set-libuv-default-thread-names branch from 82e78cc to 2d16a9d Compare January 6, 2025 21:21
Copy link
Member

@santigimeno santigimeno left a comment

Choose a reason for hiding this comment

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

LGTM. Left some nits.

@RafaelGSS RafaelGSS force-pushed the set-libuv-default-thread-names branch from 6e10efe to e0d951a Compare January 7, 2025 20:49
@santigimeno santigimeno merged commit e59e2a9 into libuv:v1.x Jan 8, 2025
40 checks passed
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