|
|
Created:
6 years, 9 months ago by danakj Modified:
6 years, 9 months ago CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, Ken Russell (switch to Gerrit), jamesr, boliu, vmpstr Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionRetry gpu compositing when fallback is not true.
If the GPU process goes down, and the renderer tries to recreate its
context too soon, it will fail. We should retry using GPU compositing
until fallback is given as true from the compositor to allow fallback
into software compositing.
R=jbauman@chromium.org, piman@chromium.org
BUG=356453, 327220
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=259668
Patch Set 1 : retry-gpu-compositing: #
Total comments: 5
Patch Set 2 : retry-gpu-compositing: checksoftware #Patch Set 3 : retry-gpu-compositing: simpler #Messages
Total messages: 15 (0 generated)
https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... File content/renderer/render_widget.cc (right): https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... content/renderer/render_widget.cc:938: if (!context_provider.get() && !fallback) { We should check for use_software here, because if gpu compositing is disabled on the command line then creating a context will never succeed and we don't want to wait for multiple retries. https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... content/renderer/render_widget.cc:940: return scoped_ptr<cc::OutputSurface>(); Does this always cause it to retry, even with the single-threaded compositor?
https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... File content/renderer/render_widget.cc (right): https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... content/renderer/render_widget.cc:938: if (!context_provider.get() && !fallback) { On 2014/03/26 17:17:26, jbauman wrote: > We should check for use_software here, because if gpu compositing is disabled on > the command line then creating a context will never succeed and we don't want to > wait for multiple retries. Oh, that's what I was going for but didn't capture, ya. https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... content/renderer/render_widget.cc:940: return scoped_ptr<cc::OutputSurface>(); On 2014/03/26 17:17:26, jbauman wrote: > Does this always cause it to retry, even with the single-threaded compositor? Yes. https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...
PTAL https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... File content/renderer/render_widget.cc (right): https://codereview.chromium.org/212103005/diff/1/content/renderer/render_widg... content/renderer/render_widget.cc:938: if (!context_provider.get() && !fallback) { On 2014/03/26 17:21:22, danakj wrote: > On 2014/03/26 17:17:26, jbauman wrote: > > We should check for use_software here, because if gpu compositing is disabled > on > > the command line then creating a context will never succeed and we don't want > to > > wait for multiple retries. > > Oh, that's what I was going for but didn't capture, ya. Done.
lgtm
piman PTAL for OWNERS
LGTM to unblock, but, do you know why the renderer fails to create its context? We used to block the reply in the GpuMessageFilter to wait until the UI has recreated its context, to make sure both the UI and the renderer talk to the same GPU process. Is that not happening any more? I worry that the "try 3 times" logic will paper over the race, but across hundreds of millions of users, some will run into the case where it's not enough.
I guess I'll land this for the GPU bots though to green up.
The CQ bit was checked by danakj@chromium.org
On Wed, Mar 26, 2014 at 2:17 PM, <piman@chromium.org> wrote: > LGTM to unblock, but, do you know why the renderer fails to create its > context? > We used to block the reply in the GpuMessageFilter to wait until the UI has > recreated its context, to make sure both the UI and the renderer talk to > the > same GPU process. Is that not happening any more? > Thanks, I always forget how that's supposed to work since I've never found/read the code before. I'll go poke around and try figure out. > > I worry that the "try 3 times" logic will paper over the race, but across > hundreds of millions of users, some will run into the case where it's not > enough. > > https://codereview.chromium.org/212103005/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/danakj@chromium.org/212103005/80001
On 2014/03/26 18:17:12, piman wrote: > LGTM to unblock, but, do you know why the renderer fails to create its context? > We used to block the reply in the GpuMessageFilter to wait until the UI has > recreated its context, to make sure both the UI and the renderer talk to the > same GPU process. Is that not happening any more? What happens is that after chrome://gpucrash, the next EstablishGpuChannel is returning channel_handle.socket.fd = -1, and channel_handle.name = "". So I guess it is not blocking. What looks like another renderer trying the same thing, does work correctly shortly after in the log. Going deeper.
On 2014/03/26 18:54:29, danakj wrote: > On 2014/03/26 18:17:12, piman wrote: > > LGTM to unblock, but, do you know why the renderer fails to create its > context? > > We used to block the reply in the GpuMessageFilter to wait until the UI has > > recreated its context, to make sure both the UI and the renderer talk to the > > same GPU process. Is that not happening any more? > > What happens is that after chrome://gpucrash, the next EstablishGpuChannel is > returning channel_handle.socket.fd = -1, and channel_handle.name = "". So I > guess it is not blocking. > > What looks like another renderer trying the same thing, does work correctly > shortly after in the log. > > Going deeper. We go to GpuMessageFilter::OnEstablishGpuChannel. It gets a host from GpuProcessHost::FromID. So the host is valid at that point. We call into GpuProcessHost::EstablishGpuChannel. We send a GpuMsg_EstablishChannel successfully. Then we hit GpuProcessHost::OnProcessCrashed, which does GpuProcessHost::SendOutstandingReplies, which sends back failed channels to the renderer. Any requests that come in after the OnProcessCrashed will succeed.
Message was sent while issue was closed.
Change committed as 259668
Message was sent while issue was closed.
On 2014/03/26 19:24:59, danakj wrote: > On 2014/03/26 18:54:29, danakj wrote: > > On 2014/03/26 18:17:12, piman wrote: > > > LGTM to unblock, but, do you know why the renderer fails to create its > > context? > > > We used to block the reply in the GpuMessageFilter to wait until the UI has > > > recreated its context, to make sure both the UI and the renderer talk to the > > > same GPU process. Is that not happening any more? > > > > What happens is that after chrome://gpucrash, the next EstablishGpuChannel is > > returning channel_handle.socket.fd = -1, and channel_handle.name = "". So I > > guess it is not blocking. > > > > What looks like another renderer trying the same thing, does work correctly > > shortly after in the log. > > > > Going deeper. > > We go to GpuMessageFilter::OnEstablishGpuChannel. > It gets a host from GpuProcessHost::FromID. So the host is valid at that point. > We call into GpuProcessHost::EstablishGpuChannel. > > We send a GpuMsg_EstablishChannel successfully. > > Then we hit GpuProcessHost::OnProcessCrashed, which does > GpuProcessHost::SendOutstandingReplies, which sends back failed channels to the > renderer. > > Any requests that come in after the OnProcessCrashed will succeed. Analysis/discussion of how to improve this situation moved to here: https://code.google.com/p/chromium/issues/detail?id=356876 |