Bug 170851

Summary: GraphicsLayerCA::recursiveCommitChanges should not descend into subtrees without changes
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, simon.fraser, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 170876    
Bug Blocks: 170928, 170938    
Attachments:
Description Flags
patch
none
patch
simon.fraser: review+
patch
none
patch
none
patch none

Description Antti Koivisto 2017-04-14 10:38:17 PDT 
With lots of layers this can be very slow as it always traverses the entire layer tree. We should track which branches actually have changes.
Comment 1 Antti Koivisto 2017-04-14 11:04:39 PDT 
Created attachment 307124 [details]
patch
Comment 2 Antti Koivisto 2017-04-14 11:14:05 PDT 
Created attachment 307125 [details]
patch
Comment 3 Radar WebKit Bug Importer 2017-04-14 11:18:14 PDT 
<rdar://problem/31631307>
Comment 4 Simon Fraser (smfr) 2017-04-14 14:51:43 PDT 
Comment on attachment 307125 [details]
patch

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

Nice, I always wanted to do this.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:4033
> +        if (ancestorCA.m_hasDescendantsWithUncommittedChanges)
> +            return;
> +        ancestorCA.m_hasDescendantsWithUncommittedChanges = true;

I would prefer you go through a setter and not poke the member variable directly.
Comment 5 Antti Koivisto 2017-04-15 04:49:51 PDT 
Created attachment 307197 [details]
patch
Comment 6 Antti Koivisto 2017-04-15 06:16:04 PDT 
https://trac.webkit.org/r215393
Comment 7 WebKit Commit Bot 2017-04-15 11:13:54 PDT 
Re-opened since this is blocked by bug 170876
Comment 8 Antti Koivisto 2017-04-17 06:22:27 PDT 
Created attachment 307266 [details]
patch

Enable the optimisation for Mac only for now.
Comment 9 Antti Koivisto 2017-04-17 06:40:35 PDT 
Created attachment 307268 [details]
patch
Comment 10 WebKit Commit Bot 2017-04-17 08:00:36 PDT 
Comment on attachment 307268 [details]
patch

Clearing flags on attachment: 307268

Committed r215410: <http://trac.webkit.org/changeset/215410>
Comment 11 WebKit Commit Bot 2017-04-17 08:00:38 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Simon Fraser (smfr) 2017-04-17 13:54:31 PDT 
That's unfortunate. Do we understand the iOS issue?
Comment 13 Antti Koivisto 2017-04-18 00:12:55 PDT 
I think so. I just wanted to do some more testing on iOS before enabling it.