Add UMA histograms for DirectComposition overlays.

Add UMA histograms for DirectComposition overlays.

These get information about how often overlays are available, how often
they're attempted to be used, and how often they're actually used.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2779643004
Cr-Commit-Position: refs/heads/master@{#460666}
Committed: https://chromium.googlesource.com/chromium/src/+/15a130ea6aa5ccdd20d9bfdc960576d689807ba2

[jbauman, 2017-03-29 00:03:45]

Description was changed from

==========
Add UMA histograms for DirectComposition overlays.

These get information about how often overlays are available, how often
they're attempted to be used, and how often they're actually used.

BUG=678800
==========

to

==========
Add UMA histograms for DirectComposition overlays.

These get information about how often overlays are available, how often
they're attempted to be used, and how often they're actually used.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[jbauman, 2017-03-29 00:03:51]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 00:05:15]

Dry run: 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, 2017-03-29 02:00:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-29 02:00:07]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jbauman, 2017-03-29 02:20:23]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 02:20:56]

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

[jbauman, 2017-03-29 02:21:24]

jbauman@chromium.org changed reviewers:
+ isherman@chromium.org

[commit-bot: I haz the power, 2017-03-29 04:21:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-29 04:21:51]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[Ilya Sherman, 2017-03-29 05:16:42]

https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
File gpu/ipc/service/direct_composition_surface_win.cc (right):

https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
gpu/ipc/service/direct_composition_surface_win.cc:355:
DXGI_FRAME_PRESENTATION_MODE_COMPOSITION_FAILURE + 1);
This code looks fragile, in the sense that someone who updates the enum won't
necessarily know to update this code.  Please add either a COUNT or a MAX entry
to the enum, and document that the enum is now being used to back an UMA
histogram, and should therefore be treated as append-only.

https://codereview.chromium.org/2779643004/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2779643004/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:21652: +    How DWM presented Chrome's
DirectComposition layers to the screen.
nit: What is DWM?  It might help to expand this acronym here.

https://codereview.chromium.org/2779643004/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:21657: +    enum="BooleanSupported">
nit: Please use a custom enum here too (see below).

https://codereview.chromium.org/2779643004/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:21662: +<histogram
name="GPU.DirectComposition.OverlaysUsed" enum="BooleanUsage">
nit: Please use a custom enum (just within this file), with labels like
"Overlay" and "No overlay".

https://codereview.chromium.org/2779643004/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:88699: +  <int value="3">Composition
Failure</int>
nit: If you ever add additional enum entries, these will end up awkwardly in the
middle.  For this reason, I'd recommend actually moving these two exceptional
cases to the top of the enum.

[jbauman, 2017-03-29 18:58:10]

On 2017/03/29 05:16:42, Ilya Sherman wrote:
>
https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
> File gpu/ipc/service/direct_composition_surface_win.cc (right):
> 
>
https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
> gpu/ipc/service/direct_composition_surface_win.cc:355:
> DXGI_FRAME_PRESENTATION_MODE_COMPOSITION_FAILURE + 1);
> This code looks fragile, in the sense that someone who updates the enum won't
> necessarily know to update this code.  Please add either a COUNT or a MAX
entry
> to the enum, and document that the enum is now being used to back an UMA
> histogram, and should therefore be treated as append-only.
> 
This is actually a win32 enum:
https://msdn.microsoft.com/en-us/library/windows/desktop/dn384107(v=vs.85).aspx
So it should be append-only anyway. I could add a new enum and add code to
convert between the two, if you want.

[jbauman, 2017-03-29 19:25:19]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 19:26:19]

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

[Ilya Sherman, 2017-03-29 22:42:01]

