Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1278)

Unified Diff: base/metrics/field_trial.cc

Issue 23710041: Fix inconsistent FieldTrial group assignment due to float errors. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Created 7 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/metrics/field_trial.h ('k') | base/metrics/field_trial_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/metrics/field_trial.cc
===================================================================
--- base/metrics/field_trial.cc (revision 223657)
+++ base/metrics/field_trial.cc (working copy)
@@ -4,6 +4,8 @@
#include "base/metrics/field_trial.h"
+#include <algorithm>
+
#include "base/build_time.h"
#include "base/logging.h"
#include "base/rand_util.h"
@@ -38,10 +40,27 @@
return Time::FromLocalExploded(exploded);
}
+// Returns the boundary value for comparing against the FieldTrial's added
+// groups for a given |divisor| (total probability) and |entropy_value|.
+FieldTrial::Probability GetGroupBoundaryValue(
+ FieldTrial::Probability divisor,
+ double entropy_value) {
+ // Add a tiny epsilon value to get consistent results when converting floating
+ // points to int. Without it, boundary values have inconsistent results, e.g.:
+ //
+ // static_cast<FieldTrial::Probability>(100 * 0.56) == 56
+ // static_cast<FieldTrial::Probability>(100 * 0.57) == 56
+ // static_cast<FieldTrial::Probability>(100 * 0.58) == 57
+ // static_cast<FieldTrial::Probability>(100 * 0.59) == 59
+ const double kEpsilon = 1e-8;
+ const FieldTrial::Probability result =
+ static_cast<FieldTrial::Probability>(divisor * entropy_value + kEpsilon);
+ // Ensure that adding the epsilon still results in a value < |divisor|.
+ return std::min(result, divisor - 1);
+}
+
} // namespace
-static const char kHistogramFieldTrialSeparator('_');
-
// statics
const int FieldTrial::kNotFinalized = -1;
const int FieldTrial::kDefaultGroupNumber = 0;
@@ -60,7 +79,7 @@
: trial_name_(trial_name),
divisor_(total_probability),
default_group_name_(default_group_name),
- random_(static_cast<Probability>(divisor_ * entropy_value)),
+ random_(GetGroupBoundaryValue(total_probability, entropy_value)),
accumulated_group_probability_(0),
next_group_number_(kDefaultGroupNumber + 1),
group_(kNotFinalized),
« no previous file with comments | « base/metrics/field_trial.h ('k') | base/metrics/field_trial_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698