Bug 128185

Summary: (try)append and insert operations don't need new operator for PODs
Product: WebKit Reporter: Wojciech Bielawski <w.bielawski>
Component: Web Template FrameworkAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, benjamin, buildbot, cmarcelo, commit-queue, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
version with overlodad methods
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion
none
version with overlodad methods
none
Version with VectorCopier usage
none
Version with VectorCopier usage
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2
none
Version with VectorCopier usage
rniwa: review-, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
none
Version with VectorCopier usage none

Description Wojciech Bielawski 2014-02-04 08:39:44 PST
append, tryAppend and insert uses operator new to copy data from src to dest. For POD types it is unefficient.
Comment 1 Wojciech Bielawski 2014-02-04 08:54:10 PST
Created attachment 223122 [details]
version with overlodad methods
Comment 2 Build Bot 2014-02-05 01:23:40 PST
Comment on attachment 223122 [details]
version with overlodad methods

Attachment 223122 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4906591912460288

New failing tests:
accessibility/anchor-linked-anonymous-block-crash.html
compositing/absolute-inside-out-of-view-fixed.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/abort-cache-ondownloading-resource-404.html
animations/3d/state-at-end-event-transform.html
animations/added-while-suspended.html
animations/animation-add-events-in-handler.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
accessibility/accessibility-object-detached.html
accessibility/alt-tag-on-image-with-nonimage-role.html
accessibility/accessibility-node-reparent.html
animations/animation-border-overflow.html
http/tests/appcache/abort-cache-onchecking-resource-404.html
http/tests/appcache/abort-cache-ondownloading-manifest-404.html
accessibility/anonymous-render-block-in-continuation-causes-crash.html
animations/animation-controller-drt-api.html
animations/3d/change-transform-in-end-event.html
compositing/absolute-position-changed-in-composited-layer.html
canvas/philip/tests/2d.canvas.readonly.html
animations/3d/transform-perspective.html
http/tests/appcache/abort-cache-onchecking-manifest-404.html
canvas/philip/tests/2d.canvas.reference.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
http/tests/appcache/abort-cache-onchecking.html
accessibility/adjacent-continuations-cause-assertion-failure.html
http/tests/appcache/404-manifest.html
http/tests/appcache/404-resource.html
Comment 3 Build Bot 2014-02-05 01:23:43 PST
Created attachment 223219 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 4 Wojciech Bielawski 2014-02-05 05:18:25 PST
Created attachment 223227 [details]
version with overlodad methods
Comment 5 Wojciech Bielawski 2014-02-05 07:37:18 PST
Created attachment 223232 [details]
Version with VectorCopier usage
Comment 6 WebKit Commit Bot 2014-02-05 07:38:45 PST
Attachment 223232 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Vector.h:1016:  Missing space after ,  [whitespace/comma] [3]
ERROR: Source/WTF/wtf/Vector.h:1033:  Missing space after ,  [whitespace/comma] [3]
ERROR: Source/WTF/wtf/Vector.h:1095:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 3 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Wojciech Bielawski 2014-02-05 08:01:55 PST
Created attachment 223237 [details]
Version with VectorCopier usage
Comment 8 Alexey Proskuryakov 2014-02-05 10:01:21 PST
Looks like this is still causing flaky test failures, please click on a yellow bubble to see details.
Comment 9 Build Bot 2014-02-05 15:02:57 PST
Comment on attachment 223237 [details]
Version with VectorCopier usage

Attachment 223237 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6462736812736512

