Improve base::RandBytes() performance by 1.75x-2.10x on POSIX.

base/rand_util_nacl.cc

Index: base/rand_util_nacl.cc
diff --git a/base/rand_util_nacl.cc b/base/rand_util_nacl.cc
index 47450aaba68be254001da2f5e9d84e182b19ed76..e972be8e9d4e5f1f85499adb3ced1fb8e37406fc 100644
--- a/base/rand_util_nacl.cc
+++ b/base/rand_util_nacl.cc
@@ -14,21 +14,21 @@ namespace {
 class NaclRandom {
  public:
   NaclRandom() {
-    size_t result = nacl_interface_query(NACL_IRT_RANDOM_v0_1,
-                                         &random_, sizeof(random_));
+    const size_t result =
+        nacl_interface_query(NACL_IRT_RANDOM_v0_1, &random_, sizeof(random_));
     CHECK_EQ(result, sizeof(random_));
   }
 
-  ~NaclRandom() {
-  }
+  ~NaclRandom() {}
 
-  void GetRandomBytes(char* buffer, uint32_t num_bytes) {
+  void GetRandomBytes(void* output, size_t num_bytes) {
+    char* output_ptr = static_cast<char*>(output);
     while (num_bytes > 0) {
       size_t nread;
-      int error = random_.get_random_bytes(buffer, num_bytes, &nread);
+      const int error = random_.get_random_bytes(output_ptr, num_bytes, &nread);
       CHECK_EQ(error, 0);
       CHECK_LE(nread, num_bytes);
-      buffer += nread;
+      output_ptr += nread;
       num_bytes -= nread;
     }
   }
@@ -43,10 +43,14 @@ base::LazyInstance<NaclRandom>::Leaky g_nacl_random = LAZY_INSTANCE_INITIALIZER;
 
 namespace base {
 
+// NOTE: This function must be cryptographically secure. http://crbug.com/140076

[wtc, 2014/01/22 22:29:09]

This comment actually contradicts the comment for RandBytes in base/rand_util.h:

  // Fills |output_length| bytes of |output| with random data.
  //
  // WARNING:
  // Do not use for security-sensitive purposes.
  // See crypto/ for cryptographically secure random number generation APIs.
  BASE_EXPORT void RandBytes(void* output, size_t output_length);

Please move this comment to the front of RandUint64().

[DaleCurtis, 2014/01/22 22:36:26]

Are you okay with moving this comment into the rand_util.h file for
RandUint64(), RandBytes(), and RandBytesAsString()? Would you like it duplicated
on each method for safety's sake?

[wtc, 2014/01/22 22:47:12]

I think this comment doesn't make sense. The purpose of your CL is to improve
performance. Your CL should stay focused on that. So I suggest you not touch the
comment. Since this comment wasn't in this file before, I think you do not need
to add it.

It is a good idea to clean up the comments, but it's better to do that in a
separate CL.

[DaleCurtis, 2014/01/22 22:54:14]

Heh, fair enough. All changes to it reverted.

+void RandBytes(void* output, size_t output_length) {

[wtc, 2014/01/22 22:29:09]

RandBytes should be defined after RandUint64 to match the declaration order in
the .h file. This is recommended by the Style Guide.

[DaleCurtis, 2014/01/22 22:54:14]

Done.

+  g_nacl_random.Pointer()->GetRandomBytes(output, output_length);
+}
+
 uint64 RandUint64() {
   uint64 result;
-  g_nacl_random.Pointer()->GetRandomBytes(
-      reinterpret_cast<char*>(&result), sizeof(result));
+  g_nacl_random.Pointer()->GetRandomBytes(&result, sizeof(result));
   return result;
 }