crypto: add buffering to randomInt

Author: tniessen

benchmark/crypto/randomInt.js

@@ -0,0 +1,38 @@
+'use strict';
+
+const common = require('../common.js');
+const { randomInt } = require('crypto');
+
+const bench = common.createBenchmark(main, {
+  mode: ['sync', 'async-sequential', 'async-parallel'],
+  min: [-(2 ** 47) + 1, -10_000, -100],
+  max: [100, 10_000, 2 ** 47],
+  n: [1e3, 1e5]
+});
+
+function main({ mode, min, max, n }) {
+  if (mode === 'sync') {
+    bench.start();
+    for (let i = 0; i < n; i++)
+      randomInt(min, max);
+    bench.end(n);
+  } else if (mode === 'async-sequential') {
+    bench.start();
+    (function next(i) {
+      if (i === n)
+        return bench.end(n);
+      randomInt(min, max, () => {
+        next(i + 1);
+      });
+    })(0);
+  } else {
+    bench.start();
+    let done = 0;
+    for (let i = 0; i < n; i++) {
+      randomInt(min, max, () => {
+        if (++done === n)
+          bench.end(n);
+      });
+    }
+  }
+}

lib/internal/crypto/random.js

@@ -179,6 +179,13 @@ function randomFill(buf, offset, size, callback) {
 // e.g.: Buffer.from("ff".repeat(6), "hex").readUIntBE(0, 6);
 const RAND_MAX = 0xFFFF_FFFF_FFFF;
 
+// Cache random data to use in randomInt. The cache size must be evenly
+// divisible by 6 because each attempt to obtain a random int uses 6 bytes.
+const randomCache = new FastBuffer(6 * 1024);
+let randomCacheOffset = randomCache.length;
+let asyncCacheFillInProgress = false;
+const asyncCachePendingTasks = [];
+
 // Generates an integer in [min, max) range where min is inclusive and max is
 // exclusive.
 function randomInt(min, max, callback) {
@@ -223,33 +230,58 @@ function randomInt(min, max, callback) {
   // than or equal to 0 and less than randLimit.
   const randLimit = RAND_MAX - (RAND_MAX % range);
 
-  if (isSync) {
-    // Sync API
-    while (true) {
-      const x = randomBytes(6).readUIntBE(0, 6);
-      if (x >= randLimit) {
-        // Try again.
-        continue;
-      }
-      return (x % range) + min;
+  // If we don't have a callback, or if there is still data in the cache, we can
+  // do this synchronously, which is super fast.
+  while (isSync || (randomCacheOffset < randomCache.length)) {
+    if (randomCacheOffset === randomCache.length) {
+      // This might block the thread for a bit, but we are in sync mode.
+      randomFillSync(randomCache);
+      randomCacheOffset = 0;
+    }
+
+    const x = randomCache.readUIntBE(randomCacheOffset, 6);
+    randomCacheOffset += 6;
+
+    if (x < randLimit) {
+      const n = (x % range) + min;
+      if (isSync) return n;
+      process.nextTick(callback, undefined, n);
+      return;
     }
-  } else {
-    // Async API
-    const pickAttempt = () => {
-      randomBytes(6, (err, bytes) => {
-        if (err) return callback(err);
-        const x = bytes.readUIntBE(0, 6);
-        if (x >= randLimit) {
-          // Try again.
-          return pickAttempt();
-        }
-        const n = (x % range) + min;
-        callback(null, n);
-      });
-    };
-
-    pickAttempt();
   }
+
+  // At this point, we are in async mode with no data in the cache. We cannot
+  // simply refill the cache, because another async call to randomInt might
+  // already be doing that. Instead, queue this call for when the cache has
+  // been refilled.
+  asyncCachePendingTasks.push({ min, max, callback });
+  asyncRefillRandomIntCache();
+}
+
+function asyncRefillRandomIntCache() {
+  if (asyncCacheFillInProgress)
+    return;
+
+  asyncCacheFillInProgress = true;
+  randomFill(randomCache, (err) => {
+    asyncCacheFillInProgress = false;
+
+    const tasks = asyncCachePendingTasks;
+    const errorReceiver = err && tasks.shift();
+    if (!err)
+      randomCacheOffset = 0;
+
+    // Restart all pending tasks. If an error occurred, we only notify a single
+    // callback (errorReceiver) about it. This way, every async call to
+    // randomInt has a chance of being successful, and it avoids complex
+    // exception handling here.
+    for (const { min, max, callback } of tasks.splice(0, tasks.length))
+      randomInt(min, max, callback);
+
+    // This is the only call that might throw, and is therefore done at the end.
+    if (errorReceiver)
+      errorReceiver.callback(err);
+  });
 }