Make sure inproc GPU service is started before navigation starts |
|||||||||
Issue descriptionThis is on low-end device, i.e. GPU service lives inside browser process. The device is "gobo" running Android O (OMB1.171105.001). I noticed that sometimes GpuService is initialized before navigation starts and sometimes after. See the screenshots, where: * Green line is SingleThreadProxy::ScheduledActionBeginLayerTreeFrameSinkCreation * Blue line is SingleThreadProxy::RequestNewLayerTreeFrameSink I.e. ScheduledActionBeginLayerTreeFrameSinkCreation() posts a task that invokes RequestNewLayerTreeFrameSink(), and sometimes that task is invoked after ChromeTabbedActivity.initializeState (which is ~180ms). Note that ChromeTabbedActivity.initializeState is Java task, while SingleThreadProxy tasks are native. I'm still investigating what exactly is happening, but I doubt that we'll be able to enforce ordering between Java and native tasks (e.g. native tasks are asynchronous tasks, while tasks posted from Java are synchronous). Anyway, back to the issue. When GpuService is started after navigation, renderer ends up waiting for quite some time in Gpu::EstablishGpuChannelSync. For example in gpu_after_navigation.html renderer waited there for 390ms. Is it possible to avoid posting a task in ScheduledActionBeginLayerTreeFrameSinkCreation? The comment there seems to imply that we do that only to streamline tests. Alternatively, can we start GpuService even before that? During browser initialization for example?
,
Nov 22 2017
,
Nov 22 2017
,
Nov 22 2017
,
Nov 23 2017
It turned out that ordering of Java / native tasks is not guaranteed at all, see issue 788008.
,
Nov 24 2017
,
Nov 24 2017
,
Nov 27 2017
,
Nov 27 2017
we skip gpu warm up if gpu is in-process, probably under the assumption that starting a thread is faster than launching a process (which is totally true on android): https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.cc?rcl=72bc6da43e2b5b52a8a32508fc5f777fbc6053e9&l=1633 but maybe still not fast enough? I find navigation generally tends to do a lot of work on IO thread, so can't be the best point to start the gpu service either
,
Nov 27 2017
> probably under the assumption that starting a thread is faster than launching a process Err, no, that's not it, at least according to the comment right above where I linked. // When running the GPU thread in-process, avoid optimistically starting it // since creating the GPU thread races against creation of the one-and-only // ChildProcess instance which is created by the renderer thread. Although I don't really understand that comment
,
Nov 27 2017
Thanks for the pointer, Bo! That comment indeed looks very cryptic.
,
Nov 27 2017
OK, so that check (!(kSingleProcess || kInProcessGPU), later refactored into !UsingInProcessGpu) was there initially from 2012: https://chromiumcodereview.appspot.com/11469022 Later in issue 450396 sievers@ explained why it was that way, and you both agreed that the concern is probably no longer valid. So I guess I'll experiment with removing UsingInProcessGpu() check from there.
,
Dec 1 2017
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f43f72247f52443c52ee60a908b174cbd90a76b commit 6f43f72247f52443c52ee60a908b174cbd90a76b Author: Dmitry Skiba <dskiba@chromium.org> Date: Tue Dec 05 22:37:28 2017 Start in-proc GPU early. On low-end Android devices in-proc GPU is started on demand, when CompositorImpl::SetVisible(true) is called for the first time. Due to the fact that native / Java messages on Android are not strictly ordered (see crbug.com/788008) sometimes GPU startup is delayed for too long, stalling the renderer (see the bug). We already have code to start GPU early (from BrowserThreadsStarted), but that code includes explicit !UsingInProcessGpu() check. However, the check (which was there from 2012), seems obsolete now. As sievers@ and boliu@ discussed in crbug.com/450396#c4 the check was there to avoid a race setting ChildProcess::current, but became obsolete once ChildProcess::current switched to TLS. This CL removes !UsingInProcessGpu() check, causing in-proc GPU to start early. Bug: 787647 Change-Id: Ifb6764e8b479003412674ce667dd6c3ea08dcb43 Reviewed-on: https://chromium-review.googlesource.com/804144 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Dmitry Skiba <dskiba@chromium.org> Cr-Commit-Position: refs/heads/master@{#521865} [modify] https://crrev.com/6f43f72247f52443c52ee60a908b174cbd90a76b/content/browser/browser_main_loop.cc
,
Dec 6 2017
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by dskiba@chromium.org
, Nov 22 2017Here is how RequestNewLayerTreeFrameSink triggers creation of Chrome_InProcGpuThread thread: SingleThreadProxy::RequestNewLayerTreeFrameSink -> BrowserGpuChannelHostFactory::EstablishRequest::Create -> (posts on IO) BrowserGpuChannelHostFactory::EstablishRequest::EstablishOnIO -> Thread("Chrome_InProcGpuThread") Some frames are missing though.