random: change Rand64() to use all bits

Rand64() currently fills only 62 bits of the result. This changes the
function to randomize all 64 bits at the cost of a bit of performance.
This is useful in testing encodings which may have edge cases when the
MSB is set.

Change-Id: I6064ae18532bc32f24086e232200f1f0966d6b2a
Reviewed-on: http://gerrit.cloudera.org:8080/5474
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <[email protected]>

src/kudu/util/random-test.cc

@@ -58,11 +58,13 @@ TEST_F(RandomTest, TestNormalDist) {
 // in Next64().
 //
 // Note: Our RNG actually only generates 31 bits of randomness for 32 bit
-// integers and 62 bits for 64 bit integers. So this test reflects that, and if
-// we change the RNG algo this test should also change.
+// integers. If all bits need to be randomized, callers must use Random::Next64().
+// This test reflects that, and if  we change the RNG algo this test should also change.
 TEST_F(RandomTest, TestUseOfBits) {
+  // For Next32():
   uint32_t ones32 = std::numeric_limits<uint32_t>::max();
   uint32_t zeroes32 = 0;
+  // For Next64():
   uint64_t ones64 = std::numeric_limits<uint64_t>::max();
   uint64_t zeroes64 = 0;
 
@@ -76,17 +78,16 @@ TEST_F(RandomTest, TestUseOfBits) {
     zeroes64 |= r64;
   }
 
-  // At the end, we should have flipped 31 and 62 bits, respectively. One
+  // At the end, we should have flipped 31 and 64 bits, respectively. One
   // detail of the current RNG impl is that Next32() always returns a number
-  // with MSB set to 0, and Next64() always returns a number with the first
-  // two bits set to zero.
+  // with MSB set to 0.
   uint32_t expected_bits_31 = std::numeric_limits<uint32_t>::max() >> 1;
-  uint64_t expected_bits_62 = std::numeric_limits<uint64_t>::max() >> 2;
+  uint64_t expected_bits_64 = std::numeric_limits<uint64_t>::max();
 
   ASSERT_EQ(0, ones32);
   ASSERT_EQ(expected_bits_31, zeroes32);
   ASSERT_EQ(0, ones64);
-  ASSERT_EQ(expected_bits_62, zeroes64);
+  ASSERT_EQ(expected_bits_64, zeroes64);
 }
 
 TEST_F(RandomTest, TestResetSeed) {

src/kudu/util/random.h

@@ -10,6 +10,7 @@
 #include <random>
 #include <vector>
 
+#include "kudu/gutil/casts.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/util/locks.h"
 
@@ -73,16 +74,12 @@ class Random {
   uint32_t Next32() { return Next(); }
 
   // Next pseudo-random 64-bit unsigned integer.
-  // FIXME: This currently only generates 62 bits of randomness due to Next()
-  // only giving 31 bits of randomness. The 2 most significant bits will always
-  // be zero.
   uint64_t Next64() {
     uint64_t large = Next();
-    // Only shift by 31 bits so we end up with zeros in MSB and not scattered
-    // throughout the 64-bit word. This is due to the weakness in Next() noted
-    // above.
     large <<= 31;
     large |= Next();
+    // Fill in the highest two MSBs.
+    large |= implicit_cast<uint64_t>(Next32()) << 62;
     return large;
   }