Mac ÜC: Enable GPU back-pressure from the browser to the renderer

Mac ÜC: Enable GPU back-pressure from the browser to the renderer

When the browser compositor does a SwapBuffers to its output surface,
take a lock on the compositor until that swap is displayed on the screen.
This prevents the renderer from running ahead of the browser.

If the renderer sends a new frame (it can send at most one), then track
this event, and request redraw as soon as the lock is released.

BUG=314190
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275478

[ccameron, 2014-06-05 01:31:51]

This is the stuff we discussed earlier today.

https://codereview.chromium.org/316103004/diff/30001/content/browser/composit...
File content/browser/compositor/delegated_frame_host.cc (right):

https://codereview.chromium.org/316103004/diff/30001/content/browser/composit...
content/browser/compositor/delegated_frame_host.cc:754:
client_->DelegatedCompositorDidSwapBuffers();
This is called from SingleThreadProxy::DidSwapFrame, which then gets renamed to
DidCommitAndDrawFrame, then to OnCompositingStarted, completely obscuring what's
actually happening here.

Also, there is a bug in SingleThreadProxy::DidSwapFrame which makes this
sometimes get called when there is no frame (I'm addressing that in a separate
CL, with unit tests, etc, to make sure it doesn't happen again).

[jbauman, 2014-06-05 20:34:10]

Have you tested whether this actually works? It looks to me like CompositorLock
doesn't do anything if threaded compositing is disabled (which it should be on
mac).

[ccameron, 2014-06-05 20:41:57]

On 2014/06/05 20:34:10, jbauman wrote:
> Have you tested whether this actually works? It looks to me like
CompositorLock
> doesn't do anything if threaded compositing is disabled (which it should be on
> mac).

The browser's compositor is single-threaded on Mac.

[ccameron, 2014-06-05 20:42:40]

On 2014/06/05 20:41:57, ccameron1 wrote:
> On 2014/06/05 20:34:10, jbauman wrote:
> > Have you tested whether this actually works? It looks to me like
> CompositorLock
> > doesn't do anything if threaded compositing is disabled (which it should be
on
> > mac).
> 
> The browser's compositor is single-threaded on Mac.

(the backpressure appears effective, except for hangs where some bugs in cc/
result in us being told that a swap is happening, when it's actually not)

[jbauman, 2014-06-05 20:44:46]

lgtm, I see how CompositorLock is working now.

[ccameron, 2014-06-05 20:48:38]

Thanks!

[ccameron, 2014-06-05 20:48:43]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-06-05 20:50:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/316103004/30001

[commit-bot: I haz the power, 2014-06-06 00:41:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-06-06 00:47:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-06 00:47:21]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[ccameron, 2014-06-06 16:44:19]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-06-06 16:46:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/316103004/30001

[commit-bot: I haz the power, 2014-06-06 17:12:26]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-06-06 18:05:19]

Change committed as 275478

[ccameron, 2014-06-06 18:09:07]

On 2014/06/06 18:05:19, I haz the power (commit-bot) wrote:
> Change committed as 275478

Hmm, just now I was thinking that maybe I should roll this into
DelegatedFrameHost, cause it already has the state tracking in
CanLockCompositorState.

Well, in a follow-up.

[no sievers, 2014-06-06 20:06:39]

On 2014/06/06 18:09:07, ccameron1 wrote:
> On 2014/06/06 18:05:19, I haz the power (commit-bot) wrote:
> > Change committed as 275478
> 
> Hmm, just now I was thinking that maybe I should roll this into
> DelegatedFrameHost, cause it already has the state tracking in
> CanLockCompositorState.
> 
> Well, in a follow-up.

Just out of curiosity: In Aura and Android we use the browser compositor's
DidCommit() callback to delay sending the CompositorFrameAck to throttle
renderer to browser frame rate. Is this achieving the same just in a different
way? I guess I don't know how this 'lock' here works :)

[ccameron, 2014-06-06 20:17:11]

On 2014/06/06 20:06:39, sievers wrote:
> On 2014/06/06 18:09:07, ccameron1 wrote:
> > On 2014/06/06 18:05:19, I haz the power (commit-bot) wrote:
> > > Change committed as 275478
> > 
> > Hmm, just now I was thinking that maybe I should roll this into
> > DelegatedFrameHost, cause it already has the state tracking in
> > CanLockCompositorState.
> > 
> > Well, in a follow-up.
> 
> Just out of curiosity: In Aura and Android we use the browser compositor's
> DidCommit() callback to delay sending the CompositorFrameAck to throttle
> renderer to browser frame rate. Is this achieving the same just in a different
> way? I guess I don't know how this 'lock' here works :)

It's true!
    The browser is the one that actually does the final Swap(). Before ÜC, we
just had the GPU process not complete its swap until the swap was acked by the
browser. This is a perfectly good way to induce backpressure.

But Mac is complicated.
    Except, with ÜC, doing so would introduce a potential deadlock -- the
browser process (UI thread) already may wait on the GPU process, and this would
make the GPU process wait on the browser process (UI thread).

But we can fix it!
    So, to work around this, I lock the compositor when the Swap is issued the
the GPU process, and hold the lock until the Swap is presented to the screen.

The fix complicates things.
    The renderer may still send us the next frame (cause it is unblocked after
the commit -- and that is a good thing), but it will get eaten and ignored by
the compositor. So we need to track if we need to force a commit.

But it gets better!
    So, on 10.9+, we can do the present inside the GPU process and save
ourselves all of these issues!

But not really.
    We're not deprecating 10.6, 10.7, or 10.8 any time soon.