New thread priority names #2752
Conversation
…hread priorities - deprecating thread_priority_critical (replaced by thread_priority_high_recursive)
hkaiser
force-pushed
the
fixing_2747
branch
2 times, most recently
from
e135d3b
to
0ab3f6f
Compare
- flyby: typo fix
|
From the comments you made on #2602 I take it that this can go ahead now, can't it? |
|
My impression so far is that this is a big improvement and it can be merged if you're in a hurry. I wanted to play some more just to check all corner cases. |
|
Ok, we'll wait for your verdict, then. |
|
The High priority tasks are now running much more predictably without being queued up behind incorrectly promoted lower priority ones. I still feel that there is some work to do on the scheduler (attached is plot showing a comparison between the local_priority_queue_scheduler and a modified one of mine for our cholesky matrix solve test). My results for scheduling are so far are inconclusive - but this branch appears to be much better than previously and we are hitting the 800Gflops mark using the HPX scheduler on a 36core daint multicore node for most of the configurations tested. |
| if (thread_priority_high_recursive == | ||
| threads::get_self_id()->get_priority()) | ||
| { | ||
| data.priority = thread_priority_high_recursive; |
There was a problem hiding this comment.
Should this check that the promoted thread is "default" or normal. If I have a child task of a high_riority_recursive and I launch it as low priority, I think it should go low and not be promoted to high.
There was a problem hiding this comment.
Right, this should additionally check whether data.priority == thread_priority_default. Which also means that we should make thread_priority_default and thread_priority_normal distinct in terms of enum values represented (just treat them in similar ways).
- also make sure high_recursive priority gets inherited only if no non-default priority is given - flyby: fix get_thread_priority_name()
|
@biddisco: Would you be able to check one more time whether my latest changes are fine with the application you're using to test, please? |
|
Seems to be working ok with the latest changes. I'm happy to see this merged ASAP. |
Introducing explicit high priority and high priority (recursive) as thread priorities, deprecating
thread_priority_critical(replaced bythread_priority_high_recursive).This fixes #2747
@biddisco Please review.