RESOLVED FIXED 101846
Call to enclosingFilterLayer() in RenderObject::containerForRepaint() is expensive
https://bugs.webkit.org/show_bug.cgi?id=101846
Summary Call to enclosingFilterLayer() in RenderObject::containerForRepaint() is expe...
Simon Fraser (smfr)
Reported 2012-11-10 12:33:55 PST
The call to enclosingFilterLayer() in RenderObject::containerForRepaint() shows up in scrolling profiles as up to 4% of the total time. It adds a second tree walk, which is terrible.
Attachments
Patch (4.52 KB, patch)
2012-11-13 13:55 PST, Max Vujovic
simon.fraser: review+
eflews.bot: commit-queue-
Patch (4.57 KB, patch)
2012-11-13 15:47 PST, Max Vujovic
no flags
Alexandru Chiculita
Comment 1 2012-11-12 07:57:49 PST
(In reply to comment #0) > The call to enclosingFilterLayer() in RenderObject::containerForRepaint() shows up in scrolling profiles as up to 4% of the total time. It adds a second tree walk, which is terrible. I can take a look at that today. Do you think that a combined tree walk will be sufficient?
Simon Fraser (smfr)
Comment 2 2012-11-12 08:49:50 PST
Actually it's not as bad as I first thought; only 0.3% of a layout-heave trace while scrolling http://www.theverge.com/2012/10/30/3576178/apple-ipad-mini-review But it's still worth cleaning up. One thought I had was to set a bit somewhere per document (on FrameView?) that says that any filters are being used, and avoid filter-related code if not.
Max Vujovic
Comment 3 2012-11-13 13:55:02 PST
Created attachment 173979 [details] Patch Here's a possible patch implementing Simon's idea.
Simon Fraser (smfr)
Comment 4 2012-11-13 13:57:15 PST
Comment on attachment 173979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173979&action=review I wonder if there are other bits of code we can optimize? > Source/WebCore/page/FrameView.h:374 > + bool hasSoftwareFilters() { return m_hasSoftwareFilters; } Should be const.
Max Vujovic
Comment 5 2012-11-13 14:00:17 PST
(In reply to comment #4) > (From update of attachment 173979 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=173979&action=review > > I wonder if there are other bits of code we can optimize? > > > Source/WebCore/page/FrameView.h:374 > > + bool hasSoftwareFilters() { return m_hasSoftwareFilters; } > > Should be const. Good catch. Thanks for the review!
Alexandru Chiculita
Comment 6 2012-11-13 14:29:59 PST
(In reply to comment #4) > (From update of attachment 173979 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=173979&action=review > > I wonder if there are other bits of code we can optimize? One easy optimisation is to make RenderLayer::enclosingFilterLayer stop when it hits the first composited layer. Most probably we need to change its name too. Another optimisation is to avoid recomputing the filter result when nothing changed. We could use the invalidation process to clear the filter result. That might be useful for tile-based painting. In that case the current implementation will recompute the filter every time the paint is called. That happens even though there's no "invalidation" request between the paint calls. However, that optimisation only applies to software applied filters that require the full source image like blur, drop-shadow or custom filters. Those filters will always compute the full layer result, but will throw it away and recompute during the following paint.
EFL EWS Bot
Comment 7 2012-11-13 14:42:28 PST
Max Vujovic
Comment 8 2012-11-13 15:47:18 PST
Created attachment 174014 [details] Patch Fixing efl EWS failure. I was missing an #if ENABLE(CSS_FILTERS) guard.
WebKit Review Bot
Comment 9 2012-11-14 09:38:20 PST
Comment on attachment 174014 [details] Patch Clearing flags on attachment: 174014 Committed r134619: <http://trac.webkit.org/changeset/134619>
WebKit Review Bot
Comment 10 2012-11-14 09:38:24 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.