128185 – (try)append and insert operations don't need new operator for PODs

Bug 128185 - (try)append and insert operations don't need new operator for PODs

Summary: (try)append and insert operations don't need new operator for PODs

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Web Template Framework (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Nobody

URL:
Keywords:

Depends on:
Blocks:

Reported:	2014-02-04 08:39 PST by Wojciech Bielawski
Modified:	2014-02-14 03:09 PST (History)
CC List:	7 users (show)

See Also:

Attachments
version with overlodad methods (6.49 KB, patch) 2014-02-04 08:54 PST, Wojciech Bielawski	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (408.32 KB, application/zip) 2014-02-05 01:23 PST, Build Bot	no flags	Details
version with overlodad methods (6.57 KB, patch) 2014-02-05 05:18 PST, Wojciech Bielawski	no flags	Details \| Formatted Diff \| Diff
Version with VectorCopier usage (3.50 KB, patch) 2014-02-05 07:37 PST, Wojciech Bielawski	no flags	Details \| Formatted Diff \| Diff
Version with VectorCopier usage (3.50 KB, patch) 2014-02-05 08:01 PST, Wojciech Bielawski	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (363.77 KB, application/zip) 2014-02-05 15:03 PST, Build Bot	no flags	Details
Version with VectorCopier usage (3.51 KB, patch) 2014-02-06 01:28 PST, Wojciech Bielawski	rniwa: review- buildbot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (515.81 KB, application/zip) 2014-02-06 08:42 PST, Build Bot	no flags	Details
Version with VectorCopier usage (3.79 KB, patch) 2014-02-11 03:37 PST, Wojciech Bielawski	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

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.