Bug 90584

Summary: Add DispatchQueue class.
Product: WebKit Reporter: Kwang Yul Seo <skyul>
Component: Web Template FrameworkAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED    
Severity: Normal CC: andersca, ap, dglazkov, dongseong.hwang, gram, gustavo, gyuyoung.kim, levin+threading, philn, rakuco, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 90375    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch buildbot: commit-queue-

Kwang Yul Seo
Reported 2012-07-04 21:33:30 PDT
DispatchQueue is a simple C++ wrapper around libdispatch (GCD). A thread pool based fallback implementation is also avaiable. So ports without libdispatch can use the same API. It provides only the small subset of the original libdispatch API. There are three types of DispatchQueue: serial, global and main. These queues correspond to the serial, concurrent global and main queue in libdispatch. DispatchQueue::async(const Function<void()>&) method is used to schedule a task regardless of the queue type. Currently, it is used by parallel image decoders to perform image decoding concurrently.
Attachments
Patch (39.62 KB, patch)
2012-07-04 21:54 PDT, Kwonjin Jeong 		no flags
DetailsFormatted DiffDiff
Patch (13.93 KB, patch)
2012-07-05 03:03 PDT, Kwonjin Jeong 		no flags
DetailsFormatted DiffDiff
Patch (41.50 KB, patch)
2012-07-05 03:16 PDT, Kwonjin Jeong 		no flags
DetailsFormatted DiffDiff
Patch (41.50 KB, patch)
2012-07-05 04:21 PDT, Kwonjin Jeong 		no flags
DetailsFormatted DiffDiff
Patch (41.49 KB, patch)
2012-07-05 04:26 PDT, Kwonjin Jeong 		no flags
DetailsFormatted DiffDiff
Patch (42.06 KB, patch)
2012-08-01 01:37 PDT, Kwonjin Jeong 		no flags
DetailsFormatted DiffDiff
Patch (41.49 KB, patch)
2012-08-01 02:27 PDT, Kwonjin Jeong 		buildbot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.
Kwonjin Jeong
Comment 1 2012-07-04 21:54:24 PDT
Created attachment 150873 [details] Patch
WebKit Review Bot
Comment 2 2012-07-04 22:22:15 PDT
Comment on attachment 150873 [details] Patch Attachment 150873 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13126985
Gyuyoung Kim
Comment 3 2012-07-04 22:33:24 PDT
Comment on attachment 150873 [details] Patch Attachment 150873 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13138848
Build Bot
Comment 4 2012-07-04 23:49:36 PDT
Comment on attachment 150873 [details] Patch Attachment 150873 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13136919
Kwonjin Jeong
Comment 5 2012-07-05 03:03:40 PDT
Created attachment 150909 [details] Patch
WebKit Review Bot
Comment 6 2012-07-05 03:06:36 PDT
Attachment 150909 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/GNUmak..." exit_code: 1 Source/WTF/wtf/chromium/MainThreadChromium.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/WTF/wtf/chromium/MainThreadChromium.cpp:65: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 7 2012-07-05 03:08:11 PDT
Comment on attachment 150909 [details] Patch Attachment 150909 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13152011
Early Warning System Bot
Comment 8 2012-07-05 03:08:55 PDT
Comment on attachment 150909 [details] Patch Attachment 150909 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13151010
Build Bot
Comment 9 2012-07-05 03:13:02 PDT
Comment on attachment 150909 [details] Patch Attachment 150909 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13136985
Build Bot
Comment 10 2012-07-05 03:15:30 PDT
Comment on attachment 150909 [details] Patch Attachment 150909 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13138906
Kwonjin Jeong
Comment 11 2012-07-05 03:16:42 PDT
Created attachment 150912 [details] Patch
WebKit Review Bot
Comment 12 2012-07-05 03:19:47 PDT
Attachment 150912 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/GNUmak..." exit_code: 1 Source/WTF/wtf/chromium/MainThreadChromium.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/WTF/wtf/chromium/MainThreadChromium.cpp:65: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 13 2012-07-05 03:24:06 PDT
Comment on attachment 150912 [details] Patch Attachment 150912 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13141128
Kwonjin Jeong
Comment 14 2012-07-05 04:21:18 PDT
Created attachment 150922 [details] Patch
WebKit Review Bot
Comment 15 2012-07-05 04:23:09 PDT
Attachment 150922 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/GNUmak..." exit_code: 1 Source/WTF/wtf/chromium/MainThreadChromium.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/WTF/wtf/chromium/MainThreadChromium.cpp:65: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kwonjin Jeong
Comment 16 2012-07-05 04:26:05 PDT
Created attachment 150924 [details] Patch
Build Bot
Comment 17 2012-07-05 04:32:19 PDT
Comment on attachment 150924 [details] Patch Attachment 150924 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13143135
Anders Carlsson
Comment 18 2012-07-05 12:39:38 PDT
What's the rationale for adding this class? We already have a WorkQueue class in the WebKit2 platform code which does more or less the same thing (and it's actually used in a bunch of places).
Sam Weinig
Comment 19 2012-07-05 13:42:29 PDT
Comment on attachment 150924 [details] Patch r- due to Anders' comment.
Kwang Yul Seo
Comment 20 2012-07-05 19:21:24 PDT
(In reply to comment #18) > What's the rationale for adding this class? > > We already have a WorkQueue class in the WebKit2 platform code which does more or less the same thing (and it's actually used in a bunch of places). Parallel image decoders create a serial queue for each image being decoded. That means a large number of serial queues can be created and used at the same time. DispatchQueue internally manages a pool of threads, and assigns a thread from the pool to each serial queue. So more than two serial queues can share a single underlying thread. libdispatch's serial queue handles this internally. DispatchQueue's generic back-end (thread-pool implementation) follows this. WorkQueue is not ideal for this situation because a WorkQueue instance creates its own thread. WorkQueueMac is the only exception because it uses libdispatch's serial queue. If parallel image decoders use WorkQueue instead of DispatchQueue, it can create too many threads and degrade the overall system performance. I admit that a serial DispatchQueue is just a subset of WorkQueue from the API perspective because it does not provide delayed dispatch and event source. The only enhancement it has is thread pooling. On the other hand, this patch is more generic than is necessary in some cases. For example, the global queue is not used anywhere and the main queue is simple wrapper around callOnMainThread. We will try to reuse WorkQueue in our parallel image decoders implementation since WorkQueue provides most of the features we need. I think we can probably do thread pooling on top WorkQueue. The first job is to move WorkQueue to WTF, so that WebCore code can use it. For the time being, please stop reviewing this patch. BTW, we barely know how WorkQueue works before submitting this patch. Thank you for the information and review.
Kwang Yul Seo
Comment 21 2012-07-10 01:28:01 PDT
We implemented a new class based on WorkQueue. I will file a bug. Close this bug as INVALID.
Kwang Yul Seo
Comment 22 2012-08-01 01:11:16 PDT
We can't use WorkQueue because Chromium does not have WorkQueue implementation. Reopen the bug to discuss how we can implement this in a cross-platform way.
Kwonjin Jeong
Comment 23 2012-08-01 01:37:27 PDT
Created attachment 155751 [details] Patch
Build Bot
Comment 24 2012-08-01 01:55:31 PDT
Comment on attachment 155751 [details] Patch Attachment 155751 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13420030
Kwonjin Jeong
Comment 25 2012-08-01 02:27:46 PDT
Created attachment 155763 [details] Patch
Build Bot
Comment 26 2012-08-01 02:36:47 PDT
Comment on attachment 155763 [details] Patch Attachment 155763 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13401741
Kwang Yul Seo
Comment 27 2012-08-09 17:29:25 PDT
(In reply to comment #26) > (From update of attachment 155763 [details]) > Attachment 155763 [details] did not pass mac-ews (mac): > Output: http://queues.webkit.org/results/13401741 Kwonjin, please upload a new patch which builds.
Note You need to log in before you can comment on or make changes to this bug.