Closed
Bug 1239290
Opened 9 years ago
Closed 7 years ago
Implement work stealing in the JobScheduler
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: nical, Assigned: nical)
References
Details
(Keywords: feature)
Attachments
(1 file, 1 obsolete file)
33.16 KB,
patch
|
Details | Diff | Splinter Review |
Currently available jobs are posted in a queue that only the workers registered to that queue can pull from, and workers are only registered to one queue. This means that some workers can end up with a lot of work to do (especially "special" workers like the one that does all of the text rendering").
I have a WIP patch that integrates the queues in the workers and adds the notion of a public queue (that other workers can steal from) and private queue (for jobs that are pinned to the worker, and are executed in priority. When a worker is out of jobs to process in its own queue it will try to steal jobs from other workers' public queues and wait only if it can't get anything to execute from there.
Assignee | ||
Comment 1•9 years ago
|
||
Assignee: nobody → nical.bugzilla
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8707411 -
Attachment is obsolete: true
Attachment #8739489 -
Flags: review?(jmuizelaar)
Comment 3•9 years ago
|
||
Comment on attachment 8739489 [details] [diff] [review]
Implement work-stealing.
Review of attachment 8739489 [details] [diff] [review]:
-----------------------------------------------------------------
It seems like this adds a lot of code to _posix.cpp and _win32.cpp. Is it not possible for us to use a useful abstraction?
Assignee | ||
Comment 4•9 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> Comment on attachment 8739489 [details] [diff] [review]
> Implement work-stealing.
>
> Review of attachment 8739489 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> It seems like this adds a lot of code to _posix.cpp and _win32.cpp. Is it
> not possible for us to use a useful abstraction?
There will be less platform-specific stuff when I replace the current queue with a lock-free implementation that does not depend on condition variables or win32 events.
In the mean time the differences between the two synchronization mechanisms are subtle enough that it'd be tricky to reuse any meaningful logic.
Also I don't think it adds that much code. The only thing this patch adds to the platform-specific logic is the worker loop which used to be shared but is rather trivial. The rest of this patch is basically merging the worker thread and the job queue abstractions which were previously separate things but already implemented per-platform.
We could also get to a point where we can remove all of the win32 stuff and only have one implementation relying on the standard library for threads, muteces and cond vars. If I remember correctly this is fine for windows xp, but not osx 10.6 but the latter should get resolved eventually.
Comment 5•9 years ago
|
||
Comment on attachment 8739489 [details] [diff] [review]
Implement work-stealing.
Review of attachment 8739489 [details] [diff] [review]:
-----------------------------------------------------------------
Can you add a benchmark that shows the benefit?
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•6 years ago
|
Attachment #8739489 -
Flags: review?(jmuizelaar)
You need to log in
before you can comment on or make changes to this bug.
Description
•