diff options
author | stevet@chromium.org <stevet@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-27 21:11:03 +0000 |
---|---|---|
committer | stevet@chromium.org <stevet@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-27 21:11:03 +0000 |
commit | ad2461c8447916a49c6d6fd917da9476228d5a43 (patch) | |
tree | a39227119eb2bd0f085ce363e6864249f6c6c058 /base/metrics | |
parent | 20bc8b9a5205386a13c4fa2618ad265b007728b4 (diff) | |
download | chromium_src-ad2461c8447916a49c6d6fd917da9476228d5a43.zip chromium_src-ad2461c8447916a49c6d6fd917da9476228d5a43.tar.gz chromium_src-ad2461c8447916a49c6d6fd917da9476228d5a43.tar.bz2 |
Remove the hash fields from FieldTrials.
We want to migrate the hash fields and related methods from FieldTrials over to experiments_helper. We've also updated the unit tests that accomodate these changes.
We've also refactored the experiments_helper APIs for GoogleExperimentIDs to take strings instead of NameGroupIds as keys... we do the hashing internally instead.
BUG=None
TEST=Ensure that base_unittests FieldTrialTest.* all pass. Ensure that unit_tests ExperimentsHelperTest.* all pass.
Review URL: http://codereview.chromium.org/10151017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134350 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base/metrics')
-rw-r--r-- | base/metrics/field_trial.cc | 54 | ||||
-rw-r--r-- | base/metrics/field_trial.h | 51 | ||||
-rw-r--r-- | base/metrics/field_trial_unittest.cc | 63 |
3 files changed, 46 insertions, 122 deletions
diff --git a/base/metrics/field_trial.cc b/base/metrics/field_trial.cc index e53fdf3..f447c1f 100644 --- a/base/metrics/field_trial.cc +++ b/base/metrics/field_trial.cc @@ -20,7 +20,6 @@ static const char kHistogramFieldTrialSeparator('_'); // statics const int FieldTrial::kNotFinalized = -1; const int FieldTrial::kDefaultGroupNumber = 0; -const uint32 FieldTrial::kReservedHashValue = 0; bool FieldTrial::enable_benchmarking_ = false; const char FieldTrialList::kPersistentStringSeparator('/'); @@ -36,14 +35,12 @@ FieldTrial::FieldTrial(const std::string& name, const int month, const int day_of_month) : name_(name), - name_hash_(HashName(name)), divisor_(total_probability), default_group_name_(default_group_name), random_(static_cast<Probability>(divisor_ * RandDouble())), accumulated_group_probability_(0), next_group_number_(kDefaultGroupNumber + 1), group_(kNotFinalized), - group_name_hash_(kReservedHashValue), enable_field_trial_(true), forced_(false) { DCHECK_GT(total_probability, 0); @@ -151,11 +148,11 @@ std::string FieldTrial::group_name() { return group_name_; } -bool FieldTrial::GetNameGroupId(NameGroupId* name_group_id) { - if (group_name_hash_ == kReservedHashValue) +bool FieldTrial::GetSelectedGroup(SelectedGroup* selected_group) { + if (group_ == kNotFinalized) return false; - name_group_id->name = name_hash_; - name_group_id->group = group_name_hash_; + selected_group->trial = name_; + selected_group->group = group_name_; return true; } @@ -168,16 +165,6 @@ std::string FieldTrial::MakeName(const std::string& name_prefix, } // static -FieldTrial::NameGroupId FieldTrial::MakeNameGroupId( - const std::string& trial_name, - const std::string& group_name) { - NameGroupId id; - id.name = HashName(trial_name); - id.group = HashName(group_name); - return id; -} - -// static void FieldTrial::EnableBenchmarking() { DCHECK_EQ(0u, FieldTrialList::GetFieldTrialCount()); enable_benchmarking_ = true; @@ -191,7 +178,6 @@ void FieldTrial::SetGroupChoice(const std::string& name, int number) { StringAppendF(&group_name_, "%d", group_); else group_name_ = name; - group_name_hash_ = HashName(group_name_); } // static @@ -215,26 +201,6 @@ double FieldTrial::HashClientId(const std::string& client_id, return BitsToOpenEndedUnitInterval(bits); } -// static -uint32 FieldTrial::HashName(const std::string& name) { - // SHA-1 is designed to produce a uniformly random spread in its output space, - // even for nearly-identical inputs. - unsigned char sha1_hash[kSHA1Length]; - SHA1HashBytes(reinterpret_cast<const unsigned char*>(name.c_str()), - name.size(), - sha1_hash); - - COMPILE_ASSERT(sizeof(uint32) < sizeof(sha1_hash), need_more_data); - uint32 bits; - memcpy(&bits, sha1_hash, sizeof(bits)); - - // We only DCHECK, since this should not happen because the registration - // of the experiment on the server should have already warn the developer. - // If this ever happen, we'll ignore this experiment/group when reporting. - DCHECK(bits != kReservedHashValue); - return base::ByteSwapToLE32(bits); -} - //------------------------------------------------------------------------------ // FieldTrialList methods and members. @@ -352,18 +318,18 @@ void FieldTrialList::StatesToString(std::string* output) { } // static -void FieldTrialList::GetFieldTrialNameGroupIds( - std::vector<FieldTrial::NameGroupId>* name_group_ids) { - DCHECK(name_group_ids->empty()); +void FieldTrialList::GetFieldTrialSelectedGroups( + FieldTrial::SelectedGroups* selected_groups) { + DCHECK(selected_groups->empty()); if (!global_) return; AutoLock auto_lock(global_->lock_); for (RegistrationList::iterator it = global_->registered_.begin(); it != global_->registered_.end(); ++it) { - FieldTrial::NameGroupId name_group_id; - if (it->second->GetNameGroupId(&name_group_id)) - name_group_ids->push_back(name_group_id); + FieldTrial::SelectedGroup selected_group; + if (it->second->GetSelectedGroup(&selected_group)) + selected_groups->push_back(selected_group); } } diff --git a/base/metrics/field_trial.h b/base/metrics/field_trial.h index be58a93..e1b30b9 100644 --- a/base/metrics/field_trial.h +++ b/base/metrics/field_trial.h @@ -77,6 +77,7 @@ #include <map> #include <string> +#include <vector> #include "base/base_export.h" #include "base/gtest_prod_util.h" @@ -92,13 +93,15 @@ class FieldTrialList; class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { public: typedef int Probability; // Probability type for being selected in a trial. - // The Unique ID of a trial, where the name and group identifiers are - // hashes of the trial and group name strings. - struct NameGroupId { - uint32 name; - uint32 group; + + // A pair representing a Field Trial and its selected group. + struct SelectedGroup { + std::string trial; + std::string group; }; + typedef std::vector<SelectedGroup> SelectedGroups; + // A return value to indicate that a given instance has not yet had a group // assignment (and hence is not yet participating in the trial). static const int kNotFinalized; @@ -135,21 +138,17 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { // is used as the group name. This causes a winner to be chosen if none was. std::string group_name(); - // Gets the unique identifier of the Field Trial, but only if a group was + // Gets the SelectedGroup of the Field Trial, but only if a group was // officially chosen, otherwise name_group_id is left untouched and false - // is returned. When true is returned, the name and group ids were - // successfully set in name_group_id. - bool GetNameGroupId(NameGroupId* name_group_id); + // is returned. When true is returned, the trial and group names were + // successfully set in selected_group. + bool GetSelectedGroup(SelectedGroup* selected_group); // Helper function for the most common use: as an argument to specify the // name of a HISTOGRAM. Use the original histogram name as the name_prefix. static std::string MakeName(const std::string& name_prefix, const std::string& trial_name); - // Helper function to create a NameGroupId from |trial_name| and |group_name|. - static NameGroupId MakeNameGroupId(const std::string& trial_name, - const std::string& group_name); - // Enable benchmarking sets field trials to a common setting. static void EnableBenchmarking(); @@ -167,7 +166,6 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, MakeName); FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, HashClientId); FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, HashClientIdIsUniform); - FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, HashName); FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, NameGroupIds); FRIEND_TEST_ALL_PREFIXES(FieldTrialTest, UseOneTimeRandomization); @@ -200,16 +198,9 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { static double HashClientId(const std::string& client_id, const std::string& trial_name); - // Creates unique identifier for the trial by hashing a name string, whether - // it's for the field trial or the group name. - static uint32 HashName(const std::string& name); - // The name of the field trial, as can be found via the FieldTrialList. const std::string name_; - // The hashed name of the field trial to be sent as a unique identifier. - const uint32 name_hash_; - // The maximum sum of all probabilities supplied, which corresponds to 100%. // This is the scaling factor used to adjust supplied probabilities. const Probability divisor_; @@ -235,10 +226,6 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { // has been called. std::string group_name_; - // The hashed name of the group to be sent as a unique identifier. - // Is not valid while group_ is equal to kNotFinalized. - uint32 group_name_hash_; - // When enable_field_trial_ is false, field trial reverts to the 'default' // group. bool enable_field_trial_; @@ -251,9 +238,6 @@ class BASE_EXPORT FieldTrial : public RefCounted<FieldTrial> { // group. static bool enable_benchmarking_; - // This value is reserved for an uninitialized hash value. - static const uint32 kReservedHashValue; - DISALLOW_COPY_AND_ASSIGN(FieldTrial); }; @@ -343,11 +327,12 @@ class BASE_EXPORT FieldTrialList { // string is parsed by CreateTrialsFromString(). static void StatesToString(std::string* output); - // Returns an array of Unique IDs for each Field Trial that has a chosen - // group. Field Trials for which a group has not been chosen yet are NOT - // returned in this list. - static void GetFieldTrialNameGroupIds( - std::vector<FieldTrial::NameGroupId>* name_group_ids); + // Fills in the supplied vector |selected_groups| (which must be empty when + // called) with a snapshot of all existing FieldTrials for which a group has + // been chosen (if the group is not yet known, then it excluded from the + // vector). + static void GetFieldTrialSelectedGroups( + FieldTrial::SelectedGroups* selected_groups); // Use a state string (re: StatesToString()) to augment the current list of // field tests to include the supplied tests, and using a 100% probability for diff --git a/base/metrics/field_trial_unittest.cc b/base/metrics/field_trial_unittest.cc index 9c50fc5..51e9aaa 100644 --- a/base/metrics/field_trial_unittest.cc +++ b/base/metrics/field_trial_unittest.cc @@ -227,51 +227,26 @@ TEST_F(FieldTrialTest, DisableProbability) { EXPECT_EQ(default_group_name, trial->group_name()); } -TEST_F(FieldTrialTest, HashName) { - // Make sure hashing is stable on all platforms. - struct { - const char* name; - uint32 hash_value; - } known_hashes[] = { - {"a", 937752454u}, - {"1", 723085877u}, - {"Trial Name", 2713117220u}, - {"Group Name", 3201815843u}, - {"My Favorite Experiment", 3722155194u}, - {"My Awesome Group Name", 4109503236u}, - {"abcdefghijklmonpqrstuvwxyz", 787728696u}, - {"0123456789ABCDEF", 348858318U} - }; - for (size_t i = 0; i < ARRAYSIZE_UNSAFE(known_hashes); ++i) { - EXPECT_EQ(known_hashes[i].hash_value, - FieldTrial::HashName(known_hashes[i].name)); - } -} - -TEST_F(FieldTrialTest, NameGroupIds) { +TEST_F(FieldTrialTest, SelectedGroups) { std::string no_group("No Group"); - uint32 no_group_id = FieldTrial::HashName(no_group); scoped_refptr<FieldTrial> trial(FieldTrialList::FactoryGetFieldTrial( no_group, 10, "Default", next_year_, 12, 31, NULL)); // There is no winner yet, so no NameGroupId should be returned. - FieldTrial::NameGroupId name_group_id; - EXPECT_FALSE(trial->GetNameGroupId(&name_group_id)); + FieldTrial::SelectedGroup selected_group; + EXPECT_FALSE(trial->GetSelectedGroup(&selected_group)); // Create a single winning group. std::string one_winner("One Winner"); - uint32 one_winner_id = FieldTrial::HashName(one_winner); trial = FieldTrialList::FactoryGetFieldTrial( one_winner, 10, "Default", next_year_, 12, 31, NULL); std::string winner("Winner"); - uint32 winner_group_id = FieldTrial::HashName(winner); trial->AppendGroup(winner, 10); - EXPECT_TRUE(trial->GetNameGroupId(&name_group_id)); - EXPECT_EQ(one_winner_id, name_group_id.name); - EXPECT_EQ(winner_group_id, name_group_id.group); + EXPECT_TRUE(trial->GetSelectedGroup(&selected_group)); + EXPECT_EQ(one_winner, selected_group.trial); + EXPECT_EQ(winner, selected_group.group); std::string multi_group("MultiGroup"); - uint32 multi_group_id = FieldTrial::HashName(multi_group); scoped_refptr<FieldTrial> multi_group_trial = FieldTrialList::FactoryGetFieldTrial(multi_group, 9, "Default", next_year_, 12, 31, NULL); @@ -279,23 +254,21 @@ TEST_F(FieldTrialTest, NameGroupIds) { multi_group_trial->AppendGroup("Me", 3); multi_group_trial->AppendGroup("You", 3); multi_group_trial->AppendGroup("Them", 3); - EXPECT_TRUE(multi_group_trial->GetNameGroupId(&name_group_id)); - EXPECT_EQ(multi_group_id, name_group_id.name); - uint32 multi_group_winner_id = - FieldTrial::HashName(multi_group_trial->group_name()); - EXPECT_EQ(multi_group_winner_id, name_group_id.group); + EXPECT_TRUE(multi_group_trial->GetSelectedGroup(&selected_group)); + EXPECT_EQ(multi_group, selected_group.trial); + EXPECT_EQ(multi_group_trial->group_name(), selected_group.group); // Now check if the list is built properly... - std::vector<FieldTrial::NameGroupId> name_group_ids; - FieldTrialList::GetFieldTrialNameGroupIds(&name_group_ids); - EXPECT_EQ(2U, name_group_ids.size()); - for (size_t i = 0; i < name_group_ids.size(); ++i) { + std::vector<FieldTrial::SelectedGroup> selected_groups; + FieldTrialList::GetFieldTrialSelectedGroups(&selected_groups); + EXPECT_EQ(2U, selected_groups.size()); + for (size_t i = 0; i < selected_groups.size(); ++i) { // Order is not guaranteed, so check all values. - EXPECT_NE(no_group_id, name_group_ids[i].name); - EXPECT_TRUE(one_winner_id != name_group_ids[i].name || - winner_group_id == name_group_ids[i].group); - EXPECT_TRUE(multi_group_id != name_group_ids[i].name || - multi_group_winner_id == name_group_ids[i].group); + EXPECT_NE(no_group, selected_groups[i].trial); + EXPECT_TRUE(one_winner != selected_groups[i].trial || + winner == selected_groups[i].group); + EXPECT_TRUE(multi_group != selected_groups[i].trial || + multi_group_trial->group_name() == selected_groups[i].group); } } |