1599251 - Make NativeLayerCA double-buffered instead of triple-buffered and add a poll-wait-loop with a timeout

Status: RESOLVED FIXED

(Core :: Graphics, task)

Description

[Markus Stange [:mstange]]

At the moment, every NativeLayerCA holds on to three surfaces by default: One in-progress surface, one surface in mFrontSurface, and one surface in mSurfaces. (It also holds on to an arbitrarily high number of surfaces that are still in use by the window server.)

This takes up more memory than we would like it to.

Here's an alternative model:

• Instead of an mSurfaces queue, just have one mBackbufferSurface field.
• When calling NextSurface:
  • If IOSurfaceIsInUse(mBackbufferSurface) then poll + sleep(0.1ms) spin until it's no longer in use, with a timeout of 500ms.
  • If timed out, allocate a new surface.
  • Otherwise, use mBackbufferSurface.

This should only take 2x window size instead of 3x to 4x window size memory.

Comment 1

[Markus Stange [:mstange]]

Two buffers doesn't seem to be enough. I have run into cases where the WindowServer keeps the old IOSurface in use for a long time (> half a second) even when the layer's contents have already been replaced with a new IOSurface. I think this may be happening in cases where the layer is not visible on the screen.
Moreover, in the cases where the window server does let go of the old IOSurface, 60 fps compositing is possible but can come with additional frame delay. I've seen cases where the old IOSurface is in use until about halfway through the "vsync budget", so we spend the first 5ms of each composite waiting. It's possible that this drastically reduces our "margin of error" of composite timing if we want to have low frame delay.

I'll use this bug to implement a more conservative approach that still uses 3 buffers when compositing is busy but falls back to 2 buffers when the layer is only updating rarely.

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1599251 - Don't leave unused surfaces in the swap chain. r=jrmuizel

When updating the layer at 60fps, the surface that we would be leaving behind
here would usually still be in use at the time this method was called.
But when some time has passed since the last update, usually all surfaces in
mSurfaces are no longer "in use" so we can "compress" the swap chain to be
double-buffered.

Depends on D54859

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1599251 - Don't allocate more than three surfaces per NativeLayer. Wait on the window server if necessary (but with a timeout of 500ms). r=jrmuizel

This effectively limits the swap chain to three buffers.
I haven't seen this sleep call show up in profiles except on very rare occasions.
In the cases where it does show up, it usually only takes up a single 1ms sample
within a many-second range.

Depends on D54860

Comment 4

[Markus Stange [:mstange]]

The second patch causes long pauses after GPU switching. I'll need to experiment with this some more.

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1599251 - Don't keep surfaces which never become unused in NativeLayer::mSurfaces. r=jrmuizel

Depends on D54860

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5127ba5a484
Don't leave unused surfaces in the swap chain. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/6d3376e13bcf
Don't keep surfaces which never become unused in NativeLayer::mSurfaces. r=jrmuizel

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/f5127ba5a484
https://hg.mozilla.org/mozilla-central/rev/6d3376e13bcf