Fix #6; Support tasks submitted from a thread that is not managed by the threadpool - #51
Merged
Merged
Conversation
Contributor
|
I suggest you have each worker check the injection every 31 or 61 tasks retrieve to maintain some fairness. This is what is used in Go scheduler: slide 77 https://assets.ctfassets.net/oxjq45e8ilak/48lwQdnyDJr2O64KUsUB5V/5d8343da0119045c4b26eb65a83e786f/100545_516729073_DMITRII_VIUKOV_Go_scheduler_Implementing_language_with_lightweight_concurrency.pdf |
cannot do anything about it
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
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.
Fixes #6
Adds an
injectQueuewhich any thread can use to add tasks. After consuming local tasks, before trying to steal, tasks are drained frominjectQueueinto the local queue and processed; ~LIFO order is maintained.One flaw of this approach is if the tasks keep spawning tasks faster than they complete and so all workers are permanently busy, the tasks in inject-queue won't run. They only run if at some point one of the workers has no pending tasks. The injection-queue is drained every 61 processed local tasks to avoid this.
On my machine current benchmarks show no difference.
Added 2 benchs. The
taskpool_spc_externalcan be used to compare VS thetaskpool_spcqueueing to local queue. Theiqs_latency/taskpool_iqs_latency.nimshows latency of injected tasks when the local queues are permanently filled.there is a race cond between a thread adding a task and worker checking the injection queue + parking (if worker see no tasks in the queue, thread adds it, worker parks). But it also seems to exist for local queue + parking. It requires #54 to fix it properly (the sleepy / sleep ticket feature);
also requires wakeAll to avoid the notification per worker.waking one eventually awakes the rest on drain/steal.Related #9