Add histogram for image errors.

Add histogram for image errors.
This CL adds a histogram for errors loading images.  It will be 
used to measure how changes impact image request responses.
 
BUG=169182
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197474

[Stephen, 2013-04-26 03:31:02]

PTAL.

[James Simonsen, 2013-04-26 18:21:41]

lgtm

[Ilya Sherman, 2013-04-26 23:59:10]

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
content/browser/loader/resource_dispatcher_host_impl.cc:764: kAllNetErrorCodes,
arraysize(kAllNetErrorCodes)));
Can you use a sparse histogram here?  (The corresponding macro is
UMA_HISTOGRAM_SPARSE_SLOWLY.)

[jar (doing other things), 2013-04-27 00:48:58]

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
content/browser/loader/resource_dispatcher_host_impl.cc:764: kAllNetErrorCodes,
arraysize(kAllNetErrorCodes)));
+1

On 2013/04/26 23:59:10, Ilya Sherman wrote:
> Can you use a sparse histogram here?  (The corresponding macro is
> UMA_HISTOGRAM_SPARSE_SLOWLY.)

[James Simonsen, 2013-04-27 00:57:07]

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
content/browser/loader/resource_dispatcher_host_impl.cc:764: kAllNetErrorCodes,
arraysize(kAllNetErrorCodes)));
On 2013/04/27 00:48:58, jar wrote:
> +1
> 
> On 2013/04/26 23:59:10, Ilya Sherman wrote:
> > Can you use a sparse histogram here?  (The corresponding macro is
> > UMA_HISTOGRAM_SPARSE_SLOWLY.)
> 

If we're going to do that, we should update all of them in this file. I thought
this one was okay because it matched the rest of the file.

[jar (doing other things), 2013-04-27 01:00:53]

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/14093027/diff/1/content/browser/loader/resour...
content/browser/loader/resource_dispatcher_host_impl.cc:764: kAllNetErrorCodes,
arraysize(kAllNetErrorCodes)));
SGTM

The list has gotten long (so memory utilization is high), and this histogram
*should* be rarely augmented (and when it is, it is dwarfed by the compute cost
that got us here typically), so this "map and lock" based histogram is now a
better choice.  The good news is that existing histograms (with 1-wide buckets)
should be completely compatible, so there is no need to change the names, or the
histograms.xml file.


On 2013/04/27 00:57:07, James Simonsen wrote:
> On 2013/04/27 00:48:58, jar wrote:
> > +1
> > 
> > On 2013/04/26 23:59:10, Ilya Sherman wrote:
> > > Can you use a sparse histogram here?  (The corresponding macro is
> > > UMA_HISTOGRAM_SPARSE_SLOWLY.)
> > 
> 
> If we're going to do that, we should update all of them in this file. I
thought
> this one was okay because it matched the rest of the file.

[Stephen, 2013-04-27 03:02:41]

https://chromiumcodereview.appspot.com/14093027/diff/1/content/browser/loader...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://chromiumcodereview.appspot.com/14093027/diff/1/content/browser/loader...
content/browser/loader/resource_dispatcher_host_impl.cc:764: kAllNetErrorCodes,
arraysize(kAllNetErrorCodes)));
OK so IIUC the code here should be
UMA_HISTOGRAM_SPARSE_SLOWLY("Net.ErrorCodesForImages",
-loader->request()->status().error()); and the other CUSTOM_ENUMERATION macros
should be updated similarly.  Is that right?

On 2013/04/27 01:00:53, jar wrote:
> SGTM
> 
> The list has gotten long (so memory utilization is high), and this histogram
> *should* be rarely augmented (and when it is, it is dwarfed by the compute
cost
> that got us here typically), so this "map and lock" based histogram is now a
> better choice.  The good news is that existing histograms (with 1-wide
buckets)
> should be completely compatible, so there is no need to change the names, or
the
> histograms.xml file.
> 
> 
> On 2013/04/27 00:57:07, James Simonsen wrote:
> > On 2013/04/27 00:48:58, jar wrote:
> > > +1
> > > 
> > > On 2013/04/26 23:59:10, Ilya Sherman wrote:
> > > > Can you use a sparse histogram here?  (The corresponding macro is
> > > > UMA_HISTOGRAM_SPARSE_SLOWLY.)
> > > 
> > 
> > If we're going to do that, we should update all of them in this file. I
> thought
> > this one was okay because it matched the rest of the file.
>

[Ilya Sherman, 2013-04-27 23:06:23]

https://chromiumcodereview.appspot.com/14093027/diff/1/content/browser/loader...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://chromiumcodereview.appspot.com/14093027/diff/1/content/browser/loader...
content/browser/loader/resource_dispatcher_host_impl.cc:764: kAllNetErrorCodes,
arraysize(kAllNetErrorCodes)));
On 2013/04/27 03:02:41, Stephen wrote:
> OK so IIUC the code here should be
> UMA_HISTOGRAM_SPARSE_SLOWLY("Net.ErrorCodesForImages",
> -loader->request()->status().error()); and the other CUSTOM_ENUMERATION macros
> should be updated similarly.  Is that right?

Assuming that the expression "-loader->request()->status().error()" gives the
net error code, then yes :)
 
> On 2013/04/27 01:00:53, jar wrote:
> > SGTM
> > 
> > The list has gotten long (so memory utilization is high), and this histogram
> > *should* be rarely augmented (and when it is, it is dwarfed by the compute
> cost
> > that got us here typically), so this "map and lock" based histogram is now a
> > better choice.  The good news is that existing histograms (with 1-wide
> buckets)
> > should be completely compatible, so there is no need to change the names, or
> the
> > histograms.xml file.
> > 
> > 
> > On 2013/04/27 00:57:07, James Simonsen wrote:
> > > On 2013/04/27 00:48:58, jar wrote:
> > > > +1
> > > > 
> > > > On 2013/04/26 23:59:10, Ilya Sherman wrote:
> > > > > Can you use a sparse histogram here?  (The corresponding macro is
> > > > > UMA_HISTOGRAM_SPARSE_SLOWLY.)
> > > > 
> > > 
> > > If we're going to do that, we should update all of them in this file. I
> > thought
> > > this one was okay because it matched the rest of the file.

Yeah, sparse histograms are relatively new, so there's a lot of existing
histograms like these that would be good to move over.

[Stephen, 2013-04-29 00:00:31]

OK updated; please take another look.

[James Simonsen, 2013-04-29 21:13:45]

still lgtm

[commit-bot: I haz the power, 2013-04-30 03:12:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skonig@chromium.org/14093027/8001

[commit-bot: I haz the power, 2013-04-30 03:20:24]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Stephen, 2013-04-30 03:22:55]

jar@ if you could take a look and LGTM the histograms.xml file change please. 
Thanks!

[jar (doing other things), 2013-04-30 19:57:19]

lgtm

[commit-bot: I haz the power, 2013-04-30 20:15:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skonig@chromium.org/14093027/8001

[commit-bot: I haz the power, 2013-04-30 21:22:53]

Change committed as 197474