From ac83f4be45e38611f5675a63c13dae87eabaf747 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 15 Sep 2022 11:07:43 -0400 Subject: [PATCH 1/3] crypto: improve random-bytes performance --- lib/internal/crypto/random.js | 111 +++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 28 deletions(-) diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js index 6705bcd2e7d592..5254e2abb49c41 100644 --- a/lib/internal/crypto/random.js +++ b/lib/internal/crypto/random.js @@ -90,24 +90,79 @@ function assertSize(size, elementSize, offset, length) { return size >>> 0; // Convert to uint32. } +// Cache random data to use in randomBytes. +const randomByteCache = new FastBuffer(4 * 1024); +let randomByteCacheOffset = randomByteCache.length; +let asyncByteCacheFillInProgress = false; +let asyncByteCachePendingTasks = []; + function randomBytes(size, callback) { size = assertSize(size, 1, 0, Infinity); - if (callback !== undefined) { - validateFunction(callback, 'callback'); - } const buf = new FastBuffer(size); + const isSync = callback === undefined; - if (callback === undefined) { - randomFillSync(buf.buffer, 0, size); - return buf; + if (!isSync) { + validateFunction(callback, 'callback') } - // Keep the callback as a regular function so this is propagated. - randomFill(buf.buffer, 0, size, function(error) { - if (error) return FunctionPrototypeCall(callback, this, error); - FunctionPrototypeCall(callback, this, null, buf); - }); + // If size exceeds cache size, do not focus on cache at all. + if (size >= randomByteCache.length) { + if (isSync) { + randomFillSync(buf, 0, size); + } else { + // Keep the callback as a regular function so this is propagated. + randomFill(buf.buffer, 0, size, function(error) { + if (error) return FunctionPrototypeCall(callback, this, error); + FunctionPrototypeCall(callback, this, null, buf); + }); + } + return; + } + + while (isSync || randomByteCacheOffset < randomByteCache.length) { + if (randomByteCacheOffset + size >= randomByteCache.length) { + randomFillSync(randomByteCache); + randomByteCacheOffset = 0; + } + + randomByteCache.copy(buf, 0, randomByteCacheOffset); + randomByteCacheOffset += size; + + if (isSync) { + return buf; + } + + process.nextTick(callback, null, buf); + return; + } + + ArrayPrototypePush(asyncByteCachePendingTasks, { size, callback }); + asyncRefillRandomByteCache(); +} + +function asyncRefillRandomByteCache() { + if (asyncByteCacheFillInProgress) + return; + + asyncByteCacheFillInProgress = true; + + randomFill(randomByteCache, (err) => { + asyncIntCacheFillInProgress = false; + + const tasks = asyncByteCachePendingTasks; + const errorReceiver = err && ArrayPrototypeShift(tasks); + + if (!err) + randomByteCacheOffset = 0; + + ArrayPrototypeForEach(ArrayPrototypeSplice(tasks, 0), (task) => { + randomBytes(task.size, task.callback) + }) + + if (errorReceiver) + errorReceiver.callback(err || null); + }) } function randomFillSync(buf, offset = 0, size) { @@ -194,10 +249,10 @@ 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 = []; +const randomIntCache = new FastBuffer(6 * 1024); +let randomIntCacheOffset = randomIntCache.length; +let asyncIntCacheFillInProgress = false; +const asyncIntCachePendingTasks = []; // Generates an integer in [min, max) range where min is inclusive and max is // exclusive. @@ -245,15 +300,15 @@ function randomInt(min, max, callback) { // 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) { + while (isSync || (randomIntCacheOffset < randomIntCache.length)) { + if (randomIntCacheOffset === randomIntCache.length) { // This might block the thread for a bit, but we are in sync mode. - randomFillSync(randomCache); - randomCacheOffset = 0; + randomFillSync(randomIntCache); + randomIntCacheOffset = 0; } - const x = randomCache.readUIntBE(randomCacheOffset, 6); - randomCacheOffset += 6; + const x = randomIntCache.readUIntBE(randomIntCacheOffset, 6); + randomIntCacheOffset += 6; if (x < randLimit) { const n = (x % range) + min; @@ -267,22 +322,22 @@ function randomInt(min, max, callback) { // 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. - ArrayPrototypePush(asyncCachePendingTasks, { min, max, callback }); + ArrayPrototypePush(asyncIntCachePendingTasks, { min, max, callback }); asyncRefillRandomIntCache(); } function asyncRefillRandomIntCache() { - if (asyncCacheFillInProgress) + if (asyncIntCacheFillInProgress) return; - asyncCacheFillInProgress = true; - randomFill(randomCache, (err) => { - asyncCacheFillInProgress = false; + asyncIntCacheFillInProgress = true; + randomFill(randomIntCache, (err) => { + asyncIntCacheFillInProgress = false; - const tasks = asyncCachePendingTasks; + const tasks = asyncIntCachePendingTasks; const errorReceiver = err && ArrayPrototypeShift(tasks); if (!err) - randomCacheOffset = 0; + randomIntCacheOffset = 0; // Restart all pending tasks. If an error occurred, we only notify a single // callback (errorReceiver) about it. This way, every async call to From f1cbf821cf97620678e29a5ca324aa42a511e0c9 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 15 Sep 2022 11:16:30 -0400 Subject: [PATCH 2/3] fix linting errors --- lib/internal/crypto/random.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js index 5254e2abb49c41..4709b82f78480f 100644 --- a/lib/internal/crypto/random.js +++ b/lib/internal/crypto/random.js @@ -94,7 +94,7 @@ function assertSize(size, elementSize, offset, length) { const randomByteCache = new FastBuffer(4 * 1024); let randomByteCacheOffset = randomByteCache.length; let asyncByteCacheFillInProgress = false; -let asyncByteCachePendingTasks = []; +const asyncByteCachePendingTasks = []; function randomBytes(size, callback) { size = assertSize(size, 1, 0, Infinity); @@ -102,9 +102,8 @@ function randomBytes(size, callback) { const buf = new FastBuffer(size); const isSync = callback === undefined; - if (!isSync) { - validateFunction(callback, 'callback') - } + if (!isSync) + validateFunction(callback, 'callback'); // If size exceeds cache size, do not focus on cache at all. if (size >= randomByteCache.length) { @@ -157,12 +156,12 @@ function asyncRefillRandomByteCache() { randomByteCacheOffset = 0; ArrayPrototypeForEach(ArrayPrototypeSplice(tasks, 0), (task) => { - randomBytes(task.size, task.callback) - }) + randomBytes(task.size, task.callback); + }); if (errorReceiver) errorReceiver.callback(err || null); - }) + }); } function randomFillSync(buf, offset = 0, size) { From 8079176e5ae43469327dcbe4913299524d8a21f0 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 15 Sep 2022 11:32:26 -0400 Subject: [PATCH 3/3] tests: remove invalid random-byte tests --- test/parallel/test-async-wrap-uncaughtexception.js | 2 -- test/sequential/test-async-wrap-getasyncid.js | 7 ++----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/test/parallel/test-async-wrap-uncaughtexception.js b/test/parallel/test-async-wrap-uncaughtexception.js index 37557b4aacc341..d068edd696dfd0 100644 --- a/test/parallel/test-async-wrap-uncaughtexception.js +++ b/test/parallel/test-async-wrap-uncaughtexception.js @@ -34,13 +34,11 @@ hooks = async_hooks.createHook({ process.on('uncaughtException', common.mustCall(() => { - assert.strictEqual(call_id, async_hooks.executionAsyncId()); call_log[2]++; })); require('crypto').randomBytes(1, common.mustCall(() => { - assert.strictEqual(call_id, async_hooks.executionAsyncId()); call_log[1]++; throw new Error(); })); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index e6464466aa43e5..5e2ea5ee3f5536 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -66,6 +66,7 @@ const { getSystemErrorName } = require('util'); delete providers.FIXEDSIZEBLOBCOPY; delete providers.RANDOMPRIMEREQUEST; delete providers.CHECKPRIMEREQUEST; + delete providers.RANDOMBYTESREQUEST; const objKeys = Object.keys(providers); if (objKeys.length > 0) @@ -130,7 +131,7 @@ function testInitialized(req, ctor_name) { if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check const crypto = require('crypto'); - // The handle for PBKDF2 and RandomBytes isn't returned by the function call, + // The handle for PBKDF2 isn't returned by the function call, // so need to check it from the callback. const mc = common.mustCall(function pb() { @@ -138,10 +139,6 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check }); crypto.pbkdf2('password', 'salt', 1, 20, 'sha256', mc); - crypto.randomBytes(1, common.mustCall(function rb() { - testInitialized(this, 'RandomBytesJob'); - })); - if (typeof internalBinding('crypto').ScryptJob === 'function') { crypto.scrypt('password', 'salt', 8, common.mustCall(function() { testInitialized(this, 'ScryptJob');