Add UMA stats for SurfaceDrawQuads in SurfaceAggregator.

Add UMA stats for SurfaceDrawQuads in SurfaceAggregator.

This CL adds UMA stats to keep track of SurfaceDrawQuads during surface
aggregation. For each call to SurfaceAggregator::Aggregate(), keep
track of surface referenced from SurfaceDrawQuads.

1. ValidSurface: The surface exists and has an active CompositorFrame.
2. MissingSurface: The surface doesn't exist.
3. NoActiveFrame: The surface exists but doesn't have an active frame.

Ideally 2 and 3 should never happen but the UMA stats will provide a
signal to if something has changed with regards to surface lifetime
management.

BUG=696596
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2752053003
Cr-Commit-Position: refs/heads/master@{#458480}
Committed: https://chromium.googlesource.com/chromium/src/+/0db8650476b17b19f443de6b58f20465ac6604f4

[kylechar, 2017-03-15 21:18:32]

Description was changed from

==========
Add UMA stats for SurfaceDrawQuads in SurfaceAggregator.

This CL adds UMA stats to keep track of SurfaceDrawQuads during surface
aggregation. For each call to SurfaceAggregator::Aggregate(), keep
track of surface referenced from SurfaceDrawQuads.

1. ValidSurface: The surface exists and has an active CompositorFrame.
2. MissingSurface: The surface doesn't exist.
3. NoActiveFrame: The surface exists but doesn't have an active frame.

Ideally 2 and 3 should never happen but the UMA stats will provide a
signal to if something has changed with regards to surface lifetime
management.

BUG=696596
==========

to

==========
Add UMA stats for SurfaceDrawQuads in SurfaceAggregator.

This CL adds UMA stats to keep track of SurfaceDrawQuads during surface
aggregation. For each call to SurfaceAggregator::Aggregate(), keep
track of surface referenced from SurfaceDrawQuads.

1. ValidSurface: The surface exists and has an active CompositorFrame.
2. MissingSurface: The surface doesn't exist.
3. NoActiveFrame: The surface exists but doesn't have an active frame.

Ideally 2 and 3 should never happen but the UMA stats will provide a
signal to if something has changed with regards to surface lifetime
management.

BUG=696596
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[kylechar, 2017-03-16 14:52:28]

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

[jbauman, 2017-03-16 19:41:11]

lgtm

[kylechar, 2017-03-16 19:49:00]

kylechar@chromium.org changed reviewers:
+ jwd@chromium.org

[kylechar, 2017-03-16 19:49:01]

Thanks!

+jwd for tools/metrics/*

[jwd, 2017-03-21 14:07:31]

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

https://codereview.chromium.org/2752053003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:8031: +    active CompositorFrame. This
is logged for one call to Aggregate().
Hm, can you change "one" to "each" for these summaries or is it not every call?
If it's not every call, can you be more specific about what call it is logged.

https://codereview.chromium.org/2752053003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:8043: +<histogram
name="Compositing.SurfaceAggregator.SurfaceDrawQuad.ValidSurface">
Note, you could have used histogram suffixes for these, and saved some precious
precious XML. The change is optional and I don't mind if you don't, but keep it
in mind for future histograms.

[kylechar, 2017-03-21 14:22:53]

Thanks!

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

https://codereview.chromium.org/2752053003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:8031: +    active CompositorFrame. This
is logged for one call to Aggregate().
On 2017/03/21 14:07:31, jwd wrote:
> Hm, can you change "one" to "each" for these summaries or is it not every
call?
> If it's not every call, can you be more specific about what call it is logged.

Done.

https://codereview.chromium.org/2752053003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:8043: +<histogram
name="Compositing.SurfaceAggregator.SurfaceDrawQuad.ValidSurface">
On 2017/03/21 14:07:31, jwd wrote:
> Note, you could have used histogram suffixes for these, and saved some
precious
> precious XML. The change is optional and I don't mind if you don't, but keep
it
> in mind for future histograms.

I'll definitely keep that in mind.

[jwd, 2017-03-21 15:08:14]

lgtm

[kylechar, 2017-03-21 16:59:27]

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

[commit-bot: I haz the power, 2017-03-21 16:59:54]

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-21 18:10:17]

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

[commit-bot: I haz the power, 2017-03-21 18:10:18]

Dry run: This issue passed the CQ dry run.

[kylechar, 2017-03-21 18:12:50]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2017-03-21 18:12:51]

The patchset sent to the CQ was uploaded after l-g-t-m from jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2752053003/#ps40001
(title: "Change to each.")

[commit-bot: I haz the power, 2017-03-21 18:13:36]

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-21 18:26:35]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490119970607400,
"parent_rev": "225850541ea9b36ecb151863f54c3a4265af085f", "commit_rev":
"0db8650476b17b19f443de6b58f20465ac6604f4"}

[commit-bot: I haz the power, 2017-03-21 18:27:19]

Description was changed from

==========
Add UMA stats for SurfaceDrawQuads in SurfaceAggregator.

This CL adds UMA stats to keep track of SurfaceDrawQuads during surface
aggregation. For each call to SurfaceAggregator::Aggregate(), keep
track of surface referenced from SurfaceDrawQuads.

1. ValidSurface: The surface exists and has an active CompositorFrame.
2. MissingSurface: The surface doesn't exist.
3. NoActiveFrame: The surface exists but doesn't have an active frame.

Ideally 2 and 3 should never happen but the UMA stats will provide a
signal to if something has changed with regards to surface lifetime
management.

BUG=696596
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Add UMA stats for SurfaceDrawQuads in SurfaceAggregator.

This CL adds UMA stats to keep track of SurfaceDrawQuads during surface
aggregation. For each call to SurfaceAggregator::Aggregate(), keep
track of surface referenced from SurfaceDrawQuads.

1. ValidSurface: The surface exists and has an active CompositorFrame.
2. MissingSurface: The surface doesn't exist.
3. NoActiveFrame: The surface exists but doesn't have an active frame.

Ideally 2 and 3 should never happen but the UMA stats will provide a
signal to if something has changed with regards to surface lifetime
management.

BUG=696596
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2752053003
Cr-Commit-Position: refs/heads/master@{#458480}
Committed:
https://chromium.googlesource.com/chromium/src/+/0db8650476b17b19f443de6b58f2...
==========

[commit-bot: I haz the power, 2017-03-21 18:27:23]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/0db8650476b17b19f443de6b58f2...