809636 - Avoid recompositing when layers become inactive

Status: NEW

(Core :: Graphics: Layers, defect)

Description

[Benoit Girard (:BenWa)]

STR:
1) Scroll a div, it will become a separate layer.
2) Wait ~1 second after it stop scrolling, the layer will be merged and the screen will be re-composited

Expected: We should avoid re-compositing since nothing has changed.

Roc does layers have a way to know that we are just flattening a layer and that nothing has changed?

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is a very good thought.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 3

[Benoit Girard (:BenWa)]

Jeff pointed out yesterday that simply waiting on the next update wasn't desirable because we wont release the additional memory used by these layers.

Is the memory used by active layers unbounded?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The next paint will release the memory. So I don't think that's a problem. We certainly couldn't create any new layers (in the same window) without releasing old layer(s).

I do have a slight concern due to the fact that going inactive can change the rendering ... e.g. subpixel AA can be rendered differently.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OTOH we're already hitting that really since our activity timeout is 3-4s. So I don't think we'll make that any worse, and we might make it better if the inactive scrollframe rendering change happens at the same time as other changes in the window.

Comment 6

[Benoit Girard (:BenWa)]

The case I'm affraid of is if there's not going to be a next paint before the browser is alt+tab. We may retain 50 layers in a page like the 3d css periodic table.

I agree that in the general case this patch is a big improvement.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Benoit Girard (:BenWa) from comment #6)
> The case I'm affraid of is if there's not going to be a next paint before
> the browser is alt+tab. We may retain 50 layers in a page like the 3d css
> periodic table.

That's possible but I suspect so unlikely to be a problem we shouldn't worry about it.

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 679696 [details] [diff] [review]
fix

Review of attachment 679696 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks fine, but I'm not convinced that it will actually help. Do we have any data on this?

It seems to me like there are 2 main states that the browser could be in when we trigger layer inactivity:

1) Browser is idle - Page loading is complete and we're not drawing anything. In this case, the cost of an unnecessary paint to flatten the layers might be worth it to reclaim the memory, especially since the CPU should be idle.

2) Browser is active - Removing this invalidate call won't have any effect, because something else will invalidate during the same refresh driver interval.

The case where we win is if the browser is idle, we schedule this paint to flatten layers, nothing else invalidates, yet as soon as we start painting, we start receiving 'actual' invalidations which are then delayed by the unnecessary paint that is in progress. That seems like a rare case.

Also note that the periodic table demo is using 3d transforms, which are always active and never time out. Solving that is a different bug, probably involving calling LayerManager::ClearCachedResources after a period of inactivity.

Comment 9

[Benoit Girard (:BenWa)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> 1) Browser is idle - Page loading is complete and we're not drawing
> anything. In this case, the cost of an unnecessary paint to flatten the
> layers might be worth it to reclaim the memory, especially since the CPU
> should be idle.

Perhaps it's worth flattening in this case but there's no point in schedule a composite. On mobile we're wasting battery every time we have a scrollable nested div.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Benoit Girard (:BenWa) from comment #9)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> > 1) Browser is idle - Page loading is complete and we're not drawing
> > anything. In this case, the cost of an unnecessary paint to flatten the
> > layers might be worth it to reclaim the memory, especially since the CPU
> > should be idle.
> 
> Perhaps it's worth flattening in this case but there's no point in schedule
> a composite.

So you're suggesting we do a layer transaction to save memory, but not recomposite when that transaction has finished?

I suppose we could do that. However, adding machinery so we can do a transaction without forcing a following composite adds complexity that I don't think would be justified here, at least not without more data. (The reason I liked this patch is that it's so simple, it just removes code!)

> On mobile we're wasting battery every time we have a scrollable
> nested div.

Not really. Only if you scroll the nested DIV, and then leave the browser completely untouched for several seconds.