Measure the X-Chrome-Variations construction time with UMA.

Measure the X-Chrome-Variations construction time with UMA.

We measure this value only the first time the header is created.

BUG=163999
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=180418

[SteveT, 2013-01-29 21:40:16]

Hey Jim,

Given you know a bit about such measurements with UMA, does this seem
reasonable?

(Feel free to pass on the review if you're not the right one to ask)


Steve

[SteveT, 2013-01-31 19:29:00]

Ping - have time to take a look?

[jar (doing other things), 2013-02-01 00:50:37]

https://codereview.chromium.org/12090055/diff/1/chrome/browser/metrics/variat...
File chrome/browser/metrics/variations/variations_http_header_provider.cc
(right):

https://codereview.chromium.org/12090055/diff/1/chrome/browser/metrics/variat...
chrome/browser/metrics/variations/variations_http_header_provider.cc:113:
(base::TimeTicks::HighResNow() - before_time).InMicroseconds(),
Since you're converting to microseconds, don't bother with HighResNow(), just
use TimeTicks::Now() here, and on line 96.

We'll get an "average" with lots of zeros, and that will tend to allow us to see
a slightly higher res at the low end (if this is consistently under 1ms).  In
addition, if it is way under 1ms, we shouldn't worry much about it in the field.
 If it is larger... then we'll see clearly... and we'll have something to fret
about.

[SteveT, 2013-02-01 14:17:45]

Okay, back to you.

I feel better about taking this approach then the local benchmarking. I'm
actually seeing more varied results now that we're not just looping it over a
hot cache.

Any feel for how many samples we'd like to see before we can make a statement on
the code's performance?

https://codereview.chromium.org/12090055/diff/1/chrome/browser/metrics/variat...
File chrome/browser/metrics/variations/variations_http_header_provider.cc
(right):

https://codereview.chromium.org/12090055/diff/1/chrome/browser/metrics/variat...
chrome/browser/metrics/variations/variations_http_header_provider.cc:113:
(base::TimeTicks::HighResNow() - before_time).InMicroseconds(),
On 2013/02/01 00:50:37, jar wrote:
> Since you're converting to microseconds, don't bother with HighResNow(), just
> use TimeTicks::Now() here, and on line 96.
> 
> We'll get an "average" with lots of zeros, and that will tend to allow us to
see
> a slightly higher res at the low end (if this is consistently under 1ms).  In
> addition, if it is way under 1ms, we shouldn't worry much about it in the
field.
>  If it is larger... then we'll see clearly... and we'll have something to fret
> about.

That sounds good. Thanks for pointing it out.

Done.

[jar (doing other things), 2013-02-01 17:24:53]

LGTM

Land it, and we'll get a bunch of data.  You'll quickly see from the
distribution when you "have enough" to see through the noise, and have an
answer.

[commit-bot: I haz the power, 2013-02-01 17:44:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevet@chromium.org/12090055/2002

[Bart N., 2013-02-01 17:46:40]

On 2013/02/01 17:24:53, jar wrote:
> LGTM
> 
> Land it, and we'll get a bunch of data.  You'll quickly see from the
> distribution when you "have enough" to see through the noise, and have an
> answer.
Isn't it going to be biased towards canary/dev population which is known to be
highly skewed? Or are you going to wait until this change hits beta/stable?

[commit-bot: I haz the power, 2013-02-01 21:41:38]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-02-01 21:45:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevet@chromium.org/12090055/2002

[commit-bot: I haz the power, 2013-02-04 15:08:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevet@chromium.org/12090055/2002

[commit-bot: I haz the power, 2013-02-04 16:14:26]

Change committed as 180418