Omnibox - Boost Frequency Scores Based on Number of Matching Pages

components/omnibox/browser/omnibox_field_trial.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_FIELD_TRIAL_H_
 #define COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_FIELD_TRIAL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <map>
 #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"
 #include "components/omnibox/browser/autocomplete_match_type.h"
 
 namespace base {
 class TimeDelta;
 }
 
 // The set of parameters customizing the HUP scoring.
@@ skipping 71 matching lines @@
 };
 
 // This class manages the Omnibox field trials.
 class OmniboxFieldTrial {
  public:
   // A mapping that contains multipliers indicating that matches of the
   // specified type should have their relevance score multiplied by the
   // 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 {
     EMPHASIZE_WHEN_NONEMPTY = 0,
     EMPHASIZE_WHEN_TITLE_MATCHES = 1,
     EMPHASIZE_WHEN_ONLY_TITLE_MATCHES = 2,
     EMPHASIZE_NEVER = 3
   };
 
   // ---------------------------------------------------------
@@ skipping 186 matching lines @@
   static bool HQPFreqencyUsesSum();
 
   // Returns the number of visits HQP should use when computing frequency
   // scores.  Returns 10 if the epxeriment isn't active.
   static size_t HQPMaxVisitsToScore();
 
   // Returns the score that should be given to typed transitions.  (The score
   // 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.
 
   // Returns the number of title words that are allowed to contribute
   // to the topicality score.  Words later in the title are ignored.
   // Returns 20 as a default if the experiment isn't active.
   static size_t HQPNumTitleWordsToAllow();
 
   // ---------------------------------------------------------
@@ skipping 60 matching lines @@
   static const char kHQPAllowMatchInSchemeRule[];
   static const char kZeroSuggestRule[];
   static const char kZeroSuggestVariantRule[];
   static const char kSuggestVariantRule[];
   static const char kDisableResultsCachingRule[];
   static const char kMeasureSuggestPollingDelayFromLastKeystrokeRule[];
   static const char kSuggestPollingDelayMsRule[];
   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[];
   static const char kPreventUWYTDefaultForNonURLInputsRule[];
   static const char kKeywordRequiresRegistryRule[];
   static const char kKeywordRequiresPrefixMatchRule[];
   static const char kKeywordScoreForSufficientlyCompleteMatchRule[];
   static const char kHQPAllowDupMatchesForScoringRule[];
   static const char kEmphasizeTitlesRule[];
 
@@ skipping 37 matching lines @@
   // prioritize different wildcard contexts, see the implementation.  How to
   // interpret the value is left to the caller; this is rule-dependent.
   static std::string GetValueForRuleInContext(
       const std::string& rule,
       metrics::OmniboxEventProto::PageClassification page_classification);
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(OmniboxFieldTrial);
 };
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_FIELD_TRIAL_H_