Add UMA for disk latency for cache accesses.

Add UMA for disk latency for cache accesses.

BUG=487749
Committed: https://crrev.com/50fb5bbcda74bd2bb7428f82a97f2c43a9ef9690
Cr-Commit-Position: refs/heads/master@{#329857}

[Randy Smith (Not in Mondays), 2015-05-13 20:14:41]

rdsmith@chromium.org changed reviewers:
+ gavinp@chromium.org, ttuttle@chromium.org

[Randy Smith (Not in Mondays), 2015-05-13 20:14:41]

rdsmith@chromium.org changed required reviewers:
+ ttuttle@chromium.org

[Randy Smith (Not in Mondays), 2015-05-13 20:14:41]

Gavin: Comments welcome and invited; you aren't primary reviewer just because
I'm not sure what level of access you have to the net at the moment.

Thomas, PTAL?

[Deprecated (see juliatuttle), 2015-05-13 20:21:38]

https://codereview.chromium.org/1134353003/diff/1/net/disk_cache/simple/simpl...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/1134353003/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_synchronous_entry.cc:221: base::Time
open_start_time(base::Time::Now());
Perhaps use TimeTicks? I am under the impression it is better for things that
don't need wallclock time, as it doesn't jump around when the system clock
changes.

https://codereview.chromium.org/1134353003/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_synchronous_entry.cc:236:
UMA_HISTOGRAM_TIMES("SimpleCache.DiskOpenLatency",
This looks good as long as you only care about open latency, but you'd mentioned
wanting to compare it to time-to-first-byte, which would correspond with the
first *data* read. I don't know how big a difference that would be.

[Randy Smith (Not in Mondays), 2015-05-13 20:32:52]

Incorporated comment; PTAL?

https://codereview.chromium.org/1134353003/diff/1/net/disk_cache/simple/simpl...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/1134353003/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_synchronous_entry.cc:221: base::Time
open_start_time(base::Time::Now());
On 2015/05/13 20:21:38, ttuttle wrote:
> Perhaps use TimeTicks? I am under the impression it is better for things that
> don't need wallclock time, as it doesn't jump around when the system clock
> changes.

Oh, good point.  I haven't done this in too long, and clearly forget things
easily.  Done.

https://codereview.chromium.org/1134353003/diff/1/net/disk_cache/simple/simpl...
net/disk_cache/simple/simple_synchronous_entry.cc:236:
UMA_HISTOGRAM_TIMES("SimpleCache.DiskOpenLatency",
On 2015/05/13 20:21:38, ttuttle wrote:
> This looks good as long as you only care about open latency, but you'd
mentioned
> wanting to compare it to time-to-first-byte, which would correspond with the
> first *data* read. I don't know how big a difference that would be.

So Bryan and I chatted, and decided we'd be mostly comparing this against
Net.TCP_ConnectionLatency, which is probably also not quite right, but at least
gets to the same "get ready to read the bytes" stage.    I'm not sure there's
really an easy way to compare time to first byte disk cache vs. network without
a fair amount more work than will happen for branch.

[Deprecated (see juliatuttle), 2015-05-13 20:50:14]

lgtm!

[Randy Smith (Not in Mondays), 2015-05-13 20:50:57]

On 2015/05/13 20:50:14, ttuttle wrote:
> lgtm!

Thomas: Thanks!

Gavin: Please tell me everything that's wrong with this when you get a chance,
and I'll fix it in a later CL.

[Randy Smith (Not in Mondays), 2015-05-13 20:51:07]

The CQ bit was checked by rdsmith@chromium.org

[commit-bot: I haz the power, 2015-05-13 20:52:29]

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

[commit-bot: I haz the power, 2015-05-13 21:12:07]

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

[commit-bot: I haz the power, 2015-05-13 21:12:08]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Randy Smith (Not in Mondays), 2015-05-13 21:12:59]

rdsmith@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Randy Smith (Not in Mondays), 2015-05-13 21:13:00]

Alexei: PTAL at histograms.xml?

[Alexei Svitkine (slow), 2015-05-14 14:53:05]

lgtm % suggestion

https://codereview.chromium.org/1134353003/diff/20001/net/disk_cache/simple/s...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/1134353003/diff/20001/net/disk_cache/simple/s...
net/disk_cache/simple/simple_synchronous_entry.cc:221: base::TimeTicks
open_start_time(base::TimeTicks::Now());
Nit: Use base/timer/elapsed_timer.h

[Randy Smith (Not in Mondays), 2015-05-14 15:25:25]

Thanks!

https://codereview.chromium.org/1134353003/diff/20001/net/disk_cache/simple/s...
File net/disk_cache/simple/simple_synchronous_entry.cc (right):

https://codereview.chromium.org/1134353003/diff/20001/net/disk_cache/simple/s...
net/disk_cache/simple/simple_synchronous_entry.cc:221: base::TimeTicks
open_start_time(base::TimeTicks::Now());
On 2015/05/14 14:53:05, Alexei Svitkine wrote:
> Nit: Use base/timer/elapsed_timer.h

Done.

[Randy Smith (Not in Mondays), 2015-05-14 15:25:40]

The CQ bit was checked by rdsmith@chromium.org

[Randy Smith (Not in Mondays), 2015-05-14 15:25:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
ttuttle@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1134353003/#ps40001
(title: "Switched to ElapsedTimer.")

[commit-bot: I haz the power, 2015-05-14 15:26:32]

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

[commit-bot: I haz the power, 2015-05-14 16:36:03]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-14 16:36:51]

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