90584 – Add DispatchQueue class.

UNCONFIRMED 90584

Add DispatchQueue class.

https://bugs.webkit.org/show_bug.cgi?id=90584

Summary Add DispatchQueue class.

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	Details Formatted Diff Diff
Patch (13.93 KB, patch) 2012-07-05 03:03 PDT, Kwonjin Jeong	no flags	Details Formatted Diff Diff
Patch (41.50 KB, patch) 2012-07-05 03:16 PDT, Kwonjin Jeong	no flags	Details Formatted Diff Diff
Patch (41.50 KB, patch) 2012-07-05 04:21 PDT, Kwonjin Jeong	no flags	Details Formatted Diff Diff
Patch (41.49 KB, patch) 2012-07-05 04:26 PDT, Kwonjin Jeong	no flags	Details Formatted Diff Diff
Patch (42.06 KB, patch) 2012-08-01 01:37 PDT, Kwonjin Jeong	no flags	Details Formatted Diff Diff
Patch (41.49 KB, patch) 2012-08-01 02:27 PDT, Kwonjin Jeong	buildbot: commit-queue-	Details Formatted Diff Diff
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.

Status UNCONFIRMED

Resolution

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component Web Template Framework

Assignee

Nobody

Reported

2012-07-04 21:33 PDT

Modified

2012-08-09 17:29 PDT History

CC List

12 users Show

URL

Keywords

Depends on

Blocks

90375

Dependencies

tree graph