ServiceWorker: UMA histogram to track the size of Service Worker scripts

ServiceWorker: UMA histogram to track the size of Service Worker scripts

This adds ServiceWorker.ScriptSize histogram that records the length of
Service Worker scripts.

BUG=408451
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181289

[Kunihiko Sakamoto, 2014-09-01 05:49:44]

ksakamoto@chromium.org changed reviewers:
+ horo@chromium.org, nhiroki@chromium.org

[kenjibaheux, 2014-09-01 07:48:58]

On 2014/09/01 05:49:44, Kunihiko Sakamoto wrote:

Marja: We are adding UMA to inform what we will be able to do with streaming.
Could you monitor this codereview? We would like to make sure that the data will
be actionable.

Some questions that we had, while discussing this offline:
 - we would want to also know about the size of imported scripts.
 - do we need a size for the whole script including its imported scripts or
individual size for the different pieces?

[marja, 2014-09-01 07:57:17]

marja@chromium.org changed reviewers:
+ marja@chromium.org

[marja, 2014-09-01 07:57:17]

lgtm

Routing some "offline" discussion here:

While we stream, we only parse the script, so we cannot detect if a script
imports another one and start streaming it too, or anything like that.

So what matters is the script size without imports, and that's what we should be
monitoring.

[nhiroki, 2014-09-01 08:18:24]

LGTM

[horo, 2014-09-01 08:29:05]

lgtm

Please create chromium side patch to update
tools/metrics/histograms/histograms.xml.

[Kunihiko Sakamoto, 2014-09-01 08:37:31]

On 2014/09/01 08:29:05, horo wrote:
> Please create chromium side patch to update
> tools/metrics/histograms/histograms.xml.

yup: https://codereview.chromium.org/525733004/

[Kunihiko Sakamoto, 2014-09-02 01:16:17]

ksakamoto@chromium.org changed reviewers:
+ jochen@chromium.org

[Kunihiko Sakamoto, 2014-09-02 01:16:18]

jochen@, could you do owner's review?

[jochen (gone - plz use gerrit), 2014-09-02 07:59:07]

https://codereview.chromium.org/524553002/diff/1/Source/web/WebEmbeddedWorker...
File Source/web/WebEmbeddedWorkerImpl.cpp (right):

https://codereview.chromium.org/524553002/diff/1/Source/web/WebEmbeddedWorker...
Source/web/WebEmbeddedWorkerImpl.cpp:366:
Platform::current()->histogramCustomCounts("ServiceWorker.ScriptSizeInKB",
m_mainScriptLoader->script().length() / 1024, 0, 10 * 1024, 50);
for KB histograms, we usually use 1000, 500000, 50 (see
https://code.google.com/p/chromium/codesearch#chromium/src/base/metrics/histo...)

[Kunihiko Sakamoto, 2014-09-02 08:51:48]

https://codereview.chromium.org/524553002/diff/1/Source/web/WebEmbeddedWorker...
File Source/web/WebEmbeddedWorkerImpl.cpp (right):

https://codereview.chromium.org/524553002/diff/1/Source/web/WebEmbeddedWorker...
Source/web/WebEmbeddedWorkerImpl.cpp:366:
Platform::current()->histogramCustomCounts("ServiceWorker.ScriptSizeInKB",
m_mainScriptLoader->script().length() / 1024, 0, 10 * 1024, 50);
On 2014/09/02 07:59:07, jochen wrote:
> for KB histograms, we usually use 1000, 500000, 50 (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/metrics/histo...)

Thanks for pointing it out.
Changed to (1000, 5000000(=5MB), 50) because 500KB max may be too small for
script size.

[jochen (gone - plz use gerrit), 2014-09-02 08:55:28]

lgtm

[Kunihiko Sakamoto, 2014-09-03 00:30:58]

The CQ bit was checked by ksakamoto@chromium.org

[commit-bot: I haz the power, 2014-09-03 00:31:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/524553002/40001

[commit-bot: I haz the power, 2014-09-03 07:01:23]

Committed patchset #2 (id:40001) as 181289