Log predictor database size on startup

Log predictor database size on startup

This patch adds UMA about the predictor's database size when it is first
initialized. This will track cases when the pref writing policy fails due
to unclean shutdown.

Additionally, the metric will be useful for determining how much of a
memory hog the predictor is, with its opaque trimming mechanism.

BUG=620466
Committed: https://crrev.com/d0da3d1035c13406324b7037a7138790b783e8f5
Cr-Commit-Position: refs/heads/master@{#400884}

[Charlie Harrison, 2016-06-20 16:43:47]

csharrison@chromium.org changed reviewers:
+ holte@chromium.org, mmenke@chromium.org

[Charlie Harrison, 2016-06-20 16:43:48]

mmenke@, here's the initial patch to add bytes used by the predictor at startup.
I added a method to log startup metrics in general because we'll likely add more
later like avg navigation count, etc.

[mmenke, 2016-06-20 17:01:24]

LGTM

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
File chrome/browser/net/predictor.cc (right):

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1180: total_bytes +=
referrer.first.spec().size();
Could optionally add in in sizeof(ReferrerValue) here.

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1181: for (const auto& subresource:
referrer.second) {
nit:  Space before colon

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1187: 1000000000, 50);
Suggest reducing this to 10 MB to get a finer breakdown.  Also, please write
this as X * 1000 * 1000, to make it easier to read.

[Charlie Harrison, 2016-06-20 17:26:37]

Thanks!
holte@, could you look at histograms.xml please?

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
File chrome/browser/net/predictor.cc (right):

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1180: total_bytes +=
referrer.first.spec().size();
On 2016/06/20 17:01:24, mmenke wrote:
> Could optionally add in in sizeof(ReferrerValue) here.

Isn't that what sizeof(subresource.second) is doing below? Updated the code to
be a bit clear and added sizeof(Referrer) as well.

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1181: for (const auto& subresource:
referrer.second) {
On 2016/06/20 17:01:24, mmenke wrote:
> nit:  Space before colon

Done.

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1187: 1000000000, 50);
On 2016/06/20 17:01:24, mmenke wrote:
> Suggest reducing this to 10 MB to get a finer breakdown.  Also, please write
> this as X * 1000 * 1000, to make it easier to read.

Done.

[mmenke, 2016-06-20 17:33:52]

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
File chrome/browser/net/predictor.cc (right):

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1180: total_bytes +=
referrer.first.spec().size();
On 2016/06/20 17:26:37, csharrison wrote:
> On 2016/06/20 17:01:24, mmenke wrote:
> > Could optionally add in in sizeof(ReferrerValue) here.
> 
> Isn't that what sizeof(subresource.second) is doing below? Updated the code to
> be a bit clear and added sizeof(Referrer) as well.

Sorry, I meant sizeof(Referrer).  I find the object names here completely
unintuitive.  :(

[Charlie Harrison, 2016-06-20 17:35:13]

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
File chrome/browser/net/predictor.cc (right):

https://codereview.chromium.org/2082653003/diff/1/chrome/browser/net/predicto...
chrome/browser/net/predictor.cc:1180: total_bytes +=
referrer.first.spec().size();
On 2016/06/20 17:33:52, mmenke wrote:
> On 2016/06/20 17:26:37, csharrison wrote:
> > On 2016/06/20 17:01:24, mmenke wrote:
> > > Could optionally add in in sizeof(ReferrerValue) here.
> > 
> > Isn't that what sizeof(subresource.second) is doing below? Updated the code
to
> > be a bit clear and added sizeof(Referrer) as well.
> 
> Sorry, I meant sizeof(Referrer).  I find the object names here completely
> unintuitive.  :(

Agreed. Hoping to clean this up once we stop using Referer headers.

[mmenke, 2016-06-20 18:03:17]

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

https://codereview.chromium.org/2082653003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:28416: +  <summary>The size in bytes of
the predictor's database on startup.</summary>
optional:  May want to say "The approximate size in bytes of..."

[Charlie Harrison, 2016-06-20 19:30:37]

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

https://codereview.chromium.org/2082653003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:28416: +  <summary>The size in bytes of
the predictor's database on startup.</summary>
On 2016/06/20 18:03:17, mmenke wrote:
> optional:  May want to say "The approximate size in bytes of..."

Done.

[Steven Holte, 2016-06-20 22:12:35]

lgtm

[Charlie Harrison, 2016-06-21 02:13:47]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-06-21 02:13:48]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2082653003/#ps40001
(title: "Update histogram desc")

[commit-bot: I haz the power, 2016-06-21 02:14:09]

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

[commit-bot: I haz the power, 2016-06-21 03:40:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-21 03:42:23]

Description was changed from

==========
Log predictor database size on startup

This patch adds UMA about the predictor's database size when it is first
initialized. This will track cases when the pref writing policy fails due
to unclean shutdown.

Additionally, the metric will be useful for determining how much of a
memory hog the predictor is, with its opaque trimming mechanism.

BUG=620466
==========

to

==========
Log predictor database size on startup

This patch adds UMA about the predictor's database size when it is first
initialized. This will track cases when the pref writing policy fails due
to unclean shutdown.

Additionally, the metric will be useful for determining how much of a
memory hog the predictor is, with its opaque trimming mechanism.

BUG=620466

Committed: https://crrev.com/d0da3d1035c13406324b7037a7138790b783e8f5
Cr-Commit-Position: refs/heads/master@{#400884}
==========

[commit-bot: I haz the power, 2016-06-21 03:42:25]

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