Bug 88464 - Cache isSelfPaintingLayer() for better performance
Summary: Cache isSelfPaintingLayer() for better performance
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks: 75001 92258
  Show dependency treegraph
 
Reported: 2012-06-06 15:32 PDT by Julien Chaffraix
Modified: 2012-09-18 11:57 PDT (History)
4 users (show)

See Also:


Attachments
Proposed change. (5.14 KB, patch)
2012-06-06 15:44 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 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.
Comment 1 Julien Chaffraix 2012-06-06 15:44:59 PDT
Created attachment 146132 [details]
Proposed change.
Comment 2 James Robinson 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?
Comment 3 Simon Fraser (smfr) 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.
Comment 4 Simon Fraser (smfr) 2012-06-06 16:41:50 PDT
You could cache it between styleChanged() calls maybe.
Comment 5 James Robinson 2012-06-06 16:43:19 PDT
Ah, I see you've hooked styleChanged.
Comment 6 Simon Fraser (smfr) 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.
Comment 7 Julien Chaffraix 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.
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2012-06-06 21:21:07 PDT
All reviewed patches have been landed.  Closing bug.