-
Notifications
You must be signed in to change notification settings - Fork 479
Convert dispatch_workq from legacy priorities to qos #253
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
Conversation
src/event/workqueue.c
Outdated
| } dispatch_workq_monitor_s, *dispatch_workq_monitor_t; | ||
|
|
||
| static dispatch_workq_monitor_s _dispatch_workq_monitors[WORKQ_NUM_PRIORITIES]; | ||
| static dispatch_workq_monitor_s _dispatch_workq_monitors[DISPATCH_QOS_MAX+1]; |
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.
do you actually need the 0 slot (DISPATCH_QOS_UNSPECIFIED) or is this just to make it easier by avoiding the index conversion?
elsewhere we have DISPATCH_QOS_MAX for similar array sizes (e.g. _dispatch_narrowing_deadlines), so this would be worth a comment if so
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.
just to avoid index conversion, but I will change to match convention used elsewhere.
src/event/workqueue.c
Outdated
| // temporary until we switch over to QoS based interface. | ||
| static dispatch_queue_t | ||
| get_root_queue_from_legacy_priority(int priority) | ||
| get_root_queue_from_dispatch_qos(dispatch_qos_t qos) |
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.
this seems effectively the same as _dispatch_get_root_queue now ? (inline_internal.h)
can we not use that here ?
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.
thanks! I thought there must be a routine that did this somewhere, but I missed finding it.
src/queue.c
Outdated
| #if DISPATCH_USE_WORKQUEUES | ||
| qos_class_t dgq_qos; | ||
| int dgq_wq_priority, dgq_wq_options; | ||
| #if HAVE_PTHREAD_WORKQUEUES |
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 this could be conditionalized more tightly with HAVE_PTHREAD_WORKQUEUE_SETDISPATCH_NP || DISPATCH_USE_LEGACY_WORKQUEUE_FALLBACK now (or add a define at the top for this combination to make it cleaner), those are the only remaining cases that need this field)
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.
will give it a try
c175b6a to
11eccaf
Compare
Update dispatch_workq (DISPATCH_USE_INTERNAL_WORKQUEUE) to use QoS-based constants instead of legacy priorities. Enhance monitoring code to count runnable threads from highest QoS to lowest and to suppress voluntary oversubscription for lower QoS queues if the total count of runnable worker threads is already over the desired threshold.
11eccaf to
930ff62
Compare
|
updated to address review comments. I'm not in a rush to get this merged, so fine with me if you need to put on the back burner until libdispatch-890 merge goes through. |
|
lgtm, thanks! yes, lets please wait until libdispatch-890 can be merged (which will also create a small conflict here) |
|
@swift-ci please test |
Convert dispatch_workq from legacy priorities to qos Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
Update dispatch_workq (DISPATCH_USE_INTERNAL_WORKQUEUE)
to use QoS-based constants instead of legacy priorities.
Enhance monitoring code to count runnable threads from
highest QoS to lowest and to suppress voluntary
oversubscription for lower QoS queues if the total count
of runnable worker threads is already over the desired
threshold.