spend too much time getting frame properties in FrameLayerBuilder::GetPaintedLayerScale in order to snap scroll areas to layer pixels

RESOLVED WORKSFORME

Status

()

Core
Layout
RESOLVED WORKSFORME
a year ago
8 months ago

People

(Reporter: dbaron, Assigned: jwatt)

Tracking

({perf})

Trunk
x86
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 affected)

Details

(Whiteboard: [qf:p2])

I just took a profile of scrolling my facebook timeline.  I noticed that about 0.1% of the total time in the profile (including idle) was getting frame properties in FrameLayerBuilder::GetPaintedLayerScaleForFrame.  This is called a *lot* because we call it multiple times when we reflow a scroll frame, as a result of bug 1012752:

nsHTMLScrollFrame::Reflow -> ScrollFrameHelper::GetScrolledRect -> FrameLayerBuilder::GetPaintedLayerScaleForFrame

ScrollFrameHelper::ReflowFinished -> ScrollFrameHelper::ScrollToImpl -> FrameLayerBuilder::GetPaintedLayerScaleForFrame

It seems like we could consider avoiding this computation, e.g., by:
 * guarding the LayerManagerDataProperty() with a frame state bit (or bool:1 on nsIFrame if we're out of bits) that says that we have it, if my suspicion that most frames don't have it is correct
 * storing the scale in the ReflowInput during reflow (and mostly copying from parent to child) -- although I'm not sure how this helps with the ReflowFinished case

Or maybe there's something more clever we can do based on where this scale changes?  Any ideas, Markus?
Flags: needinfo?(mstange)
OS: Unspecified → Windows 10
Hardware: Unspecified → x86
See bug 1293031 comment 17 - I think some caching should help a lot here. Milan was playing with this a little but I don't know how far he got. I'll ask him tomorrow.
Flags: needinfo?(mstange)
It looks like tn wrote a patch in bug 1159459 that fixes this, but couldn't measure any improvement so didn't land it.

Updated

11 months ago
Whiteboard: [qf] → [qf:p1]
I wonder if we could avoid the pixel-snapping accuracy when we're:
 * determining the height of the scrollbar
 * scrolling somewhere not near the edge of the scrollport
I think tnikkel was going to look into resurrecting his patch.
Flags: needinfo?(tnikkel)
Probably caching the information we need, and then invalidating it appropriately, is the best option, though.

Updated

10 months ago
Assignee: nobody → npancholi

Comment 6

10 months ago
load-balance to jwatt
Assignee: npancholi → jwatt

Comment 7

9 months ago
No reply from Jwatt on ETA for this bug to be fixed within fx57. Moving to qf:p2
Whiteboard: [qf:p1] → [qf:p2]
(Assignee)

Comment 8

8 months ago
Sorry, Jean, I didn't see a question regarding ETA. I hope to have some patches up for review shortly though.
Note that the work in bug 1363922 may have made this obsolete.
(Assignee)

Comment 10

8 months ago
Yeah, I'm not seeing this on Windows 10 any more.
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Flags: needinfo?(tnikkel)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.