Summary: | Clean up CrossThreadTask | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 132798, 132799 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Zan Dobersek
2014-05-10 23:28:15 PDT
Created attachment 231253 [details]
Patch
Comment on attachment 231253 [details]
Patch
This is a fine simplification, but I think we should go even further. I don’t think we want to use CrossThreadCopier. Instead we should have lambdas for each of these and copy the arguments explicitly at the call site. Need a new patch that compiles, though. EWS shows this failing to compile on most platforms.
(In reply to comment #2) > (From update of attachment 231253 [details]) > This is a fine simplification, but I think we should go even further. I don’t think we want to use CrossThreadCopier. Instead we should have lambdas for each of these and copy the arguments explicitly at the call site. Need a new patch that compiles, though. EWS shows this failing to compile on most platforms. I'm all for creating manual cross-thread copies at call sites, but with C++11 lambdas this incurs an additional copy for objects of types like String. Cross-thread copies for these are created through str.isolatedCopy(), but the return value has to be copied once more when it's being value-captured by the lambda function. C++14 fixes that by providing support for init-capture -- http://isocpp.org/files/papers/N3648.html I guess this won't be perfect until we make C++14 mandatory. Until then we can either stick to CrossThreadTask and CrossThreadCopier, or suffer through the additional copying overhead. (In reply to comment #3) > I'm all for creating manual cross-thread copies at call sites, but with C++11 lambdas this incurs an additional copy for objects of types like String. Sure, but lets keep in mind that “copy” here just means a ref/deref pair. I think that’s an OK cost. (In reply to comment #4) > (In reply to comment #3) > > I'm all for creating manual cross-thread copies at call sites, but with C++11 lambdas this incurs an additional copy for objects of types like String. > > Sure, but lets keep in mind that “copy” here just means a ref/deref pair. I think that’s an OK cost. OK, I'll give it a look. Created attachment 231284 [details]
Patch
Comment on attachment 231284 [details] Patch Clearing flags on attachment: 231284 Committed r168640: <http://trac.webkit.org/changeset/168640> All reviewed patches have been landed. Closing bug. |