Bug 120630

Summary: Use std::is_pod to define WTF::Vector behaviour
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: Web Template FrameworkAssignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, benjamin, cmarcelo, commit-queue, darin, eflews.bot, gyuyoung.kim, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch
benjamin: review-, eflews.bot: commit-queue-
patch v2 eflews.bot: commit-queue-

Mikhail Pozdnyakov
Reported 2013-09-03 05:20:08 PDT
At the moment WTF::isPod is used in WTF::VectorTraits to define WTF::Vector behaviour. The problem is that WTF::isPod is limited (works with native types only) so non-optimized code path is selected in many cases where it should not be selected, for example if having vector of plain structures. Using std::is_pod would solve the problem.
Attachments
patch (9.36 KB, patch)
2013-09-03 05:24 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
patch (1.72 KB, patch)
2013-09-03 05:26 PDT, Mikhail Pozdnyakov 		benjamin: review-
eflews.bot: commit-queue-
DetailsFormatted DiffDiff
patch v2 (7.71 KB, patch)
2013-09-04 05:31 PDT, Mikhail Pozdnyakov 		eflews.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Mikhail Pozdnyakov
Comment 1 2013-09-03 05:24:36 PDT
Created attachment 210356 [details] patch
Mikhail Pozdnyakov
Comment 2 2013-09-03 05:26:37 PDT
Created attachment 210357 [details] patch
EFL EWS Bot
Comment 3 2013-09-03 05:54:21 PDT
Comment on attachment 210357 [details] patch Attachment 210357 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1684325
EFL EWS Bot
Comment 4 2013-09-03 06:04:11 PDT
Comment on attachment 210357 [details] patch Attachment 210357 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1691231
Anders Carlsson
Comment 5 2013-09-03 07:44:41 PDT
I think this is a good idea (and I do think we should move towards the STL type traits instead of the WTF ones), but it seems to me like this can change code in subtle ways and I'd like to understand what those changes are.
Benjamin Poulain
Comment 6 2013-09-03 17:35:34 PDT
Comment on attachment 210357 [details] patch You should try to remove IsPod from WTF. You can use TypeTraits's compile_asserts to verify the behavior.
Mikhail Pozdnyakov
Comment 7 2013-09-04 05:31:29 PDT
Created attachment 210446 [details] patch v2 isPod is removed from WTF.
Kenneth Rohde Christiansen
Comment 8 2013-09-04 05:33:42 PDT
Comment on attachment 210446 [details] patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=210446&action=review > Source/WTF/ChangeLog:10 > + (workes with native types only) so non-optimized code path was selected works* > Source/WTF/ChangeLog:12 > + vector of plain structures. Using of std::is_pod solves the problem. Use of*
Mikhail Pozdnyakov
Comment 9 2013-09-04 05:34:31 PDT
(In reply to comment #6) > (From update of attachment 210357 [details]) > You should try to remove IsPod from WTF. You can use TypeTraits's compile_asserts to verify the behavior. I've removed IsPod from WTF but I'm not sure I understood you correctly: should we verify STL behaviour within TypeTraits.cpp ?
EFL EWS Bot
Comment 10 2013-09-04 05:41:55 PDT
Comment on attachment 210446 [details] patch v2 Attachment 210446 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1694529
EFL EWS Bot
Comment 11 2013-09-04 06:05:41 PDT
Comment on attachment 210446 [details] patch v2 Attachment 210446 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1691507
Anders Carlsson
Comment 12 2013-09-10 00:45:49 PDT
Let’s do this, but let’s only change VectorTraits.h
Anders Carlsson
Comment 13 2013-09-25 08:34:42 PDT
Comment on attachment 210446 [details] patch v2 I believe this has been done.
Note You need to log in before you can comment on or make changes to this bug.