Omnibox - Boost Frequency Scores Based on Number of Matching Pages

components/omnibox/browser/omnibox_field_trial.h

Index: components/omnibox/browser/omnibox_field_trial.h
diff --git a/components/omnibox/browser/omnibox_field_trial.h b/components/omnibox/browser/omnibox_field_trial.h
index c4c6afe7d1a2e68a7720a87ff9c0894b9d979ab5..18da22603848fa655c6f92db12d81606e5d045de 100644
--- a/components/omnibox/browser/omnibox_field_trial.h
+++ b/components/omnibox/browser/omnibox_field_trial.h
@@ -12,6 +12,7 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/hash_tables.h"
 #include "base/macros.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
 #include "components/metrics/proto/omnibox_input_type.pb.h"
@@ -103,6 +104,11 @@ class OmniboxFieldTrial {
   // given number.  Omitted types are assumed to have multipliers of 1.0.
   typedef std::map<AutocompleteMatchType::Type, float> DemotionMultipliers;
 
+  // A mapping from the number of matching pages to the multiplier by which
+  // all those matching pages' frequency scores should be multiplied.
+  // Omitted values are assumed to have multipliers of 1.0.
+  typedef base::hash_map<size_t, double> NumMatchesMultipliers;

[Peter Kasting, 2016/12/01 07:07:53]

I don't know that I love the data structure choice here.  Leaving aside whether
a map vs. a hash_map makes sense, why not a simple vector of pair<size_t,
double> in sorted order?  If you expect the size to be a couple of elements
lookups will probably be slightly faster than either map type; if it will be
larger, you could use a binary search to match the algorithmic efficiency of a
map but with better cache behavior; and most importantly, the nature of the data
structure implies that it's tracking ranges rather than exact values, so if you
just send down "1:2,3:1.5", it implies that 2 should be mapped to 2.0, which
makes more sense than mapping it to 1.0 as the hash_map suggests.

[Mark P, 2016/12/04 01:06:42]

Sure.  I debated data structures with myself.  Happy to change to the other.

+
   // Do not change these values as they need to be in sync with values
   // specified in experiment configs on the variations server.
   enum EmphasizeTitlesCondition {
@@ -309,6 +315,11 @@ class OmniboxFieldTrial {
   // of non-typed transitions is 1.)  Returns 20 if the experiment isn't active.
   static float HQPTypedValue();
 
+  // Populates |num_matches_multipliers|.  See comment by the declaration of
+  // NumMatchesMultipliers.  Left empty if not set.
+  static void HQPGetNumMatchesMultipliers(
+      NumMatchesMultipliers* num_matches_multipliers);

[Peter Kasting, 2016/12/01 07:07:53]

Nit: You could probably return the multipliers by value rather than by
outpointer, since this will be done once.  (I tend to find such code easier to
read.)

[Mark P, 2016/12/04 01:06:42]

Okay.  I guess it's small enough to copy off the stack.  (I assume that's what
gets done.)  I agree with you on readability.

+
   // ---------------------------------------------------------
   // For the HQPNumTitleWords experiment that's part of the
   // bundled omnibox field trial.
@@ -389,6 +400,7 @@ class OmniboxFieldTrial {
   static const char kHQPFixFewVisitsBugRule[];
   static const char kHQPFreqencyUsesSumRule[];
   static const char kHQPMaxVisitsToScoreRule[];
+  static const char kHQPNumMatchesMultipliersRule[];
   static const char kHQPNumTitleWordsRule[];
   static const char kHQPAlsoDoHUPLikeScoringRule[];
   static const char kHUPSearchDatabaseRule[];