Bug 147446 – Minimize children vector capacity changes in PlatformCALayerRemote::recursiveBuildTransaction()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris Dumez]

PlatformCALayerRemote::recursiveBuildTransaction() currently does:
1. Clear the children vector (which frees the underlying memory)
2. Append layer IDs to the vector one by one, causing the Vector capacity to grow

Since we know how many layer IDs we are going to add, we can minimize the number of Vector capacity changes by:
1. Resizing the children vector to the target size (which will grow or shrink the capacity as needed)
2. Update the layer IDs in the children vector, which won't cause any capacity change

[Chris Dumez]

Created attachment 257839 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 257839 [details]
Patch

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

Could we also reserveInitialCapacity()/uncheckedAppend()?

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:162
> +            for (size_t i = 0; i < m_children.size(); ++i)

Why change this?

[Chris Dumez]

Comment on attachment 257839 [details]
Patch

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

>> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:162
>> +            for (size_t i = 0; i < m_children.size(); ++i)
> 
> Why change this?

reserveInitialCapacity() would not be OK here as the m_properties.children is not a newly created Vector here (may have elements already). I guess we could do something like:
m_properties.children.shrink(0); // clear items without freeing underlying capacity
m_properties.children.reserve(reserveCapacity); // Resize underlying capacity
for (const auto& layer : m_children)
  m_properties.children.uncheckedAppend(layer->layerID());

That said, I still prefer my approach as I find it clearer than shrink() + reserveCapacity(). Also, the pattern I used is not unusual (see for e.g. copy*ToVector() in HashMap.h.

[WebKit Commit Bot]

Comment on attachment 257839 [details]
Patch

Clearing flags on attachment: 257839

Committed r187594: <http://trac.webkit.org/changeset/187594>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.