crypto: improve random synchronous number generation performance by mertcanaltin · Pull Request #66348 · nodejs/node

mertcanaltin · 2026-09-27T12:14:39Z

Improves random synchronous number generation.

Instead of creating the RandomBytesJob object, it now calls the CSPRNG directly.

.venv) ➜  node git:(crypto-randomfillsync-fast-path) ✗ npx node-benchmark-compare rb3.csv
                                          confidence improvement accuracy (*)   (**)  (***)
crypto/randomBytes.js n=100000 size=1024         ***     68.32 %       ±1.65% ±2.21% ±2.88%
crypto/randomBytes.js n=100000 size=16384         **     -2.70 %       ±1.69% ±2.25% ±2.93%
crypto/randomBytes.js n=100000 size=64           ***     95.33 %       ±4.88% ±6.56% ±8.67%
crypto/randomBytes.js n=100000 size=8192         ***     21.15 %       ±1.45% ±1.93% ±2.52%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 4 comparisons, you can thus expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

The slowdown at 16 KB is caused by the garbage collector, not by random number generation.

Assisted by: Claude Code Opus 5.5

nodejs-github-bot · 2026-09-27T12:14:45Z

Review requested:

@nodejs/crypto

github-actions · 2026-09-27T13:15:28Z

Benchmark GHA (crypto / randomBytes): https://github.com/nodejs/node/actions/runs/36321760949

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                          confidence improvement accuracy (*)    (**)   (***)
crypto/randomBytes.js n=100000 size=1024         ***     94.12 %      ±14.55% ±19.20% ±24.68%
crypto/randomBytes.js n=100000 size=16384        ***     33.41 %      ±10.25% ±13.51% ±17.35%
crypto/randomBytes.js n=100000 size=64           ***    133.51 %      ±15.59% ±20.59% ±26.49%
crypto/randomBytes.js n=100000 size=8192         ***     55.43 %      ±13.73% ±18.12% ±23.27%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

panva

Please also declare the new binding in typings/internalBinding/crypto.d.ts

Can you add a regression test for --trace-sync-io with randomFillSync() after the first event-loop turn? The old job emits a warning there while the new path is silent.

The error guard is needed on success too: CSPRNG() can retry after a RAND failure and succeed while leaving the earlier error queued. A transient-failure probe clears the queue with the old job but leaves it populated with this path.

github-actions · 2026-09-27T13:54:35Z

Benchmark GHA (crypto / randomBytes): https://github.com/nodejs/node/actions/runs/36324017648

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                          confidence improvement accuracy (*)    (**)   (***)
crypto/randomBytes.js n=100000 size=1024         ***     71.00 %      ±16.97% ±22.39% ±28.78%
crypto/randomBytes.js n=100000 size=16384        ***     25.38 %      ±12.69% ±16.74% ±21.48%
crypto/randomBytes.js n=100000 size=64           ***     93.42 %      ±17.28% ±22.81% ±29.33%
crypto/randomBytes.js n=100000 size=8192         ***     43.81 %      ±16.01% ±21.12% ±27.12%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

mertcanaltin · 2026-09-27T13:56:31Z

Thanks, I applied your suggestions, I added new bindings in typings & regression test for --trace-sync-io

mertcanaltin · 2026-09-27T14:16:37Z

@panva ClearErrorOnReturn calls ERR_clear_error() twice, which costs about half of the gain.

randomBytes(16): main 670ns, current 426ns, alternative 301ns.

Alternative: on success, only clear the queue if it has an error.

if (ncrypto::CSPRNG(...)) {
  if (ERR_peek_error() != 0) ERR_clear_error();
     return;
 }

On failure, capture() already clears the queue, is this acceptable?

panva · 2026-09-27T14:21:47Z

The conditional clear on success looks fine, and Capture() drains the queue on failure. Can we also keep a conditional clear before CSPRNG()? It uses ERR_peek_last_error() to decide whether to retry, so a pre-existing error could affect that decision or end up in the reported exception.

if (ERR_peek_error() != 0) ERR_clear_error();
if (ncrypto::CSPRNG(in.data() + byte_offset, size)) {
  if (ERR_peek_error() != 0) ERR_clear_error();
  return;
}

codecov · 2026-09-27T15:25:43Z

Codecov Report

❌ Patch coverage is 60.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.34%. Comparing base (3641c36) to head (fb978fe).
⚠️ Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
src/crypto/crypto_random.cc	57.14%	3 Missing and 9 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66348      +/-   ##
==========================================
- Coverage   90.36%   90.34%   -0.02%     
==========================================
  Files         792      792              
  Lines      275386   275406      +20     
  Branches    52770    52785      +15     
==========================================
- Hits       248859   248827      -32     
- Misses      16936    16992      +56     
+ Partials     9591     9587       -4

