Bug 158542

Summary: Greatly simplify CrossThreadTask.h
Product: WebKit Reporter: Brady Eidson <beidson>
Component: Web Template FrameworkAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, andersca, benjamin, cdumez, cmarcelo, commit-queue, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Brady Eidson
Reported 2016-06-08 14:20:23 PDT
Greatly simplify CrossThreadTask.h The current implementation predates variadic templates in C++11. Now with C++14 we can get rid of most of this nonsense. There is an additional move constructor call per argument object, which is disappointing. Maybe there's a way to get rid of that and I can't see that way. But compared to two extra copies (which is what we needed up through this morning before I landed http://trac.webkit.org/changeset/201809) we're still way better off.
Attachments
Patch (14.49 KB, patch)
2016-06-08 14:22 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch (14.74 KB, patch)
2016-06-08 14:51 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch (14.95 KB, patch)
2016-06-08 15:40 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Patch (14.63 KB, patch)
2016-06-08 15:57 PDT, Brady Eidson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Brady Eidson
Comment 1 2016-06-08 14:22:35 PDT
Created attachment 280837 [details] Patch
Sam Weinig
Comment 2 2016-06-08 14:31:49 PDT
Can this use std::index_sequence rather than the homespun GeneratedIndices? For an example usage of it, see WebKit2’s HandleMessage.h.
Brady Eidson
Comment 3 2016-06-08 14:32:43 PDT
(In reply to comment #2) > Can this use std::index_sequence rather than the homespun GeneratedIndices? > For an example usage of it, see WebKit2’s HandleMessage.h. Holy crap, yes.
Brady Eidson
Comment 4 2016-06-08 14:51:13 PDT
Created attachment 280843 [details] Patch
Brady Eidson
Comment 5 2016-06-08 15:19:59 PDT
Yikes on the build failures. Currently exploring why I'm seemingly good locally...
Brady Eidson
Comment 6 2016-06-08 15:40:04 PDT
Created attachment 280849 [details] Patch
Brady Eidson
Comment 7 2016-06-08 15:57:17 PDT
Created attachment 280852 [details] Patch
Saam Barati
Comment 8 2016-06-09 00:50:58 PDT
Comment on attachment 280852 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280852&action=review > Source/WTF/wtf/CrossThreadTask.h:80 > +void callMemberFunctionForCrossThreadTaskImpl(C* object, MF function, ArgsTuple&& args, std::index_sequence<ArgsIndex...>) std::index_sequence is quite clever. After enough reading on the web, I actually understand what's happening here.
Darin Adler
Comment 9 2016-06-09 10:10:31 PDT
Comment on attachment 280852 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=280852&action=review > Source/WTF/wtf/CrossThreadTask.h:65 > +template <typename F, typename ArgsTuple, typename ArgsIndicies = std::make_index_sequence<std::tuple_size<ArgsTuple>::value>> The word indices has only two "i"s in it.
WebKit Commit Bot
Comment 10 2016-06-09 10:33:25 PDT
Comment on attachment 280852 [details] Patch Clearing flags on attachment: 280852 Committed r201872: <http://trac.webkit.org/changeset/201872>
WebKit Commit Bot
Comment 11 2016-06-09 10:33:30 PDT
All reviewed patches have been landed. Closing bug.
Brady Eidson
Comment 12 2016-06-09 10:41:28 PDT
(In reply to comment #10) > Comment on attachment 280852 [details] > Patch > > Clearing flags on attachment: 280852 > > Committed r201872: <http://trac.webkit.org/changeset/201872> Fixed the typo in http://trac.webkit.org/changeset/201873
Note You need to log in before you can comment on or make changes to this bug.