Omnibox: Add Histograms to Time Parts of SearchProvider

Omnibox: Add Histograms to Time Parts of SearchProvider

Hopefully these will help me narrow down what part of SearchProvider
is slow.

I didn't add histograms to parts of the code that are only
executed when suggest is enabled.  People reporting the slowness
attest that the slowness appears while suggest is disabled;
I don't need to monitor the suggest section of the code at all.

BUG=237703, 178705, 262263, 168933
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225323

[Mark P, 2013-08-27 23:46:22]

What do you think?

--mark

[msw, 2013-08-29 17:32:24]

https://codereview.chromium.org/23469012/diff/1/chrome/browser/autocomplete/s...
File chrome/browser/autocomplete/search_provider.cc (right):

https://codereview.chromium.org/23469012/diff/1/chrome/browser/autocomplete/s...
chrome/browser/autocomplete/search_provider.cc:639: base::TimeTicks
now(base::TimeTicks::Now());
eliminate |now|

https://codereview.chromium.org/23469012/diff/1/chrome/browser/autocomplete/s...
chrome/browser/autocomplete/search_provider.cc:645:
UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.InMemoryDatabaseTime",
Is it worthwhlie to dig in beyond the overall DoHistoryQueryTime now?

https://codereview.chromium.org/23469012/diff/1/chrome/browser/autocomplete/s...
chrome/browser/autocomplete/search_provider.cc:666:
"Omnibox.SearchProvider.GetMostRecentKeywordTermsDefaultProviderTime",
nit: RecentKeywordTermsTime? I guess names don't matter much anyway...

https://codereview.chromium.org/23469012/diff/1/chrome/browser/autocomplete/s...
chrome/browser/autocomplete/search_provider.cc:1033:
UMA_HISTOGRAM_CUSTOM_COUNTS(
Ditto on the questionable necessity of this given the overall
ConvertResultsTime. A set of fewer high-level data points will help us narrow
our investigation once we have a top-level culprit. I'm not excited to see more
fine grained histograms added now that might just be useless.

https://codereview.chromium.org/23469012/diff/1/chrome/browser/autocomplete/s...
chrome/browser/autocomplete/search_provider.cc:1182:
UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.UpdateStarredTime",
ditto

[msw, 2013-09-25 00:55:22]

LGTM; my comments were really nits and can be ignored.
*Crosses fingers to avoid conflicts and checks the CQ bit.*

[commit-bot: I haz the power, 2013-09-25 01:03:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/23469012/1

[commit-bot: I haz the power, 2013-09-25 02:36:30]

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

[commit-bot: I haz the power, 2013-09-25 03:04:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/23469012/1

[commit-bot: I haz the power, 2013-09-25 08:07:41]

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

[commit-bot: I haz the power, 2013-09-25 13:41:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/23469012/1

[commit-bot: I haz the power, 2013-09-25 16:54:09]

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

[commit-bot: I haz the power, 2013-09-25 22:51:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpearson@chromium.org/23469012/1

[commit-bot: I haz the power, 2013-09-26 04:29:49]

Change committed as 225323

[Mark P, 2013-09-26 19:47:18]

Thanks for submitting this!

It'd be nice to add these histograms to histograms.xml so they show up on the
dashboard.  I can't do it; I'm still on leave.  It's not important though, if
you're willing to dremel for them, or are willing to let this lagginess issue
sit until I get back.

--mark

[msw, 2013-09-26 19:58:18]

Hmm, for some reason I thought this would give us about:histograms reporting so
we could just get reports from users hitting the slowness... I think aggregated
UMA histograms will be far less useful, but I guess I'll add the corresponding
histograms.xml enums since this is already in...

[Mark P, 2013-09-26 22:35:33]

On 2013/09/26 19:58:18, msw wrote:
> Hmm, for some reason I thought this would give us about:histograms reporting
so
> we could just get reports from users hitting the slowness...  I think
aggregated
> UMA histograms will be far less useful, but I guess I'll add the corresponding
> histograms.xml enums since this is already in...

Good point, about:histograms is probably enough. After all, this is for
debugging
on a per particular user basis.  There's really no need for histograms.xml for
this
then.

--mark