Add hints to yield_now to allow schedulers to deprioritize a yielding thread#18
Merged
Merged
Conversation
jorajeev
reviewed
Apr 13, 2021
|
|
||
| asynch::block_on(async move { | ||
| while count.load(Ordering::SeqCst) < NUM_TASKS { | ||
| asynch::yield_now().await; |
Member
There was a problem hiding this comment.
How about adding a variant of this test without the yield, showing that PCT finds the schedule where the test panics because we hit the max_steps limit?
jorajeev
reviewed
Apr 13, 2021
| .collect::<Vec<_>>(); | ||
|
|
||
| while count.load(Ordering::SeqCst) < NUM_THREADS { | ||
| thread::yield_now(); |
Member
There was a problem hiding this comment.
Same as above -- add a variant of the test that panics without the yield?
jorajeev
reviewed
Apr 13, 2021
jorajeev
left a comment
jorajeev
left a comment
Member
There was a problem hiding this comment.
Looks good -- maybe add tests that show PCT finds the unfair executions?
…ng thread Tasks that implement busy-wait loops or other infinite loops are a fairness issue for schedulers like PCT, which will run a thread indefinitely until it blocks. This change makes both sync and async versions of `yield_now` act as a hint to the scheduler that the current task should be deprioritized. Only the PCT scheduler acts on this hint, by moving the current task to the lowest priority. We could also make the DFS scheduler react to this hint by not allowing it to re-schedule the current task immediately, but that would be unsound.
jamesbornholt
force-pushed
the
pct-yield
branch
from
da10ad3 to
490b8d5
Compare
Member
Author
|
Good idea, done. |
jorajeev
approved these changes
Apr 13, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks that implement busy-wait loops or other infinite loops are a fairness
issue for schedulers like PCT, which will run a thread indefinitely until it
blocks. This change makes both sync and async versions of
yield_nowact asa hint to the scheduler that the current task should be deprioritized. Only
the PCT scheduler acts on this hint, by moving the current task to the lowest
priority. We could also make the DFS scheduler react to this hint by not
allowing it to re-schedule the current task immediately, but that would be
unsound.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.