Add Histograms to Measure Time in BrowserMainRunnerImpl::Initialize

Add Histograms to Measure Time in BrowserMainRunnerImpl::Initialize

BUG=454789
Committed: https://crrev.com/7fac1d8a323676e36b32c080b3b344bf9d7e9939
Cr-Commit-Position: refs/heads/master@{#350381}

[robliao, 2015-09-23 00:47:38]

robliao@chromium.org changed reviewers:
+ nick@chromium.org, rkaplow@chromium.org

[robliao, 2015-09-23 00:47:38]

nick@chromium.org: Please review changes in 
content/browser/browser_main_runner.cc

rkaplow@chromium.org: Please review changes in 
tools/metrics/histograms/histograms.xml

Thanks!

[rkaplow, 2015-09-23 14:22:47]

lgtm

[ncarter (slow), 2015-09-23 16:34:19]

lgtm as-is, but two things to think about (I'm happy to do another pass if you
decide either is worth addressing)

https://codereview.chromium.org/1359163004/diff/1/content/browser/browser_mai...
File content/browser/browser_main_runner.cc (right):

https://codereview.chromium.org/1359163004/diff/1/content/browser/browser_mai...
content/browser/browser_main_runner.cc:156: // not run these parts of
initialization twice.
This comment suggests that on Android, step2 might happen more than once. Will
that be a source of measurement bias in the step2 metrics? Similarly, for the
LongTime case, there may be more samples to that histogram than to the Step1
histogram? 

My best guess: this is probably uncommon and we can know for sure if we pay
attention to the relative sample counts for these histograms.

https://codereview.chromium.org/1359163004/diff/1/content/browser/browser_mai...
content/browser/browser_main_runner.cc:165: if
(parameters.command_line.HasSwitch(switches::kWaitForDebugger))
FYI, this may skew the results (though I expect it to be a rare case, it could
explain a significant chunk of outlier samples).

[robliao, 2015-09-23 16:55:13]

Thanks for the comments!

https://codereview.chromium.org/1359163004/diff/1/content/browser/browser_mai...
File content/browser/browser_main_runner.cc (right):

https://codereview.chromium.org/1359163004/diff/1/content/browser/browser_mai...
content/browser/browser_main_runner.cc:156: // not run these parts of
initialization twice.
On 2015/09/23 16:34:19, ncarter wrote:
> This comment suggests that on Android, step2 might happen more than once. Will
> that be a source of measurement bias in the step2 metrics? Similarly, for the
> LongTime case, there may be more samples to that histogram than to the Step1
> histogram? 
> 
> My best guess: this is probably uncommon and we can know for sure if we pay
> attention to the relative sample counts for these histograms.

Agreed. My analysis currently excludes Android as we're focusing on desktop
startup performance. We can revisit when Android becomes more interesting to us.

https://codereview.chromium.org/1359163004/diff/1/content/browser/browser_mai...
content/browser/browser_main_runner.cc:165: if
(parameters.command_line.HasSwitch(switches::kWaitForDebugger))
On 2015/09/23 16:34:19, ncarter wrote:
> FYI, this may skew the results (though I expect it to be a rare case, it could
> explain a significant chunk of outlier samples).

Good catch. I agree with the analysis and if we have a debugger attached a
priori, that would also skew the results a bit. Given that the existing numbers
that capture a superset of this section are reasonable, I would also guess that
this instance is low.

It's certainly a tradeoff between code simplicity and absolute correctness. If
the numbers look wrong, we can revisit.

[robliao, 2015-09-23 16:55:18]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2015-09-23 16:55:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359163004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359163004/1

[commit-bot: I haz the power, 2015-09-23 17:31:52]

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

[commit-bot: I haz the power, 2015-09-23 17:31:53]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[robliao, 2015-09-23 21:17:02]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2015-09-23 21:19:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1359163004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1359163004/1

[commit-bot: I haz the power, 2015-09-23 23:07:48]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-09-23 23:08:33]

Patchset 1 (id:??) landed as
https://crrev.com/7fac1d8a323676e36b32c080b3b344bf9d7e9939
Cr-Commit-Position: refs/heads/master@{#350381}