Summary: | Make CrossThreadCopier more efficient (fewer copies!) | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||||
Component: | Web Template Framework | Assignee: | Brady Eidson <beidson> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, commit-queue | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=158528 | ||||||||||
Attachments: |
|
Description
Brady Eidson
2016-06-06 19:50:22 PDT
Created attachment 280771 [details]
Patch
Yup, no good - I hit the ASSERT in DRT locally as the mac-debug EWS is seeing. Exploring. Created attachment 280777 [details]
Patch
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.
(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. 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! Created attachment 280810 [details]
Patch w/test
Not marked as a patch yet, because this will rely on 158528 landing first.
|