Fix base::RandGenerator bug (it had non-uniform random distribution).

base/rand_util_unittest.cc

Index: base/rand_util_unittest.cc
diff --git a/base/rand_util_unittest.cc b/base/rand_util_unittest.cc
index d7fa37af827e8589299c4a066ef83308ca4a384a..86a4de147eab3885e429ad09a928feb954cf24d3 100644
--- a/base/rand_util_unittest.cc
+++ b/base/rand_util_unittest.cc
@@ -5,6 +5,7 @@
 #include "base/rand_util.h"
 
 #include <limits>
+#include <vector>
 
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -61,3 +62,80 @@ TEST(RandUtilTest, RandGeneratorForRandomShuffle) {
   EXPECT_LE(std::numeric_limits<ptrdiff_t>::max(),
             std::numeric_limits<int64>::max());
 }
+
+TEST(RandUtilTest, RandGeneratorIsUniform) {
+  // Verify that RandGenerator has a uniform distribution. This is a
+  // regression test that consistently failed when RandGenerator was
+  // implemented this way:
+  //
+  //   return base::RandUint64() % max;
+  //
+  // A degenerate case for such an implementation is e.g. a top of range
+  // that is 2/3rds of the way to MAX_UINT64, in which case the bottom
+  // half of the range would be twice as likely to occur as the top
+  // half.
+  const uint64 kTopOfRange = (std::numeric_limits<uint64>::max() / 3L) * 2L;
+  const uint64 kExpectedAverage = kTopOfRange / 2L;
+  const uint64 kAllowedVariance = kExpectedAverage / 50L;  // +/- 2%
+  const int kMinAttempts = 1000;
+  const int kMaxAttempts = 1000000;
+
+  double cumulative_average = 0.0;
+  int count = 0;
+  while (count < kMaxAttempts) {
+    uint64 value = base::RandGenerator(kTopOfRange);
+    cumulative_average = (count * cumulative_average + value) / (count + 1);
+
+    // Don't quit too quickly for things to start converging, or we may have
+    // a false positive.
+    if (count > kMinAttempts &&
+        kExpectedAverage - kAllowedVariance < cumulative_average &&
+        cumulative_average < kExpectedAverage + kAllowedVariance) {
+      break;
+    }
+
+    ++count;
+  }
+
+  ASSERT_LT(count, kMaxAttempts) << "Expected average was " <<
+      kExpectedAverage << ", average ended at " << cumulative_average;
+}
+
+TEST(RandUtilTest, RandUint64ProducesBothValuesOfAllBits) {
+  // This tests to see that our underlying random generator is good
+  // enough, for some value of good enough.
+  const size_t kNumBits = sizeof(uint64) * 8;
+  std::vector<bool> bits_one(kNumBits, false);
+  std::vector<bool> bits_zero(kNumBits, false);
+  ASSERT_FALSE(bits_one[0]);
+  ASSERT_FALSE(bits_zero[0]);
+
+  bool succeeded = false;
+  for (size_t i = 0; i < 1000000; ++i) {

[jar (doing other things), 2011/05/30 17:02:07]

Each bit *should* have a 50-50 probabilty.  Hence over 100 cycles without
getting a bit to toggle would be ridiculously improbable, and probably 1000
would be wildly conservative.

Then again... this test should never make it even near to that bound unless it
is broken.  The advantage of ending the test on this boundary, is we avoid
running so long that the test simply "doesn't finish" and we have a less clear
failure.

+    uint64 value = base::RandUint64();

[jar (doing other things), 2011/05/30 17:02:07]

FYI: You could have used bit-wise AND and bitwise OR to see how long it took to
get a 0 in each position, and a 1 in each position, rather than the vector
manipulation:
uint64 found_ones = 0x0;  // bit of 1 shows we found a 1.
uint64 found_zeros = ~found_ones;  // bit 0 means found a 0.
while (0 != found_zeros && ~found_ones != 0) {
  int64 value = base::RandInt64();
  found_ones |= value;
  found_zeros &= value;
}

+    for (uint64 shift = 0; shift < kNumBits; ++shift) {
+      uint64 mask = 1ULL << shift;
+      uint64 bit = (value & mask) >> shift;
+      if (bit == 1ULL) {
+        bits_one[shift] = true;
+      } else {
+        ASSERT_EQ(bit, 0ULL);
+        bits_zero[shift] = true;
+      }
+    }
+
+    size_t bit = 0;
+    for (; bit < kNumBits; ++bit) {
+      if (!bits_one[bit] || !bits_zero[bit])
+        break;
+    }
+
+    if (bit == kNumBits) {
+      succeeded = true;
+      break;
+    }
+  }
+
+  ASSERT_TRUE(succeeded) <<
+      "Didn't achieve all bit values in maximum number of tries.";
+}