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

RESOLVED FIXED 128185

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

https://bugs.webkit.org/show_bug.cgi?id=128185

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

Wojciech Bielawski

Reported 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.

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 attachment proposed patch, testcase, etc.

Wojciech Bielawski

Comment 1 2014-02-04 08:54:10 PST

Created attachment 223122 [details] version with overlodad methods

Build Bot

Comment 2 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

Build Bot

Comment 3 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

Wojciech Bielawski

Comment 4 2014-02-05 05:18:25 PST

Created attachment 223227 [details] version with overlodad methods

Wojciech Bielawski

Comment 5 2014-02-05 07:37:18 PST

Created attachment 223232 [details] Version with VectorCopier usage

WebKit Commit Bot

Comment 6 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.

Wojciech Bielawski

Comment 7 2014-02-05 08:01:55 PST

Created attachment 223237 [details] Version with VectorCopier usage

Alexey Proskuryakov

Comment 8 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.

Build Bot

Comment 9 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

Build Bot

Comment 10 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

Wojciech Bielawski

Comment 11 2014-02-06 01:28:47 PST

Created attachment 223317 [details] Version with VectorCopier usage

Build Bot

Comment 12 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

Build Bot

Comment 13 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

Ryosuke Niwa

Comment 14 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.

Wojciech Bielawski

Comment 15 2014-02-11 03:37:33 PST

Created attachment 223837 [details] Version with VectorCopier usage

Darin Adler

Comment 16 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>?

Wojciech Bielawski

Comment 17 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

WebKit Commit Bot

Comment 18 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>

WebKit Commit Bot

Comment 19 2014-02-14 03:09:54 PST

All reviewed patches have been landed. Closing bug.

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

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component Web Template Framework

Assignee

Nobody

Reported

2014-02-04 08:39 PST

Modified

2014-02-14 03:09 PST History

CC List

7 users Show

URL

Keywords

Depends on

Blocks