skpbench: add option for gpu timing

skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003

Committed: https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96
Committed: https://skia.googlesource.com/skia/+/c6618dd1dadeac8b47b81fbee108c42cca8ab166

[csmartdalton, 2016-09-30 18:08:55]

Description was changed from

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
==========

to

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003
==========

[csmartdalton, 2016-09-30 18:09:20]

csmartdalton@google.com changed reviewers:
+ bsalomon@google.com, egdaniel@google.com

[bsalomon, 2016-09-30 18:37:16]

https://codereview.chromium.org/2388433003/diff/1/include/private/SkGpuTimer.h
File include/private/SkGpuTimer.h (right):

https://codereview.chromium.org/2388433003/diff/1/include/private/SkGpuTimer....
include/private/SkGpuTimer.h:15: using SkPlatformGpuTimerQuery = intptr_t;
What do you think about putting this in sk_gpu_test? It was my intention to put
the fence stuff there when I moved it tools but never followed through.

https://codereview.chromium.org/2388433003/diff/1/tools/gpu/gl/GLTestContext.cpp
File tools/gpu/gl/GLTestContext.cpp (right):

https://codereview.chromium.org/2388433003/diff/1/tools/gpu/gl/GLTestContext....
tools/gpu/gl/GLTestContext.cpp:26: constexpr static GrGLenum
GL_SYNC_GPU_COMMANDS_COMPLETE  = 0x9117;
I believe we more commonly use "static constexpr" rather than "constexpr static"

[csmartdalton, 2016-10-03 16:52:24]

https://codereview.chromium.org/2388433003/diff/1/include/private/SkGpuTimer.h
File include/private/SkGpuTimer.h (right):

https://codereview.chromium.org/2388433003/diff/1/include/private/SkGpuTimer....
include/private/SkGpuTimer.h:15: using SkPlatformGpuTimerQuery = intptr_t;
On 2016/09/30 18:37:16, bsalomon wrote:
> What do you think about putting this in sk_gpu_test? It was my intention to
put
> the fence stuff there when I moved it tools but never followed through.

Done.

https://codereview.chromium.org/2388433003/diff/1/tools/gpu/gl/GLTestContext.cpp
File tools/gpu/gl/GLTestContext.cpp (right):

https://codereview.chromium.org/2388433003/diff/1/tools/gpu/gl/GLTestContext....
tools/gpu/gl/GLTestContext.cpp:26: constexpr static GrGLenum
GL_SYNC_GPU_COMMANDS_COMPLETE  = 0x9117;
On 2016/09/30 18:37:16, bsalomon wrote:
> I believe we more commonly use "static constexpr" rather than "constexpr
static"

Done.

[csmartdalton, 2016-10-04 01:20:28]

rebase

[bsalomon, 2016-10-04 18:00:01]

lgtm, how (in)consistent is the gpu timer with fence based timing?

https://codereview.chromium.org/2388433003/diff/80001/tools/gpu/GpuTimer.h
File tools/gpu/GpuTimer.h (right):

https://codereview.chromium.org/2388433003/diff/80001/tools/gpu/GpuTimer.h#ne...
tools/gpu/GpuTimer.h:75: }
I think most of the other files in here would have a // namespace sk_gpu_test
here.

[csmartdalton, 2016-10-04 19:06:35]

Rebased, fixed compiler issues, addressed the comment, and made a couple more
stylistic and name changes. I'll check the box now but feel free to look it
over.

https://codereview.chromium.org/2388433003/diff/80001/tools/gpu/GpuTimer.h
File tools/gpu/GpuTimer.h (right):

https://codereview.chromium.org/2388433003/diff/80001/tools/gpu/GpuTimer.h#ne...
tools/gpu/GpuTimer.h:75: }
On 2016/10/04 18:00:01, bsalomon wrote:
> I think most of the other files in here would have a // namespace sk_gpu_test
> here.

Done.

[csmartdalton, 2016-10-04 19:06:44]

The CQ bit was checked by csmartdalton@google.com

[csmartdalton, 2016-10-04 19:06:45]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/2388433003/#ps120001
(title: "skpbench: add option for gpu timing")

[commit-bot: I haz the power, 2016-10-04 19:06:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[csmartdalton, 2016-10-04 19:17:36]

On 2016/10/04 18:00:01, bsalomon wrote:
> how (in)consistent is the gpu timer with fence based timing?

https://docs.google.com/a/google.com/spreadsheets/d/1Nk5jsTxehEluw4U0OZYxy9OR...

The timers themselves are both very repeatable, but the GPU clock is always
significantly faster than what we see on the CPU clock.

I'm not sure what to make of it, because the last time I measured with small
resolutions I got a very different impression of how GPU bound we are.

[commit-bot: I haz the power, 2016-10-04 19:33:45]

Description was changed from

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003
==========

to

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003

Committed:
https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96
==========

[commit-bot: I haz the power, 2016-10-04 19:33:46]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96

[mtklein_C, 2016-10-04 19:49:30]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/2390383002/ by mtklein@chromium.org.

The reason for reverting is: many bots failing.

[csmartdalton, 2016-10-04 20:35:23]

Description was changed from

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003

Committed:
https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96
==========

to

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003

Committed:
https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96
==========

[csmartdalton, 2016-10-04 22:08:13]

Oops, bad assert logic. Patchsets 8 and 9 should take care of it.

[csmartdalton, 2016-10-05 15:10:05]

delete skstd::exchange was overkill

[bsalomon, 2016-10-05 15:23:30]

lgtm

[csmartdalton, 2016-10-05 15:24:08]

The CQ bit was checked by csmartdalton@google.com

[commit-bot: I haz the power, 2016-10-05 15:24:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 15:42:05]

Description was changed from

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003

Committed:
https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96
==========

to

==========
skpbench: add option for gpu timing

Adds a gpu timing option with a GL implementation.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388433003

Committed:
https://skia.googlesource.com/skia/+/c06720d06faab3b01eba1b8693e0ac791f06dc96
Committed:
https://skia.googlesource.com/skia/+/c6618dd1dadeac8b47b81fbee108c42cca8ab166
==========

[commit-bot: I haz the power, 2016-10-05 15:42:06]

Committed patchset #12 (id:210001) as
https://skia.googlesource.com/skia/+/c6618dd1dadeac8b47b81fbee108c42cca8ab166