-
Notifications
You must be signed in to change notification settings - Fork 461
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
Basic Linux and Windows Thread Prioritisation for worker threads #842
base: main
Are you sure you want to change the base?
Conversation
We can probably move all platform-dependent thread prioritisation logic into a new function like |
The high-priority class should be reserved for threads that must respond to time-critical events, user input threads should be THREAD_PRIORITY_NORMAL.
This function is a wrapper around SetThreadDescription, which accepts UTF-8 strings.
|
||
if (likely(wcstr != NULL)) { | ||
SetThreadDescription(hThread, wcstr); | ||
free(wcstr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should just do this unconditionally. free(NULL)
is well defined. We could even simplify that to then:
if (result == 0) {
assert(wcstr);
SetThreadDescription(hThread, wcstr);
}
free(wcstr);
This PR implements basic thread prioritisation on Linux and Windows and fixes a bug in
dispatch_get_global_queue
where the wrong QOS type is used.Note that this only sets the initial thread priority by translating QoS to thread priorities. Relative priorities are ignored for now. Later QoS overrides and other priority adjustments have no effect.
See the comments for implementation details.
Result on Windows
hugo@YellowBox MSYS /tmp # ./a.exe priority 0 descr com.apple.root.default-qos priority -1 descr com.apple.root.utility-qos priority -4 descr com.apple.root.background-qos priority 1 descr com.apple.root.user-initiated-qos
Result on Linux