Chromium Code Reviews Chromium Code Reviews
Issue
14863002:
Only update composited-scrolling state once after layout. (Closed)
DescriptionOnly update composited-scrolling state once after layout.
Before this patch, we would noisily update needs-composited-scrolling and
related state. This allowed for complex sequences of dirtying/cleaning and
guaranteeing correctness became difficult. With this patch, we update state
in only one spot: after layout (and before RLC::updateCompositingLayers).
We used to have a member, m_hasOutOfFlowPositionedDescendant, which, despite
its name, was only true if we had an out of flow positioned descendant whose
containing block was an ancestor of ours (and therefore does not respect our
clip). This was confusing. We now maintain m_hasUnclippedDescendant to
capture this concept.
That said, it turns out that we *do* still need
m_hasOutOfFlowPositionedDescendant, but we need it to mean exactly what it
says: the layer has an out of flow descendant anywhere in its subtree,
regardless of where the descendant's containing block is. I apologize for
the confusion this will cause when reviewing this patch, but I think the
new names make a lot more sense.
Consolidating the composited scrolling updates has a nice side effect: we
no longer need to check for 'unclipped' descendants as part of computing
the descendant dependent flags, greatly simplifying
updateDescendantDependent flags. Instead, we compute m_hasUnclippedDescendant
dynamically in RenderLayerCompositor::updateCompositingRequirementsState.
At this point, we should talk about the complexity of the updates performed
by the RLC. It needs to update m_hasUnclippedDescendant,
m_descendantsAreContiguousInStackingOrder and m_needsCompositedScrolling, and
we've done our best to minimize the work done to accomplish these updates.
To update m_hasUnclippedDescendant, we march up from every out-of-flow
positioned layer towards its containing block marking every layer along the
way as having an unclipped descendant. In the worst
case (every leaf is out-of-flow positioned and has the root as its containing
block and the tree is linear), this is O(n^2) in the size of the tree, but in
practice, we do much better; layers are typically close to their containing
blocks and we rarely traverse the same RenderLayers twice.
We only care about updating m_needsCompositedScrolling and
m_descendantsAreContiguousInStackingOrder for scrollable layers, so we
simply iterate over the FrameView's collection of scrollable areas and
call updateNeedsCompositedScrolling, which in turn updates the contiguity
state if necessary. This is O(s) where s is the sum of the sizes of the
stacking contexts inhabited by the scrollable layers. Again, this can be
quite large, but in practice tends to be reasonable.
You should really look at the next patch for accurate timing numbers as we
intend to replace our contiguity check:
https://codereview.chromium.org/13427009/
But for interest's sake, here are some performance numbers gathered on alex.
1. g+ photos. I scrolled down continuously for 46.9s letting the infinite
scroll kick in.
- We spent 118.2ms doing our updates (On average, 0.24ms per layout).
2. gmail. I loaded the page and scrolled.
- We spent 18ms doing our updates (On average, 0.2ms per layout).
3. squished presidents: loaded page.
- We spent 43.6ms doing our updates.
- Note we only did two updates, but each of these was immediately
followed by a commensurately longer computeCompositingRequirements.
This is part of a larger plan to finish the opt-in checks. Here's the aggregate
patch (it's getting stale, so YMMV): https://codereview.chromium.org/14741004/.
Also, this is the second patch in the series. This patch depends on
https://codereview.chromium.org/14858004/
R=jchaffraix@chromium.org
BUG=
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=151497
Patch Set 1 #Patch Set 2 : Adding TRACE_EVENTs to track the cost of the comp-scroll updates. #Patch Set 3 : Added early out of comp-scroll update when the setting isn't enabled. #Patch Set 4 : Optimized. #
Total comments: 11
Patch Set 5 : Address reviewer comments. #
Total comments: 10
Patch Set 6 : Rebase. #Patch Set 7 : . #
Total comments: 9
Patch Set 8 : subtreeHasOutOfFlowPositionedDescendant -> subtreeContainsOutOfFlowPositionedLayer #
Total comments: 4
Patch Set 9 : . #Patch Set 10 : rebase #Patch Set 11 : updating expectations. #Messages
Total messages: 17 (0 generated)
|
