Limit the number of history urls indexed for omnibox suggestions.

components/omnibox/browser/url_index_private_data.cc

Index: components/omnibox/browser/url_index_private_data.cc
diff --git a/components/omnibox/browser/url_index_private_data.cc b/components/omnibox/browser/url_index_private_data.cc
index b9f2a93ea945bc2e73e33afdb396d0bd0ef8a975..d25ff9bb6feceec8ff2e4473dc17d2b66c53201b 100644
--- a/components/omnibox/browser/url_index_private_data.cc
+++ b/components/omnibox/browser/url_index_private_data.cc
@@ -397,8 +397,19 @@ scoped_refptr<URLIndexPrivateData> URLIndexPrivateData::RebuildFromHistory(
   history::URLDatabase::URLEnumerator history_enum;
   if (!history_db->InitURLEnumeratorForSignificant(&history_enum))
     return nullptr;
+
   rebuilt_data->last_time_rebuilt_from_history_ = base::Time::Now();
+
+  // Limiting the number of URLs indexed degrades the quality of suggestions to
+  // save memory. This limit is only applied for urls indexed at startup and
+  // more urls can be indexed during the browsing session. The primary use case
+  // is for Android devices where the session is typically short.
+  int max_urls_indexed = OmniboxFieldTrial::MaxNumHistoryUrlsIndexedAtStartup();

[Mark P, 2017/05/08 22:39:17]

nit: const

[Mark P, 2017/05/08 22:39:17]

I don't think our field trial config can make an experiment target low-end
devices explicitly.  I think you might want code here (or perhaps in the
omnibox_field_trial.cc implementation of the function to return -1 on those
devices?) that restricts the trial to just those devices.

[ssid, 2017/05/10 01:01:35]

Done.

[ssid, 2017/05/10 01:01:35]

I was thinking Lets first experiment for all devices and see how the suggestions
are affected.

I looked at History.InMemoryTypedUrlVisitCount and it shows more than 100k urls
in some cases. If we can find a sweet spot to limit this number for suggestions
then we could leave those devices little more usable. Once we find the limit I
think we could remove this experiment and set hard limits based on low-end /
high end devices.

wdyt?

[Mark P, 2017/05/10 17:23:09]

I'm reluctant.  We recently had discussions and complaints that led to filing
bugs 692215 and 692386, which both involve allowing more items to go into the
index.  Doing this blanket on all devices or platforms feels like it's moving in
the wrong direction.

Also, experiment config cannot target low-end devices explicitly.  Unless you
put in code now, you cannot decide to do that later without another code change
and rollout.

If you feel strongly about wanting to consider targeting all devices, I suggest
you expand the parameters your experiment looks at, e.g., have two parameters
MaxNumHQPUrlsIndexedAtStartupOnLowEndDevices and
MaxNumHQPUrlsIndexedAtStartupOnNonLowEndDevices and have the function
MaxNumHQPUrlsIndexedAtStartup() examine the appropriate one depending on device
settings.

[ssid, 2017/05/10 19:47:58]

Okay I am only looking to fix the very high numbers in UMA. Maybe a limit of say
1000 or even 10000 would be good, which I would suspect not change the quality.
Yes, this was my initial plan. To change the code later.
Thanks! Made it 2 different params as you suggested.

+  int num_urls_indexed = 0;
   for (history::URLRow row; history_enum.GetNextURL(&row);) {
+    // Do not use <= to account for case of -1 for unlimited urls.
+    if (num_urls_indexed++ == max_urls_indexed)
+      break;
     rebuilt_data->IndexRow(
         history_db, nullptr, row, scheme_whitelist, nullptr);
   }