Files with missing lines	Coverage Δ
lib/internal/crypto/random.js	`96.41% <100.00%> (-0.05%)`	⬇️
src/crypto/crypto_random.cc	`71.01% <57.14%> (-3.54%)`	⬇️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Instead of creating a RandomBytesJob object, it calls CSPRNG() directly now. Assisted-by: Claude Code Signed-off-by: Mert Can Altin <mertgold60@gmail.com>

github-actions · 2026-09-27T16:07:05Z

Benchmark GHA (crypto / randomBytes): https://github.com/nodejs/node/actions/runs/36332027908

Results

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

Benchmark results:

                                          confidence improvement accuracy (*)    (**)   (***)
crypto/randomBytes.js n=100000 size=1024         ***     93.72 %      ±15.24% ±20.12% ±25.87%
crypto/randomBytes.js n=100000 size=16384        ***     30.52 %       ±9.91% ±13.07% ±16.77%
crypto/randomBytes.js n=100000 size=64           ***    120.99 %      ±16.95% ±22.38% ±28.80%
crypto/randomBytes.js n=100000 size=8192         ***     55.18 %      ±13.48% ±17.78% ±22.84%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case, there are 4 comparisons, you can thus
expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

[!WARNING]
Do not take GHA benchmark results as face value, always confirm them
using a dedicated machine, e.g. Jenkins CI.

mertcanaltin · 2026-09-27T16:13:17Z

Thanks, I applied and new results!

Conditional cleanup recovered most of the profits.

node git:(crypto-randomfillsync-fast-path) ✗ npx node-benchmark-compare rb3.csv
                                          confidence improvement accuracy (*)   (**)  (***)
crypto/randomBytes.js n=100000 size=1024         ***     68.32 %       ±1.65% ±2.21% ±2.88%
crypto/randomBytes.js n=100000 size=16384         **     -2.70 %       ±1.69% ±2.25% ±2.93%
crypto/randomBytes.js n=100000 size=64           ***     95.33 %       ±4.88% ±6.56% ±8.67%
crypto/randomBytes.js n=100000 size=8192         ***     21.15 %       ±1.45% ±1.93% ±2.52%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 4 comparisons, you can thus expect the following amount of false-positive results:
  0.20 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.04 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

jasnell · 2026-09-28T04:16:12Z

+  CHECK(args[1]->IsUint32());         // Offset
+  CHECK(args[2]->IsUint32());         // Size
+
+  ArrayBufferOrViewContents<unsigned char> in(args[0]);


Since we're in here... we need to start updating these to check IsImmutable() on the ArrayBuffer. The ArrayBufferOrViewContents<unsigned char> should probably throw if IsImmutable() is true and we should have an ArrayBufferOrViewContents<const unsigned char> that works with immutable ones.

i'll take care of that once #66350 lands and we'll be able to start chipping away at these issues with known issue coverage.

Can I send this in this PR? Or separate?

Don't, #66350 has passing CI and I have this particular issue covered already just pending #66350 landing.

Ok, Thanks.

nodejs-github-bot · 2026-09-28T17:31:07Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78057/

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Sep 27, 2026

mertcanaltin force-pushed the crypto-randomfillsync-fast-path branch 3 times, most recently from a065d1f to 6e53cc3 Compare September 27, 2026 12:50

panva reviewed Sep 27, 2026

View reviewed changes

Comment thread src/crypto/crypto_random.cc Outdated

mertcanaltin force-pushed the crypto-randomfillsync-fast-path branch from 0fc0085 to bc5ab7a Compare September 27, 2026 13:42

crypto: improve random synchronous number generation performance

fb978fe

Instead of creating a RandomBytesJob object, it calls CSPRNG() directly now. Assisted-by: Claude Code Signed-off-by: Mert Can Altin <mertgold60@gmail.com>

mertcanaltin force-pushed the crypto-randomfillsync-fast-path branch from bc5ab7a to fb978fe Compare September 27, 2026 16:05

jasnell reviewed Sep 28, 2026

View reviewed changes

panva approved these changes Sep 28, 2026

View reviewed changes

panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 28, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 28, 2026

This comment was marked as outdated.

Sign in to view

Uh oh!

Conversation

mertcanaltin commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 27, 2026

Uh oh!

github-actions Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

panva left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mertcanaltin commented Sep 27, 2026

Uh oh!

mertcanaltin commented Sep 27, 2026

Uh oh!

panva commented Sep 27, 2026

Uh oh!

codecov Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mertcanaltin commented Sep 27, 2026

Uh oh!

jasnell Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

panva Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

panva Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Sep 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mertcanaltin commented Sep 27, 2026 •

edited

Loading

github-actions Bot commented Sep 27, 2026 •

edited

Loading

github-actions Bot commented Sep 27, 2026 •

edited

Loading

codecov Bot commented Sep 27, 2026 •

edited

Loading

github-actions Bot commented Sep 27, 2026 •

edited

Loading

panva Sep 28, 2026 •

edited

Loading

panva Sep 28, 2026 •

edited

Loading

mertcanaltin Sep 28, 2026 •

edited

Loading