GPU: call CollectGraphicsinfo() for warming up the sandbox on Linux.

GPU: call CreateGLContext() to warm up the sandbox on Linux.

We refactor the sandbox warmup code in a new WarmUpSandbox() function.
We call CreateGLContext() as part of the warm-up for Nvidia drivers.

BUG=140214
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152012

[jln (very slow on Chromium), 2012-08-16 00:33:02]

Please, let me know if I should split the WarmUp refactor part of this CL into
another CL.

It's nice to have everything in one place, but this introduce some minor
re-ordering of things on Windows.

[Ken Russell (switch to Gerrit), 2012-08-16 00:45:54]

https://chromiumcodereview.appspot.com/10831343/diff/8001/content/gpu/gpu_mai...
File content/gpu/gpu_main.cc (right):

https://chromiumcodereview.appspot.com/10831343/diff/8001/content/gpu/gpu_mai...
content/gpu/gpu_main.cc:245: CollectGraphicsInfo(gpu_info);
I think you should be more explicit about this. Instead of relying on the
implementation of CollectGraphicsInfo, call GLContext::CreateGLContext() or
whatever you need to. My opinion only; I defer to zmo's review.

[jln (very slow on Chromium), 2012-08-16 00:52:52]

https://chromiumcodereview.appspot.com/10831343/diff/8001/content/gpu/gpu_mai...
File content/gpu/gpu_main.cc (right):

https://chromiumcodereview.appspot.com/10831343/diff/8001/content/gpu/gpu_mai...
content/gpu/gpu_main.cc:245: CollectGraphicsInfo(gpu_info);
On 2012/08/16 00:45:54, kbr wrote:
> I think you should be more explicit about this. Instead of relying on the
> implementation of CollectGraphicsInfo, call GLContext::CreateGLContext() or
> whatever you need to. My opinion only; I defer to zmo's review.

I refactored the local "CollectGraphicsInfo" at the bottom of this file. 

My reasoning for doing this (and calling it from here) was that if we call
CollectGraphicsInfo() in the WarmUp function, we want it to be as useful as if
it had been called directly.

Maybe that's getting too complex?

I could also remove zmo's optimization on condition that the driver is nvidia,
but I thought refactoring things in a WarmUp function would be nice.

[Zhenyao Mo, 2012-08-16 00:53:47]

Agree with Ken, calling CreateGLContext in WarmUp is more clear/readable, even
though it's not much different from CollectGraphicsInfo.

Otherwise LGTM.

[jln (very slow on Chromium), 2012-08-16 00:59:38]

On Wed, Aug 15, 2012 at 5:53 PM,  <zmo@chromium.org> wrote:
> Agree with Ken, calling CreateGLContext in WarmUp is more clear/readable,
> even
> though it's not much different from CollectGraphicsInfo.

Ok. Just to double check: we don't care about WarmUpSandbox having the
side effect of collecting the gpu_info for us if we hadn't already
done so before?

Julien

[jln (very slow on Chromium), 2012-08-16 01:21:38]

On 2012/08/16 00:59:38, Julien Tinnes wrote:
> On Wed, Aug 15, 2012 at 5:53 PM,  <mailto:zmo@chromium.org> wrote:
> > Agree with Ken, calling CreateGLContext in WarmUp is more clear/readable,
> > even
> > though it's not much different from CollectGraphicsInfo.
> 
> Ok. Just to double check: we don't care about WarmUpSandbox having the
> side effect of collecting the gpu_info for us if we hadn't already
> done so before?
> 
> Julien

I talked to Zhenyao in person and we don't seem to care about that optimization.

PTAL!

[jln (very slow on Chromium), 2012-08-16 01:48:17]

On 2012/08/16 01:21:38, Julien Tinnes wrote:
> On 2012/08/16 00:59:38, Julien Tinnes wrote:
> > On Wed, Aug 15, 2012 at 5:53 PM,  <mailto:zmo@chromium.org> wrote:
> > > Agree with Ken, calling CreateGLContext in WarmUp is more clear/readable,
> > > even
> > > though it's not much different from CollectGraphicsInfo.
> > 
> > Ok. Just to double check: we don't care about WarmUpSandbox having the
> > side effect of collecting the gpu_info for us if we hadn't already
> > done so before?
> > 
> > Julien
> 
> I talked to Zhenyao in person and we don't seem to care about that
optimization.
> 
> PTAL!

I also had to call MakeCurrent/ReleaseCurrent or I would see an error about
opening /dev/nvidia0 (I didn't see anything go wrong, but I suspect it's worth
fixing now).

Julien

[jln (very slow on Chromium), 2012-08-16 18:35:08]

On 2012/08/16 01:48:17, Julien Tinnes wrote:
> On 2012/08/16 01:21:38, Julien Tinnes wrote:
> > On 2012/08/16 00:59:38, Julien Tinnes wrote:
> > > On Wed, Aug 15, 2012 at 5:53 PM,  <mailto:zmo@chromium.org> wrote:
> > > > Agree with Ken, calling CreateGLContext in WarmUp is more
clear/readable,
> > > > even
> > > > though it's not much different from CollectGraphicsInfo.
> > > 
> > > Ok. Just to double check: we don't care about WarmUpSandbox having the
> > > side effect of collecting the gpu_info for us if we hadn't already
> > > done so before?
> > > 
> > > Julien
> > 
> > I talked to Zhenyao in person and we don't seem to care about that
> optimization.
> > 
> > PTAL!
> 
> I also had to call MakeCurrent/ReleaseCurrent or I would see an error about
> opening /dev/nvidia0 (I didn't see anything go wrong, but I suspect it's worth
> fixing now).
> 
> Julien

Zmo: do you mind taking another look ? I didn't change much, but since I'm
unfamiliar with this code, I would appreciate it.

Ken/Antoine: I'll also need owner LGTM please.

[piman, 2012-08-16 18:38:33]

https://chromiumcodereview.appspot.com/10831343/diff/5/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://chromiumcodereview.appspot.com/10831343/diff/5/content/gpu/gpu_main.c...
content/gpu/gpu_main.cc:256: context->ReleaseCurrent(surface.get());
nit:indent

https://chromiumcodereview.appspot.com/10831343/diff/5/content/gpu/gpu_main.c...
content/gpu/gpu_main.cc:265: }
Is there any way we could avoid all these levels of nesting? Maybe adding a
CreateDummyContext function or something so that you can early exit on failure.

[jln (very slow on Chromium), 2012-08-16 18:55:31]

On 2012/08/16 18:38:33, piman wrote:
> https://chromiumcodereview.appspot.com/10831343/diff/5/content/gpu/gpu_main.cc
> File content/gpu/gpu_main.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10831343/diff/5/content/gpu/gpu_main.c...
> content/gpu/gpu_main.cc:256: context->ReleaseCurrent(surface.get());
> nit:indent
> 
>
https://chromiumcodereview.appspot.com/10831343/diff/5/content/gpu/gpu_main.c...
> content/gpu/gpu_main.cc:265: }
> Is there any way we could avoid all these levels of nesting? Maybe adding a
> CreateDummyContext function or something so that you can early exit on
failure.

Done, PTAL!

[piman, 2012-08-16 19:11:35]

lgtm

[Ken Russell (switch to Gerrit), 2012-08-16 21:26:00]

lgtm

[commit-bot: I haz the power, 2012-08-16 21:28:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jln@chromium.org/10831343/8002

[commit-bot: I haz the power, 2012-08-17 02:09:53]

Change committed as 152012