Open Bug 1247437 Opened 8 years ago Updated 2 years ago

Fall back to completely flattening if layerization causes OOM

Categories

(Core :: Graphics: Layers, defect)

defect

Tracking

()

People

(Reporter: jnicol, Assigned: jnicol)

References

(Blocks 1 open bug)

Details

Lots of the OOMs we seem to be experiencing on Android turn out to be because of complex layer trees.

Why don't we: If during rendering a layer we detect we're running out of memory (eg TextureClient allocation fails), then immediately abort the transaction, flatten the layer tree completely, and try again.

The user's experience on that page will no doubt suck, but it will be better than crashing.

We might want to only do this on release, since the crash reports help us find and fix more specific cases of bad layerization.
Yup, I love this. Our OOMs tend to be in GL allocation, but there are pretty much always TextureClient allocation failures in the logs before.
Markus, Matt, wondering if either of you have any ideas or objections to do with this? I'd like to get cracking on it asap.

I was thinking I'd make it so ClientLayer::RenderLayer() implementations abort if they fail a texture allocation (or perhaps if we receive a memory pressure event), and report that they failed for that reason up to ClientLayerManager::EndTransaction(). Which itself will end the transaction indicating in some way that it failed for this reason. nsDisplayList::PaintRoot() will catch that and try again, telling FrameLayerBuilder to use fewer layers this time.
Flags: needinfo?(mstange)
Flags: needinfo?(matt.woodrow)
Sounds reasonable to me. It might be simplest to trigger the flattened paint asynchronously so that you don't have to do any strange gymnastics in PaintRoot. And you need to store a bit of state somewhere. How long do you want the page to stay in flattened mode after hit an allocation problem?
Flags: needinfo?(mstange)
Yeah, sounds reasonable.

I assume you've seen this: http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#5304

I was hoping that code would die one day soon, but maybe it can't.
Flags: needinfo?(matt.woodrow)
(In reply to Markus Stange [:mstange] from comment #3)
> It might be simplest to trigger the flattened paint asynchronously so
> that you don't have to do any strange gymnastics in PaintRoot.

Any chance you could expand on how to do this exactly?

> How long do you want the page to stay in flattened mode after hit an allocation problem?

Hadn't worked that one out yet, was going to experiment a little first.
Flags: needinfo?(mstange)
(In reply to Jamie Nicol [:jnicol] from comment #5)
> (In reply to Markus Stange [:mstange] from comment #3)
> > It might be simplest to trigger the flattened paint asynchronously so
> > that you don't have to do any strange gymnastics in PaintRoot.
> 
> Any chance you could expand on how to do this exactly?

I'm not sure that this is such a good idea any more. Here's what I had in mind, but don't let that distract you from your plans:

nsLayoutUtils::PaintFrame:
  if aFrame has property "OOMed during painting at some point":
    builder.SetFlattenEverything(true);
  // do regular stuff
  if displayList.PaintRoot somehow returned that this paint OOMed:
    set "OOMed during painting at some point" property on aFrame
    aFrame->SchedulePaint() // will result in a call to nsLayoutUtils::PaintFrame on the next tick

And in the places that create new animated geometry roots, we could guard those with checks to the builder's "flatten everything" setting.
Flags: needinfo?(mstange)
> nsLayoutUtils::PaintFrame:
>  if aFrame has property "OOMed during painting at some point":
>    builder.SetFlattenEverything(true);
>  // do regular stuff
>  if displayList.PaintRoot somehow returned that this paint OOMed:
>    set "OOMed during painting at some point" property on aFrame
>    aFrame->SchedulePaint() // will result in a call to nsLayoutUtils::PaintFrame on the next tick

That's not entirely dissimilar to what I'd managed to throw together.

I however am storing the state in the PresShell rather than the frame. Then passing a flag to nsLayoutUtils::PaintFrame(). I did this as I saw that's the way that the "compressed" paint Matt implemented worked, which is a reasonably similar concept. Matt, would you agree it makes sense to implement this in roughly the same way? or is the frame the better option?

> And in the places that create new animated geometry roots, we could guard those with checks to 
> the builder's "flatten everything" setting.

This sounds like it should work, along with setting the max active layers to 1? So far I was just using the ContainerState::mFlattenToSingleLayer flag.

Which does work in terms of flattening, but also prevents any scrolling. I can re-enable scrolling by modifying ContainerState::SetupScrollingMetadata(), but that's perhaps not the right thing to do. Obviously scrolling does not work well when everything is flattened. Things like fixed/sticky items scroll as the page scrolls until they are repainted at the correct position. But maybe that's just something we have to live with in the name of not crashing?
(In reply to Jamie Nicol [:jnicol] from comment #7) 
> I however am storing the state in the PresShell rather than the frame. Then
> passing a flag to nsLayoutUtils::PaintFrame(). I did this as I saw that's
> the way that the "compressed" paint Matt implemented worked, which is a
> reasonably similar concept. Matt, would you agree it makes sense to
> implement this in roughly the same way? or is the frame the better option?

Either way is fine, I don't think it matters much.

> 
> > And in the places that create new animated geometry roots, we could guard those with checks to 
> > the builder's "flatten everything" setting.
> 
> This sounds like it should work, along with setting the max active layers to
> 1? So far I was just using the ContainerState::mFlattenToSingleLayer flag.

mFlattenToSingleLayer should be sufficient, I don't think you need the other changes.

> 
> Which does work in terms of flattening, but also prevents any scrolling. I
> can re-enable scrolling by modifying
> ContainerState::SetupScrollingMetadata(), but that's perhaps not the right
> thing to do. Obviously scrolling does not work well when everything is
> flattened. Things like fixed/sticky items scroll as the page scrolls until
> they are repainted at the correct position. But maybe that's just something
> we have to live with in the name of not crashing?

We probably need to disable APZ if we've flattened, since it won't be able to scroll correctly.
Keywords: feature
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.