Bug 90074

Summary: Don't malloc RenderGeometryMap steps individually
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02
none
patch2 simon.fraser: review+

Description Antti Koivisto 2012-06-27 07:13:08 PDT
Mallocs and frees for steps under RenderGeometryMap::pus/popMappingsToAncestor can total ~2% of the profile when animating transforms.
Comment 1 Radar WebKit Bug Importer 2012-06-27 07:14:30 PDT
<rdar://problem/11759092>
Comment 2 Antti Koivisto 2012-06-27 08:18:39 PDT
Created attachment 149752 [details]
patch
Comment 3 WebKit Review Bot 2012-06-27 08:22:44 PDT
Attachment 149752 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/rendering/RenderGeometryMap.h:118:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Simon Fraser (smfr) 2012-06-27 08:35:07 PDT
Comment on attachment 149752 [details]
patch

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

> Source/WebCore/rendering/RenderGeometryMap.h:61
> +    OwnPtr<TransformationMatrix> m_transform; // Includes offset if non-null.

How is this copyable, since it has an OwnPtr<>. Doesn't using it in a Vector by value require that it's copyable?

Also, does the copy cost show up on profiles, since we insert before the end often?
Comment 5 WebKit Review Bot 2012-06-27 11:08:44 PDT
Comment on attachment 149752 [details]
patch

Attachment 149752 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13097817

New failing tests:
compositing/geometry/video-fixed-scrolling.html
compositing/iframes/overlapped-nested-iframes.html
Comment 6 WebKit Review Bot 2012-06-27 11:08:48 PDT
Created attachment 149775 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 7 Antti Koivisto 2012-06-27 14:39:06 PDT
> How is this copyable, since it has an OwnPtr<>. Doesn't using it in a Vector by value require that it's copyable?

SimpleClassHashTraits should make it safe. Vector doesn't need copying, just moving, and moving with memcpy/memmove works just fine with OwnPtr. The same reason pure Vector<OwnPtr<Foo>> works basically (which also has SimpleClassHashTraits). 

I'll see about the test failures though.

> Also, does the copy cost show up on profiles, since we insert before the end often?

I'll check but that seems unlikely. The inserts are rarely very far from the end and the type is not particularly big.
Comment 8 Antti Koivisto 2012-06-28 11:28:12 PDT
Created attachment 149978 [details]
patch2

Fixed the test failures.
Comment 9 Antti Koivisto 2012-06-28 11:58:40 PDT
http://trac.webkit.org/changeset/121446