Bug 90074 – Don't malloc RenderGeometryMap steps individually

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Antti Koivisto]

Mallocs and frees for steps under RenderGeometryMap::pus/popMappingsToAncestor can total ~2% of the profile when animating transforms.

[Radar WebKit Bug Importer]

<rdar://problem/11759092>

[Antti Koivisto]

Created attachment 149752 [details]
patch

[WebKit Review Bot]

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.

[Simon Fraser (smfr)]

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?

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Antti Koivisto]

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

[Antti Koivisto]

Created attachment 149978 [details]
patch2

Fixed the test failures.

[Antti Koivisto]

http://trac.webkit.org/changeset/121446