|Summary:||Make CrossThreadCopier more efficient (fewer copies!)|
|Product:||WebKit||Reporter:||Brady Eidson <beidson>|
|Component:||Web Template Framework||Assignee:||Brady Eidson <beidson>|
|Severity:||Normal||CC:||benjamin, cdumez, cmarcelo, commit-queue|
|Version:||WebKit Nightly Build|
Description Brady Eidson 2016-06-06 19:50:22 PDT
Make CrossThreadCopier more efficient (less copies!)
Comment 2 Brady Eidson 2016-06-07 23:02:35 PDT
Yup, no good - I hit the ASSERT in DRT locally as the mac-debug EWS is seeing. Exploring.
Comment 4 Alex Christensen 2016-06-08 00:05:57 PDT
Comment on attachment 280777 [details] Patch This sounds good. My first thoughts: 1) Could we add an API test similar to the RefLogger that counts how many constructors have been called? 2) It seems like this whole thing would be more elegant using variadic templates, but that is always hard to figure out. 3) It's way too late at night to be reviewing code.
Comment 5 Brady Eidson 2016-06-08 07:55:03 PDT
(In reply to comment #4) > Comment on attachment 280777 [details] > Patch > > This sounds good. My first thoughts: > 1) Could we add an API test similar to the RefLogger that counts how many > constructors have been called? Interesting idea. Will take a look. > 2) It seems like this whole thing would be more elegant using variadic > templates, but that is always hard to figure out. Indeed, we started the CrossThreadCopier before variadic templates. I was actually going to spend some time today working on a variadic template overhaul, just to see how it would look. But that will be an experiment with no guarantee of success. I still want to get this overhaul in. > 3) It's way too late at night to be reviewing code. I noticed, since you didn't actually review it.
Comment 6 Brady Eidson 2016-06-08 09:17:15 PDT
I'll land an API test in https://bugs.webkit.org/show_bug.cgi?id=158528 And then update this patch with the changes to that api test!
Comment 7 Brady Eidson 2016-06-08 09:34:13 PDT
Created attachment 280810 [details] Patch w/test Not marked as a patch yet, because this will rely on 158528 landing first.