Bug 88464

Summary: Cache isSelfPaintingLayer() for better performance
Product: WebKit Reporter: Julien Chaffraix <jchaffraix>
Component: Layout and RenderingAssignee: Julien Chaffraix <jchaffraix>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, jamesr, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 75001, 92258    
Attachments:
Description Flags
Proposed change. none

Julien Chaffraix
Reported 2012-06-06 15:32:56 PDT
Currently we recompute isSelfPaintingLayer() every time the function is called. I have seen this function showing up on some benchmarks when scrolling on big tables such as http://dglazkov.github.com/performance-tests/biggrid.html. The painting code checks this boolean several time per paint phase so it should help both the self-painting and the non-self-painting layers. Patch forthcoming.
Attachments
Proposed change. (5.14 KB, patch)
2012-06-06 15:44 PDT, Julien Chaffraix
no flags
Julien Chaffraix
Comment 1 2012-06-06 15:44:59 PDT
Created attachment 146132 [details] Proposed change.
James Robinson
Comment 2 2012-06-06 16:38:45 PDT
Comment on attachment 146132 [details] Proposed change. View in context: https://bugs.webkit.org/attachment.cgi?id=146132&action=review > Source/WebCore/rendering/RenderLayer.cpp:4667 > || renderer()->hasReflection() can this change after the RenderLayer is created?
Simon Fraser (smfr)
Comment 3 2012-06-06 16:41:27 PDT
Comment on attachment 146132 [details] Proposed change. Seems like hasMask() could also change, as well as some of the isNormalFlowOnly() criteria.
Simon Fraser (smfr)
Comment 4 2012-06-06 16:41:50 PDT
You could cache it between styleChanged() calls maybe.
James Robinson
Comment 5 2012-06-06 16:43:19 PDT
Ah, I see you've hooked styleChanged.
Simon Fraser (smfr)
Comment 6 2012-06-06 16:45:03 PDT
Comment on attachment 146132 [details] Proposed change. Oh, I see you update it in styleChanged(). Should be OK. I guess you could assert that m_isSelfPaintingLayer == shouldBeSelfPaintingLayer() in painting, but that's probably overkill.
Julien Chaffraix
Comment 7 2012-06-06 16:49:00 PDT
Please no fighting, style change is covered! :) (In reply to comment #6) > (From update of attachment 146132 [details]) > Oh, I see you update it in styleChanged(). Should be OK. I guess you could assert that m_isSelfPaintingLayer == shouldBeSelfPaintingLayer() in painting, but that's probably overkill. It's possible but if this were to happen I would expect a lot of the code to break pretty badly (it would mean we start mutating the style without notifying us). Also shouldBeNormalFlowOnly has been working pretty well for some time without bug that I know of.
WebKit Review Bot
Comment 8 2012-06-06 21:21:03 PDT
Comment on attachment 146132 [details] Proposed change. Clearing flags on attachment: 146132 Committed r119676: <http://trac.webkit.org/changeset/119676>
WebKit Review Bot
Comment 9 2012-06-06 21:21:07 PDT
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.