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 Framework

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, ap, benjamin, buildbot, cmarcelo, commit-queue, rniwa

Priority:

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

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.