New failing tests:
compositing/checkerboard.html
compositing/absolute-inside-out-of-view-fixed.html
animations/3d/matrix-transform-type-animation.html
http/tests/cache/cancel-multiple-post-xhrs.html
animations/added-while-suspended.html
animations/animation-add-events-in-handler.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
accessibility/alt-tag-on-image-with-nonimage-role.html
compositing/bounds-in-flipped-writing-mode.html
accessibility/accessibility-node-reparent.html
animations/animation-border-overflow.html
accessibility/accessibility-object-detached.html
animations/animation-controller-drt-api.html
http/tests/cache/content-type-ignored-during-revalidation.html
compositing/absolute-position-changed-in-composited-layer.html
http/tests/cache/cancel-during-revalidation-succeeded.html
canvas/philip/tests/2d.canvas.readonly.html
animations/3d/transform-perspective.html
http/tests/cache/cancel-during-failure-crash.html
compositing/animation/animated-composited-inside-hidden.html
canvas/philip/tests/2d.canvas.reference.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
http/tests/cache/cached-main-resource.html
accessibility/adjacent-continuations-cause-assertion-failure.html
canvas/philip/tests/2d.clearRect+fillRect.basic.html
compositing/absolute-position-changed-with-composited-parent-layer.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html
Comment 10 Build Bot 2014-02-05 15:03:02 PST
Created attachment 223268 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 11 Wojciech Bielawski 2014-02-06 01:28:47 PST
Created attachment 223317 [details]
Version with VectorCopier usage
Comment 12 Build Bot 2014-02-06 08:42:00 PST
Comment on attachment 223317 [details]
Version with VectorCopier usage

Attachment 223317 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6053413678743552

New failing tests:
compositing/checkerboard.html
accessibility/anchor-linked-anonymous-block-crash.html
compositing/absolute-inside-out-of-view-fixed.html
animations/3d/matrix-transform-type-animation.html
http/tests/cache/cancel-multiple-post-xhrs.html
animations/added-while-suspended.html
animations/animation-add-events-in-handler.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
accessibility/alt-tag-on-image-with-nonimage-role.html
compositing/bounds-in-flipped-writing-mode.html
accessibility/accessibility-node-reparent.html
animations/animation-border-overflow.html
accessibility/accessibility-object-detached.html
animations/animation-controller-drt-api.html
http/tests/cache/content-type-ignored-during-revalidation.html
compositing/absolute-position-changed-in-composited-layer.html
http/tests/cache/cancel-during-revalidation-succeeded.html
canvas/philip/tests/2d.canvas.readonly.html
animations/3d/transform-perspective.html
http/tests/cache/cancel-during-failure-crash.html
compositing/animation/animated-composited-inside-hidden.html
canvas/philip/tests/2d.canvas.reference.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
http/tests/cache/cached-main-resource.html
accessibility/adjacent-continuations-cause-assertion-failure.html
compositing/absolute-position-changed-with-composited-parent-layer.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html
Comment 13 Build Bot 2014-02-06 08:42:03 PST
Created attachment 223335 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 14 Ryosuke Niwa 2014-02-06 20:08:08 PST
Comment on attachment 223317 [details]
Version with VectorCopier usage

Looks like this patch makes the tests flaky.  EWS has been retrying it for many many iterations.
Comment 15 Wojciech Bielawski 2014-02-11 03:37:33 PST
Created attachment 223837 [details]
Version with VectorCopier usage
Comment 16 Darin Adler 2014-02-11 15:13:26 PST
Comment on attachment 223837 [details]
Version with VectorCopier usage

View in context: https://bugs.webkit.org/attachment.cgi?id=223837&action=review

> Source/WTF/wtf/Vector.h:161
> +    template<typename U>
> +    static void uninitializedCopy(const T* src, const T* srcEnd, U* dst)
> +    {
> +        VectorCopier<false, T>::uninitializedCopy(src, srcEnd, dst);
> +    }

Maybe instead we should make VectorCopier<true, T> derive from VectorCopier<false, T>?
Comment 17 Wojciech Bielawski 2014-02-12 00:44:21 PST
Such approach won't compile. Since VectorCopier<false, T>::uninitializedCopy<U> is template metod the VectorCopier<true, T> derives only already instantinated methods, so when VectorCopier<ture, T>::uninitializedCopy<U> is called when noone before has instantinated VectorCopier<false, T>::uninitializedCopy<U> the compiler reports no matching function
Comment 18 WebKit Commit Bot 2014-02-14 03:09:50 PST
Comment on attachment 223837 [details]
Version with VectorCopier usage

Clearing flags on attachment: 223837

Committed r164097: <http://trac.webkit.org/changeset/164097>
Comment 19 WebKit Commit Bot 2014-02-14 03:09:54 PST
All reviewed patches have been landed.  Closing bug.