Example of poor performance test

Deleted: Screenshot_14.png 129 KB

	Screenshot_14.png 129 KB View Download

I don't know that we can optimize away the layer tree update with contain: strict, because even if the content is within the wrapper we still need to know how other content will interact with it. Careful thought required.

This is related to crbug.com/667370.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

One to monitor for containment improvements.

We've found a similar case where we're experiencing the same issue I believe.

You can use the attached example, it's a page with a grid like layout with many cells, and each cell has a progress bar inside. The progress bar's width gets updated, and we're seeing long "Update Layer Tree" times even if we use "contain: strict|content" on the cells (maybe "contain: layout size" could be enough for this even).
In the attached example we have a 100x100 grid, and every second 1 progress bar gets its width changed. You can tweak the constants on the example if you want different number of rows or updates per second.

I've been investigating a little bit what's going on but I'm not an expert on this code so any suggestions/feedback about how to fix it would be welcomed.

When the width of a progress bar changes LayoutBoxModelObject::StyleDidChange() is called.
This method calls PaintLayer::StyleDidChange() which ends up calling PaintLayer::SetNeedsCompositingInputsUpdate()
because diff.NeedsLayout() is "true".
Then PaintLayer::SetNeedsCompositingInputsUpdateInternal() marks all the ancestors:
  current->child_needs_compositing_inputs_update_ = true;
This causes that later CompositingInputsUpdater::UpdateRecursive() is called for all the layers,
when in theory it shouldn't be needed.

The idea would be that a change on the width of the progress bar
shouldn't mark all the ancestors but stop in the one with "contain: content"
(or maybe "contain: layout size" is enough for this case).
But I'm not sure if I'm missing other things and this doesn't make sense at all.

Deleted: update-layer-tree.html 1.5 KB

update-layer-tree.html 1.5 KB View Download

Playing with this we've found that if we add an early return in PaintLayer::SetNeedsCompositingInputsUpdate()
if the containing block has "contain: content"
the time for "Update Layer Tree" phase gets reduced drastically.

Something like:
  void PaintLayer::SetNeedsCompositingInputsUpdate() {
    if (GetLayoutObject().ContainingBlock() &&
        GetLayoutObject().ContainingBlock()->ShouldApplyContentContainment()) {
      return;
    }
   SetNeedsCompositingInputsUpdateInternal();
   ...

Of course this is very tight to this specific example, and we'd need to find a generic solution. But I don't know this code, so I don't know the implications of all this or if all this makes any sense.

Please, could you share your thoughts on the topic? Thanks!

Sorry but in the last two comments I used "contain: content" by mistake,
it should be "contain: strict" in those examples.
As I mentioned there maybe "contain: size layout" could be enough, but in the exmaples I wanted to use "contain: size" for sure, that's why "contain: content" was wrong.
Attaching the proper example using "contain: strict".

About the issue itself I was checking how PaintLayer::SetNeedsCompositingInputsUpdate() works,
trying to stop marking things when I find a "contain: strict" element on the ancestor chain.
However that's not how things work as later in PaintLayerCompositor::UpdateIfNeeded(),
we start from the RootLayer() and call CompositingInputsUpdater::UpdateRecursive()
but that method won't go deep on the tree as the root layer is not marked as ChildNeedsCompositingInputsUpdate().
So I guess this approach is totally wrong, I don't know if you have any better idea of what we can do in this scenario (if any),
to avoid visiting all the layer tree and do work only in a specific subtree.

Deleted: update-layer-tree.html 1.5 KB

update-layer-tree.html 1.5 KB View Download

In your case, if you dirty layout on a PaintLayer within one of the cells, but due to the
structure of the DOM, the fan-out of the root element is O(# cells), so even if only
one needs to be re-computed, all of them are visited.

A way to fix this could be to specify a "compositing inputs root". This would function the
similarly to a StyleRecalcRoot (see core/css/style_recalc_root.h): the least common ancestor
of all PaintLayers that need compositing inputs updated.

This object would be stored on the PaintLayerCompositor, and every time SetNeedsCompositingInputsUpdate
is called, a method Update is called to update the least common ancestor.

The hardest part in doing all this is to compute the CompositingInputsUpdater::AncestorInfo efficiently
for the root. However, this is also where contain:strict saves the day, because clips etc can't
escape this PaintLayer, so if a CompositingInputsRoot is required to be contain:strict, it may
be easy enough to compute this. If you'd like to try this out I can advise/code review.