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-

Description Mikhail Pozdnyakov 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.
Comment 1 Mikhail Pozdnyakov 2013-09-03 05:24:36 PDT 
Created attachment 210356 [details]
patch
Comment 2 Mikhail Pozdnyakov 2013-09-03 05:26:37 PDT 
Created attachment 210357 [details]
patch
Comment 3 EFL EWS Bot 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
Comment 4 EFL EWS Bot 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
Comment 5 Anders Carlsson 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.
Comment 6 Benjamin Poulain 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.
Comment 7 Mikhail Pozdnyakov 2013-09-04 05:31:29 PDT 
Created attachment 210446 [details]
patch v2

isPod is removed from WTF.
Comment 8 Kenneth Rohde Christiansen 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*
Comment 9 Mikhail Pozdnyakov 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 ?
Comment 10 EFL EWS Bot 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
Comment 11 EFL EWS Bot 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
Comment 12 Anders Carlsson 2013-09-10 00:45:49 PDT 
Let’s do this, but let’s only change VectorTraits.h
Comment 13 Anders Carlsson 2013-09-25 08:34:42 PDT 
Comment on attachment 210446 [details]
patch v2

I believe this has been done.