ServiceWorker: Add UMA to measure execution times of onpush

ServiceWorker: Add UMA to measure execution times of onpush

Depends on https://codereview.chromium.org/707243003

BUG=401438, 407997
Committed: https://crrev.com/24be4a1347df1022b23b6cb4df6858070af3f811
Cr-Commit-Position: refs/heads/master@{#303641}

[johnme, 2014-11-08 01:25:27]

johnme@chromium.org changed reviewers:
+ mpearson@chromium.org, nhiroki@chromium.org

[johnme, 2014-11-08 01:25:28]

mpearson@chromium.org: Please review histograms.xml

nhiroki@chromium.org: Please review Service Worker changes

Thanks!

[nhiroki, 2014-11-10 03:41:04]

lgtm

[Mark P, 2014-11-10 17:21:21]

histograms.xml generally lgtm, one minor possible clarification

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

https://codereview.chromium.org/706253003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:31002: +  <summary>Execution time of
ServiceWorkerGlobalScope.onpush.</summary>
Is this recorded on both successful executions and failed ones, or only the
former?
(or do only the former exist?)

[johnme, 2014-11-10 20:43:33]

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

https://codereview.chromium.org/706253003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:31002: +  <summary>Execution time of
ServiceWorkerGlobalScope.onpush.</summary>
On 2014/11/10 17:21:21, Mark P wrote:
> Is this recorded on both successful executions and failed ones, or only the
> former?
> (or do only the former exist?)

Currently both, as we can't easily tell whether JavaScript did what it wanted to
do. In future though issue 430888 (and perhaps better tracking of exceptions)
will provide a mechanism for JS to tell us whether it was successful; once
that's the case, would you recommend only tracking successful executions? Or
tracking both separately?

[Mark P, 2014-11-10 21:42:15]

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

https://codereview.chromium.org/706253003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:31002: +  <summary>Execution time of
ServiceWorkerGlobalScope.onpush.</summary>
On 2014/11/10 20:43:33, johnme wrote:
> On 2014/11/10 17:21:21, Mark P wrote:
> > Is this recorded on both successful executions and failed ones, or only the
> > former?
> > (or do only the former exist?)
> 
> Currently both, as we can't easily tell whether JavaScript did what it wanted
to
> do. In future though issue 430888 (and perhaps better tracking of exceptions)
> will provide a mechanism for JS to tell us whether it was successful; once
> that's the case, would you recommend only tracking successful executions? Or
> tracking both separately?

It depends on failure states.  Is timeout, for instance a failure state?  The
usual practice I see is a timing metric for successful executions and another
histogram that counts the different types of failures, with no timing
information on failures.

[johnme, 2014-11-11 13:13:41]

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

https://codereview.chromium.org/706253003/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:31002: +  <summary>Execution time of
ServiceWorkerGlobalScope.onpush.</summary>
On 2014/11/10 21:42:14, Mark P wrote:
> On 2014/11/10 20:43:33, johnme wrote:
> > On 2014/11/10 17:21:21, Mark P wrote:
> > > Is this recorded on both successful executions and failed ones, or only
the
> > > former?
> > > (or do only the former exist?)
> > 
> > Currently both, as we can't easily tell whether JavaScript did what it
wanted
> to
> > do. In future though issue 430888 (and perhaps better tracking of
exceptions)
> > will provide a mechanism for JS to tell us whether it was successful; once
> > that's the case, would you recommend only tracking successful executions? Or
> > tracking both separately?
> 
> It depends on failure states.  Is timeout, for instance a failure state?  The
> usual practice I see is a timing metric for successful executions and another
> histogram that counts the different types of failures, with no timing
> information on failures.
> 

Sounds good. I changed it to only log if (result ==
blink::WebServiceWorkerEventResultCompleted). In the short term that won't
change anything (as result is always WebServiceWorkerEventResultCompleted), but
as soon as we start detecting failures (including timeouts once we start
limiting execution time) that means this will only log execution time when
successful.

And we'll start reporting failure types as part of https://crbug.com/401438.

[johnme, 2014-11-11 13:13:49]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2014-11-11 13:14:03]

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

[commit-bot: I haz the power, 2014-11-11 14:09:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-11-11 14:10:35]

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