1439291 - LayerPropertiesBase::Compute change is expensive for inactive layers

Status: NEW

(Core :: Web Painting, defect, P3)

Description

[Matt Woodrow (:mattwoodrow)]

Using the testcase in the URL field, ComputeChange (along with cloning the Layer tree, and freeing the clone) is around 10% of the total paint time.

This is especially annoying, since the vast majority of these inactive Layer trees haven't changed, and we're doing a lot of work to compute an empty invalidation rect.

We clone the Layer tree before making modifications, call BuildLayer to build the inactive Layer tree (and mutate retained Layers), and then call ComputeChange.

We do this, because when a Layer changes, we need to invalidate all pixels that it previously occupied, and we can only compute that using the values from before modification.

I think the easiest way to improve this would be to refactor FrameLayerBuilder to set all Layer properties at the same time using a single setter. We set most of them at fairly similar times, and I don't think it'd be too hard to combine the rest.

Then the implementation of Layer::SetProperties can include the code from LayerPropertiesBase::ComputeChange, and can return the invalid area for that Layer in local coordinate space. We already do comparisons of new/old values here (to decide when to call Mutated), so this should be cheap.

FrameLayerBuilder can then be responsible for converting the coordinates into the root Layers coordinate space as it exits the recursive BuildContainerLayerFor calls.

Comment 1

[Mayank Bansal]

FWIW, this testcase is way slower in WR. The CPU usage is 80%-100% of my 2C/4T processor.
non-WR is much faster, and the CPU use is ~40%.

non-WR : https://perfht.ml/2FjK4Xe
WR: https://perfht.ml/2Fhtc3t

Comment 2

[Matt Woodrow (:mattwoodrow)]

I added this testcase into the displaylist_mutate talos test, so we're covering this WR regression in bug 1426770.