On 2017/03/29 18:58:10, jbauman wrote:
> On 2017/03/29 05:16:42, Ilya Sherman wrote:
> >
>
https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
> > File gpu/ipc/service/direct_composition_surface_win.cc (right):
> > 
> >
>
https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
> > gpu/ipc/service/direct_composition_surface_win.cc:355:
> > DXGI_FRAME_PRESENTATION_MODE_COMPOSITION_FAILURE + 1);
> > This code looks fragile, in the sense that someone who updates the enum
won't
> > necessarily know to update this code.  Please add either a COUNT or a MAX
> entry
> > to the enum, and document that the enum is now being used to back an UMA
> > histogram, and should therefore be treated as append-only.
> > 
> This is actually a win32 enum:
>
https://msdn.microsoft.com/en-us/library/windows/desktop/dn384107(v=vs.85).aspx
> So it should be append-only anyway. I could add a new enum and add code to
> convert between the two, if you want.

Could you use UMA_HISTOGRAM_SPARSE_SLOWLY?  If you do, you won't need to provide
a max for the histogram.  Otherwise, this code would break whenever Microsoft
adds an enum entry, which means you'd definitely want to interpose an
intermediate enum with an "OTHER" catch-all bucket.

[jbauman, 2017-03-29 23:23:40]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 23:24:38]

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

[jbauman, 2017-03-29 23:50:30]

PTAL.

On 2017/03/29 22:42:01, Ilya Sherman wrote:
> On 2017/03/29 18:58:10, jbauman wrote:
> > On 2017/03/29 05:16:42, Ilya Sherman wrote:
> > >
> >
>
https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
> > > File gpu/ipc/service/direct_composition_surface_win.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2779643004/diff/20001/gpu/ipc/service/direct_...
> > > gpu/ipc/service/direct_composition_surface_win.cc:355:
> > > DXGI_FRAME_PRESENTATION_MODE_COMPOSITION_FAILURE + 1);
> > > This code looks fragile, in the sense that someone who updates the enum
> won't
> > > necessarily know to update this code.  Please add either a COUNT or a MAX
> > entry
> > > to the enum, and document that the enum is now being used to back an UMA
> > > histogram, and should therefore be treated as append-only.
> > > 
> > This is actually a win32 enum:
> >
>
https://msdn.microsoft.com/en-us/library/windows/desktop/dn384107(v=vs.85).aspx
> > So it should be append-only anyway. I could add a new enum and add code to
> > convert between the two, if you want.
> 
> Could you use UMA_HISTOGRAM_SPARSE_SLOWLY?  If you do, you won't need to
provide
> a max for the histogram.  Otherwise, this code would break whenever Microsoft
> adds an enum entry, which means you'd definitely want to interpose an
> intermediate enum with an "OTHER" catch-all bucket.

Done.

[Ilya Sherman, 2017-03-29 23:55:17]

Metrics LGTM, thanks.

[jbauman, 2017-03-30 00:13:04]

jbauman@chromium.org changed reviewers:
+ sunnyps@chromium.org

[sunnyps, 2017-03-30 00:30:38]

lgtm

[commit-bot: I haz the power, 2017-03-30 01:27:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 01:27:59]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[jbauman, 2017-03-30 04:40:43]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2017-03-30 04:41:01]

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, 2017-03-30 04:46:05]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490848843306740,
"parent_rev": "0799d3367209d4185480b0408397eea79cd67fc8", "commit_rev":
"15a130ea6aa5ccdd20d9bfdc960576d689807ba2"}

[commit-bot: I haz the power, 2017-03-30 04:46:50]

Description was changed from

==========
Add UMA histograms for DirectComposition overlays.

These get information about how often overlays are available, how often
they're attempted to be used, and how often they're actually used.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add UMA histograms for DirectComposition overlays.

These get information about how often overlays are available, how often
they're attempted to be used, and how often they're actually used.

BUG=678800
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2779643004
Cr-Commit-Position: refs/heads/master@{#460666}
Committed:
https://chromium.googlesource.com/chromium/src/+/15a130ea6aa5ccdd20d9bfdc9605...
==========

[commit-bot: I haz the power, 2017-03-30 04:46:51]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/15a130ea6aa5ccdd20d9bfdc9605...