gpu: Add a class MeasurementTimers that measures wall, cpu and gpu time.

gpu: Add a class MeasurementTimers that measures wall, cpu and gpu time.

The perf test texture_upload_perftest has been updated to use these new measurements.

BUG=423481
Committed: https://crrev.com/b27d8d3fd8ac59e01b345192c4ada2aef07cb844
Cr-Commit-Position: refs/heads/master@{#316314}

[Daniele Castagna, 2015-01-30 20:39:00]

dcastagna@chromium.org changed reviewers:
+ piman@chromium.org, reveman@chromium.org

[piman, 2015-01-30 21:12:04]

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.cc
File gpu/perftests/measurements.cc (right):

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
gpu/perftests/measurements.cc:57: glQueryCounter(gl_queries_[0], GL_TIMESTAMP);
This only works with GL_ARB_timer_query, which is not available everywhere.
There is also GL_EXT_disjoint_timer_query which has some subtle differences. Or
it's possible that the platform has neither of those, and we can't directly get
GPU timestamps.

See the code in GPUTracer (in gpu/command_buffer/service/gpu_tracer.cc) which
attempts to deal with the differences.

It may be good to extract/reuse that part of the code?

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
gpu/perftests/measurements.cc:58: CHECK(glIsQueryARB(gl_queries_[0]));
nit: DCHECK instead of CHECK (here and other places).

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.h
File gpu/perftests/measurements.h (right):

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.h...
gpu/perftests/measurements.h:46: base::ElapsedTimer wall_timer_;
ElapsedTimer uses TimeTicks::Now which can have low resolution on Windows.
Should we use TimeTicks::NowFromSystemTraceTime() which is high resolution?

[Daniele Castagna, 2015-02-03 04:12:59]

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.cc
File gpu/perftests/measurements.cc (right):

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
gpu/perftests/measurements.cc:57: glQueryCounter(gl_queries_[0], GL_TIMESTAMP);
On 2015/01/30 21:12:04, piman (Very slow to review) wrote:
> This only works with GL_ARB_timer_query, which is not available everywhere.
> There is also GL_EXT_disjoint_timer_query which has some subtle differences.
Or
> it's possible that the platform has neither of those, and we can't directly
get
> GPU timestamps.
> 
> See the code in GPUTracer (in gpu/command_buffer/service/gpu_tracer.cc) which
> attempts to deal with the differences.
> 
> It may be good to extract/reuse that part of the code?

Makes sense. I already reached out to dyen@ and vmiura@. I'm trying to extract
the GPU timing related code from gpu_tracer. I'll update here as soon as I have
a cl for that.

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
gpu/perftests/measurements.cc:58: CHECK(glIsQueryARB(gl_queries_[0]));
On 2015/01/30 21:12:04, piman (Very slow to review) wrote:
> nit: DCHECK instead of CHECK (here and other places).

Done.

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.h
File gpu/perftests/measurements.h (right):

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.h...
gpu/perftests/measurements.h:46: base::ElapsedTimer wall_timer_;
On 2015/01/30 21:12:04, piman (Very slow to review) wrote:
> ElapsedTimer uses TimeTicks::Now which can have low resolution on Windows.
> Should we use TimeTicks::NowFromSystemTraceTime() which is high resolution?

Done.

[Daniele Castagna, 2015-02-06 22:52:00]

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.cc
File gpu/perftests/measurements.cc (right):

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
gpu/perftests/measurements.cc:57: glQueryCounter(gl_queries_[0], GL_TIMESTAMP);
On 2015/02/03 04:12:59, dcastagna wrote:
> On 2015/01/30 21:12:04, piman (Very slow to review) wrote:
> > This only works with GL_ARB_timer_query, which is not available everywhere.
> > There is also GL_EXT_disjoint_timer_query which has some subtle differences.
> Or
> > it's possible that the platform has neither of those, and we can't directly
> get
> > GPU timestamps.
> > 
> > See the code in GPUTracer (in gpu/command_buffer/service/gpu_tracer.cc)
which
> > attempts to deal with the differences.
> > 
> > It may be good to extract/reuse that part of the code?
> 
> Makes sense. I already reached out to dyen@ and vmiura@. I'm trying to extract
> the GPU timing related code from gpu_tracer. I'll update here as soon as I
have
> a cl for that.

Still working with dyen@ on this: crrev.com/904743002.

[Daniele Castagna, 2015-02-13 21:13:01]

On 2015/02/06 at 22:52:00, dcastagna wrote:
> https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.cc
> File gpu/perftests/measurements.cc (right):
> 
>
https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
> gpu/perftests/measurements.cc:57: glQueryCounter(gl_queries_[0],
GL_TIMESTAMP);
> On 2015/02/03 04:12:59, dcastagna wrote:
> > On 2015/01/30 21:12:04, piman (Very slow to review) wrote:
> > > This only works with GL_ARB_timer_query, which is not available
everywhere.
> > > There is also GL_EXT_disjoint_timer_query which has some subtle
differences.
> > Or
> > > it's possible that the platform has neither of those, and we can't
directly
> > get
> > > GPU timestamps.
> > > 
> > > See the code in GPUTracer (in gpu/command_buffer/service/gpu_tracer.cc)
which
> > > attempts to deal with the differences.
> > > 
> > > It may be good to extract/reuse that part of the code?
> > 
> > Makes sense. I already reached out to dyen@ and vmiura@. I'm trying to
extract
> > the GPU timing related code from gpu_tracer. I'll update here as soon as I
have
> > a cl for that.
> 
> Still working with dyen@ on this: crrev.com/904743002.

Done.

[Daniele Castagna, 2015-02-13 21:17:42]

Sorry, I meant to answer "done" to the specific comment, not to the whole
thread.

The patch has been updated to use the GPUTimer we introduced with
https://codereview.chromium.org/904743002

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.cc
File gpu/perftests/measurements.cc (right):

https://codereview.chromium.org/893673002/diff/1/gpu/perftests/measurements.c...
gpu/perftests/measurements.cc:57: glQueryCounter(gl_queries_[0], GL_TIMESTAMP);
On 2015/02/06 at 22:52:00, dcastagna wrote:
> On 2015/02/03 04:12:59, dcastagna wrote:
> > On 2015/01/30 21:12:04, piman (Very slow to review) wrote:
> > > This only works with GL_ARB_timer_query, which is not available
everywhere.
> > > There is also GL_EXT_disjoint_timer_query which has some subtle
differences.
> > Or
> > > it's possible that the platform has neither of those, and we can't
directly
> > get
> > > GPU timestamps.
> > > 
> > > See the code in GPUTracer (in gpu/command_buffer/service/gpu_tracer.cc)
which
> > > attempts to deal with the differences.
> > > 
> > > It may be good to extract/reuse that part of the code?
> > 
> > Makes sense. I already reached out to dyen@ and vmiura@. I'm trying to
extract
> > the GPU timing related code from gpu_tracer. I'll update here as soon as I
have
> > a cl for that.
> 
> Still working with dyen@ on this: crrev.com/904743002.

Done.

[piman, 2015-02-13 21:57:29]

lgtm

[Daniele Castagna, 2015-02-13 22:24:48]

The CQ bit was checked by dcastagna@chromium.org

[commit-bot: I haz the power, 2015-02-13 22:26:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/893673002/40001

[commit-bot: I haz the power, 2015-02-13 23:07:17]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-13 23:08:21]

Patchset 3 (id:??) landed as
https://crrev.com/b27d8d3fd8ac59e01b345192c4ada2aef07cb844
Cr-Commit-Position: refs/heads/master@{#316314}