crypto: add buffering to randomInt #35110

tniessen · 2020-09-08T22:43:39Z

This adds a buffer to randomInt to prevent calling randomBytes on each invocation. It also simplifies the actual calculation to always be synchronous, and doesn't duplicate the logic for asynchronous behavior. However, this change does include additional code to manage the cache.

Performance is improved significantly, by large factors, for synchronous calls and sequential asynchronous calls. For parallel calls, performance is about the same when requesting a few hundred random integers at a time.

Performance is significantly worse when requesting a large number of random integers asynchronously and in parallel. The reason is that this implementation delays all requests until the buffer has been refilled with random data, then serves as many requests as it can, then delays the remaining requests until the buffer has been refilled again, and so on. With a buffer size of 3KiB, 500 random integers can be provided without having to call randomBytes. When the benchmark requests 100,000 random integers asynchronously and in parallel, instead of processing 100,000 calls to randomBytes in parallel, the buffered implementation provides 200 batches of 500 numbers each sequentially.

We could add additional code to handle such cases and avoid this performance penalty, but I decided to leave the additional complexity out for now. I assume that most use cases will require a sequence of random numbers, and not necessarily a large amount of random numbers at once.
We could also change the buffer size, but that would require more memory, and could lead to slightly longer delays when refilling the buffer.
We could also disable buffering for async in general, and only use it for sync calls. That would make any single call to randomInt with a callback slow, but would make generating huge amounts of random integers at the same time faster.

Benchmark results based on 30 runs (3988798 compared to the master branch at b5a47ca):

                                                                                               confidence improvement accuracy (*)     (**)    (***)
 crypto/randomInt.js n=1000 max=100 min=-100 mode='async-parallel'                                             7.61 %       ±8.19%  ±10.90%  ±14.19%
 crypto/randomInt.js n=1000 max=100 min=-100 mode='async-sequential'                                  ***    113.40 %       ±9.73%  ±12.98%  ±16.98%
 crypto/randomInt.js n=1000 max=100 min=-100 mode='sync'                                              ***    151.27 %       ±4.81%   ±6.41%   ±8.36%
 crypto/randomInt.js n=1000 max=100 min=-10000 mode='async-parallel'                                           1.72 %       ±8.22%  ±10.97%  ±14.35%
 crypto/randomInt.js n=1000 max=100 min=-10000 mode='async-sequential'                                ***    110.33 %       ±9.82%  ±13.12%  ±17.17%
 crypto/randomInt.js n=1000 max=100 min=-10000 mode='sync'                                            ***    138.80 %       ±7.85%  ±10.54%  ±13.92%
 crypto/randomInt.js n=1000 max=100 min=-140737488355327 mode='async-parallel'                        ***     39.70 %       ±9.72%  ±12.99%  ±17.03%
 crypto/randomInt.js n=1000 max=100 min=-140737488355327 mode='async-sequential'                      ***    277.28 %      ±12.17%  ±16.35%  ±21.61%
 crypto/randomInt.js n=1000 max=100 min=-140737488355327 mode='sync'                                  ***    264.60 %       ±9.85%  ±13.13%  ±17.16%
 crypto/randomInt.js n=1000 max=10000 min=-100 mode='async-parallel'                                    *      6.06 %       ±5.92%   ±7.88%  ±10.26%
 crypto/randomInt.js n=1000 max=10000 min=-100 mode='async-sequential'                                ***     97.52 %      ±10.61%  ±14.20%  ±18.65%
 crypto/randomInt.js n=1000 max=10000 min=-100 mode='sync'                                            ***    138.26 %       ±8.36%  ±11.26%  ±14.94%
 crypto/randomInt.js n=1000 max=10000 min=-10000 mode='async-parallel'                                        -4.46 %       ±8.33%  ±11.16%  ±14.69%
 crypto/randomInt.js n=1000 max=10000 min=-10000 mode='async-sequential'                              ***    103.98 %      ±10.82%  ±14.46%  ±18.96%
 crypto/randomInt.js n=1000 max=10000 min=-10000 mode='sync'                                          ***    141.13 %       ±3.42%   ±4.56%   ±5.95%
 crypto/randomInt.js n=1000 max=10000 min=-140737488355327 mode='async-parallel'                      ***     40.64 %       ±4.59%   ±6.11%   ±7.95%
 crypto/randomInt.js n=1000 max=10000 min=-140737488355327 mode='async-sequential'                    ***    263.09 %      ±23.93%  ±32.22%  ±42.72%
 crypto/randomInt.js n=1000 max=10000 min=-140737488355327 mode='sync'                                ***    246.81 %       ±4.33%   ±5.83%   ±7.72%
 crypto/randomInt.js n=1000 max=140737488355328 min=-100 mode='async-parallel'                        ***     37.40 %       ±7.18%   ±9.59%  ±12.55%
 crypto/randomInt.js n=1000 max=140737488355328 min=-100 mode='async-sequential'                      ***    275.54 %      ±11.41%  ±15.31%  ±20.20%
 crypto/randomInt.js n=1000 max=140737488355328 min=-100 mode='sync'                                  ***    247.66 %       ±3.52%   ±4.71%   ±6.17%
 crypto/randomInt.js n=1000 max=140737488355328 min=-10000 mode='async-parallel'                      ***     36.03 %       ±7.56%  ±10.11%  ±13.27%
 crypto/randomInt.js n=1000 max=140737488355328 min=-10000 mode='async-sequential'                    ***    254.88 %      ±18.48%  ±24.85%  ±32.86%
 crypto/randomInt.js n=1000 max=140737488355328 min=-10000 mode='sync'                                ***    251.64 %       ±3.86%   ±5.16%   ±6.76%
 crypto/randomInt.js n=1000 max=140737488355328 min=-140737488355327 mode='async-parallel'                    -0.22 %       ±7.35%   ±9.80%  ±12.78%
 crypto/randomInt.js n=1000 max=140737488355328 min=-140737488355327 mode='async-sequential'          ***    104.97 %      ±12.95%  ±17.36%  ±22.87%
 crypto/randomInt.js n=1000 max=140737488355328 min=-140737488355327 mode='sync'                      ***    138.63 %       ±4.24%   ±5.71%   ±7.56%
 crypto/randomInt.js n=100000 max=100 min=-100 mode='async-parallel'                                  ***    -41.12 %       ±1.21%   ±1.62%   ±2.13%
 crypto/randomInt.js n=100000 max=100 min=-100 mode='async-sequential'                                ***    691.82 %       ±7.30%   ±9.81%  ±12.97%
 crypto/randomInt.js n=100000 max=100 min=-100 mode='sync'                                            ***   1310.10 %      ±36.17%  ±48.74%  ±64.70%
 crypto/randomInt.js n=100000 max=100 min=-10000 mode='async-parallel'                                ***    -40.20 %       ±1.28%   ±1.72%   ±2.26%
 crypto/randomInt.js n=100000 max=100 min=-10000 mode='async-sequential'                              ***    691.30 %       ±8.86%  ±11.91%  ±15.77%
 crypto/randomInt.js n=100000 max=100 min=-10000 mode='sync'                                          ***   1290.11 %      ±55.81%  ±75.21%  ±99.85%
 crypto/randomInt.js n=100000 max=100 min=-140737488355327 mode='async-parallel'                      ***    -38.02 %       ±0.83%   ±1.11%   ±1.47%
 crypto/randomInt.js n=100000 max=100 min=-140737488355327 mode='async-sequential'                    ***   1434.15 %      ±22.58%  ±30.43%  ±40.39%
 crypto/randomInt.js n=100000 max=100 min=-140737488355327 mode='sync'                                ***   2233.63 %      ±61.72%  ±83.18% ±110.43%
 crypto/randomInt.js n=100000 max=10000 min=-100 mode='async-parallel'                                ***    -40.04 %       ±1.48%   ±1.99%   ±2.64%
 crypto/randomInt.js n=100000 max=10000 min=-100 mode='async-sequential'                              ***    694.65 %       ±7.53%  ±10.12%  ±13.38%
 crypto/randomInt.js n=100000 max=10000 min=-100 mode='sync'                                          ***   1338.29 %      ±31.28%  ±42.15%  ±55.95%
 crypto/randomInt.js n=100000 max=10000 min=-10000 mode='async-parallel'                              ***    -39.62 %       ±1.15%   ±1.54%   ±2.04%
 crypto/randomInt.js n=100000 max=10000 min=-10000 mode='async-sequential'                            ***    685.36 %       ±8.82%  ±11.87%  ±15.71%
 crypto/randomInt.js n=100000 max=10000 min=-10000 mode='sync'                                        ***   1286.39 %      ±43.29%  ±58.33%  ±77.43%
 crypto/randomInt.js n=100000 max=10000 min=-140737488355327 mode='async-parallel'                    ***    -38.56 %       ±1.02%   ±1.37%   ±1.80%
 crypto/randomInt.js n=100000 max=10000 min=-140737488355327 mode='async-sequential'                  ***   1450.61 %      ±18.13%  ±24.43%  ±32.43%
 crypto/randomInt.js n=100000 max=10000 min=-140737488355327 mode='sync'                              ***   2137.98 %      ±87.00% ±117.25% ±155.67%
 crypto/randomInt.js n=100000 max=140737488355328 min=-100 mode='async-parallel'                      ***    -39.14 %       ±0.82%   ±1.09%   ±1.43%
 crypto/randomInt.js n=100000 max=140737488355328 min=-100 mode='async-sequential'                    ***   1453.44 %      ±16.70%  ±22.50%  ±29.87%
 crypto/randomInt.js n=100000 max=140737488355328 min=-100 mode='sync'                                ***   2161.60 %      ±72.54%  ±97.76% ±129.79%
 crypto/randomInt.js n=100000 max=140737488355328 min=-10000 mode='async-parallel'                    ***    -38.39 %       ±1.10%   ±1.47%   ±1.93%
 crypto/randomInt.js n=100000 max=140737488355328 min=-10000 mode='async-sequential'                  ***   1449.92 %      ±13.92%  ±18.74%  ±24.86%
 crypto/randomInt.js n=100000 max=140737488355328 min=-10000 mode='sync'                              ***   2169.74 %      ±67.67%  ±91.20% ±121.08%
 crypto/randomInt.js n=100000 max=140737488355328 min=-140737488355327 mode='async-parallel'          ***    -51.28 %       ±1.46%   ±1.96%   ±2.57%
 crypto/randomInt.js n=100000 max=140737488355328 min=-140737488355327 mode='async-sequential'        ***    695.29 %       ±8.29%  ±11.15%  ±14.75%
 crypto/randomInt.js n=100000 max=140737488355328 min=-140737488355327 mode='sync'                    ***   1271.56 %      ±36.90%  ±49.73%  ±66.01%

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

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

nodejs-github-bot · 2020-09-08T22:43:43Z

Review requested:

@nodejs/crypto

bnoordhuis · 2020-09-10T14:49:32Z

I'm decidedly 'meh' on this. No one's been complaining about its speed so far.

tniessen · 2020-09-14T12:38:49Z

@bnoordhuis How about adding this for the sync case only? That's a relatively small change with a big payoff, and I think the performance of the sync case is more critical anyway. The downside is that this approach results in the existing code duplication between sync and async behavior, which essentially implements the entire thing twice.

aduh95 · 2020-11-08T11:35:14Z

@nodejs/crypto can you review this please? Those benchmark results look very good!

lib/internal/crypto/random.js

mcollina · 2021-01-03T15:20:54Z

lib/internal/crypto/random.js

+const randomCache = new FastBuffer(3072);
+let randomCacheOffset = randomCache.length;
+let asyncCacheFillInProgress = false;
+const asyncCachePendingTasks = [];


using an array for this is usually not performing as expected for high numbers. Using a linked list solves this problem.

we have fixed-queue which also might be appropriate?

jasnell

It might make sense to introduce a random buffer internal utility that can be used in multiple places (here, the randomuuid fn, etc). That can be done later tho

lib/internal/crypto/random.js

tniessen · 2021-01-13T15:54:31Z

@mcollina I doubled the cache size, but a more efficient way to improve performance might be to reduce the number of bytes that is used for each random integer. In most cases, we don't actually need 6 bytes, probably ony 2 to 4 bytes. (It depends on the use case, of course.)

puzpuzpuz · 2021-01-18T06:36:29Z

@tniessen could you resolve conflicts on this one?

tniessen · 2021-01-18T14:35:19Z

Benchmark results still look good:

Table

n	max	min	mode	confidence	improvement	accuracy (*)	accuracy (**)	accuracy (***)
100000	10000	-10000	'async-parallel'	***	41.05 %	±4.60%	±6.17%	±8.12%
100000	10000	-10000	'async-sequential'	***	1931.62 %	±106.63%	±143.69%	±190.75%
100000	10000	-10000	'sync'	***	1976.14 %	±114.57%	±154.41%	±204.98%
100000	10000	-100	'async-parallel'	***	43.43 %	±5.20%	±6.95%	±9.10%
100000	10000	-100	'async-sequential'	***	1888.73 %	±99.87%	±134.60%	±178.69%
100000	10000	-100	'sync'	***	1895.70 %	±99.80%	±134.49%	±178.54%
100000	10000	-140737488355327	'async-parallel'	***	44.39 %	±4.65%	±6.23%	±8.22%
100000	10000	-140737488355327	'async-sequential'	***	3455.63 %	±230.82%	±311.08%	±412.99%
100000	10000	-140737488355327	'sync'	***	3056.68 %	±158.66%	±213.82%	±283.87%
100000	100	-10000	'async-parallel'	***	39.66 %	±5.26%	±7.05%	±9.27%
100000	100	-10000	'async-sequential'	***	1936.95 %	±97.77%	±131.76%	±174.89%
100000	100	-10000	'sync'	***	1885.94 %	±75.29%	±101.47%	±134.70%
100000	100	-100	'async-parallel'	***	39.74 %	±5.70%	±7.64%	±10.06%
100000	100	-100	'async-sequential'	***	1878.43 %	±88.73%	±119.57%	±158.72%
100000	100	-100	'sync'	***	1892.60 %	±96.45%	±129.98%	±172.55%
100000	100	-140737488355327	'async-parallel'	***	40.54 %	±4.49%	±6.03%	±7.96%
100000	100	-140737488355327	'async-sequential'	***	3335.10 %	±234.14%	±315.55%	±418.91%
100000	100	-140737488355327	'sync'	***	3140.61 %	±170.90%	±230.32%	±305.77%
100000	140737488355328	-10000	'async-parallel'	***	47.40 %	±4.92%	±6.60%	±8.71%
100000	140737488355328	-10000	'async-sequential'	***	3300.80 %	±182.10%	±245.41%	±325.80%
100000	140737488355328	-10000	'sync'	***	2980.74 %	±144.38%	±194.58%	±258.31%
100000	140737488355328	-100	'async-parallel'	***	48.80 %	±5.10%	±6.86%	±9.06%
100000	140737488355328	-100	'async-sequential'	***	3385.05 %	±175.80%	±236.92%	±314.53%
100000	140737488355328	-100	'sync'	***	3041.96 %	±176.29%	±237.58%	±315.41%
100000	140737488355328	-140737488355327	'async-parallel'	***	21.46 %	±4.38%	±5.86%	±7.71%
100000	140737488355328	-140737488355327	'async-sequential'	***	2011.00 %	±146.63%	±197.62%	±262.35%
100000	140737488355328	-140737488355327	'sync'	***	1863.61 %	±92.30%	±124.39%	±165.14%
1000	10000	-10000	'async-parallel'	***	52.00 %	±11.51%	±15.44%	±20.35%
1000	10000	-10000	'async-sequential'	***	159.33 %	±10.13%	±13.58%	±17.88%
1000	10000	-10000	'sync'	***	545.42 %	±38.66%	±52.05%	±69.01%
1000	10000	-100	'async-parallel'	***	38.86 %	±6.89%	±9.18%	±11.99%
1000	10000	-100	'async-sequential'	***	156.91 %	±13.86%	±18.61%	±24.56%
1000	10000	-100	'sync'	***	605.82 %	±54.76%	±73.78%	±97.89%
1000	10000	-140737488355327	'async-parallel'	***	78.15 %	±5.72%	±7.61%	±9.90%
1000	10000	-140737488355327	'async-sequential'	***	331.03 %	±20.68%	±27.83%	±36.86%
1000	10000	-140737488355327	'sync'	***	516.57 %	±42.54%	±57.30%	±76.01%
1000	100	-10000	'async-parallel'	***	36.48 %	±8.30%	±11.06%	±14.44%
1000	100	-10000	'async-sequential'	***	152.74 %	±10.06%	±13.46%	±17.65%
1000	100	-10000	'sync'	***	566.73 %	±35.87%	±48.30%	±64.04%
1000	100	-100	'async-parallel'	***	31.18 %	±6.98%	±9.30%	±12.11%
1000	100	-100	'async-sequential'	***	161.54 %	±11.41%	±15.28%	±20.10%
1000	100	-100	'sync'	***	543.65 %	±36.10%	±48.61%	±64.46%
1000	100	-140737488355327	'async-parallel'	***	97.30 %	±10.66%	±14.31%	±18.89%
1000	100	-140737488355327	'async-sequential'	***	353.64 %	±29.03%	±39.09%	±51.84%
1000	100	-140737488355327	'sync'	***	494.78 %	±20.94%	±28.15%	±37.24%
1000	140737488355328	-10000	'async-parallel'	***	82.55 %	±10.74%	±14.38%	±18.90%
1000	140737488355328	-10000	'async-sequential'	***	325.50 %	±17.69%	±23.80%	±31.51%
1000	140737488355328	-10000	'sync'	***	525.51 %	±45.32%	±61.05%	±80.97%
1000	140737488355328	-100	'async-parallel'	***	80.98 %	±11.91%	±15.94%	±20.93%
1000	140737488355328	-100	'async-sequential'	***	325.43 %	±15.49%	±20.82%	±27.53%
1000	140737488355328	-100	'sync'	***	483.61 %	±36.11%	±48.61%	±64.44%
1000	140737488355328	-140737488355327	'async-parallel'	***	47.12 %	±7.40%	±9.88%	±12.92%
1000	140737488355328	-140737488355327	'async-sequential'	***	164.94 %	±12.99%	±17.42%	±22.93%
1000	140737488355328	-140737488355327	'sync'	***	601.63 %	±45.89%	±61.83%	±82.04%

Unformatted output

                                                                                               confidence improvement accuracy (*)     (**)    (***)
 crypto/randomInt.js n=100000 max=10000 min=-10000 mode='async-parallel'                              ***     41.05 %       ±4.60%   ±6.17%   ±8.12%
 crypto/randomInt.js n=100000 max=10000 min=-10000 mode='async-sequential'                            ***   1931.62 %     ±106.63% ±143.69% ±190.75%
 crypto/randomInt.js n=100000 max=10000 min=-10000 mode='sync'                                        ***   1976.14 %     ±114.57% ±154.41% ±204.98%
 crypto/randomInt.js n=100000 max=10000 min=-100 mode='async-parallel'                                ***     43.43 %       ±5.20%   ±6.95%   ±9.10%
 crypto/randomInt.js n=100000 max=10000 min=-100 mode='async-sequential'                              ***   1888.73 %      ±99.87% ±134.60% ±178.69%
 crypto/randomInt.js n=100000 max=10000 min=-100 mode='sync'                                          ***   1895.70 %      ±99.80% ±134.49% ±178.54%
 crypto/randomInt.js n=100000 max=10000 min=-140737488355327 mode='async-parallel'                    ***     44.39 %       ±4.65%   ±6.23%   ±8.22%
 crypto/randomInt.js n=100000 max=10000 min=-140737488355327 mode='async-sequential'                  ***   3455.63 %     ±230.82% ±311.08% ±412.99%
 crypto/randomInt.js n=100000 max=10000 min=-140737488355327 mode='sync'                              ***   3056.68 %     ±158.66% ±213.82% ±283.87%
 crypto/randomInt.js n=100000 max=100 min=-10000 mode='async-parallel'                                ***     39.66 %       ±5.26%   ±7.05%   ±9.27%
 crypto/randomInt.js n=100000 max=100 min=-10000 mode='async-sequential'                              ***   1936.95 %      ±97.77% ±131.76% ±174.89%
 crypto/randomInt.js n=100000 max=100 min=-10000 mode='sync'                                          ***   1885.94 %      ±75.29% ±101.47% ±134.70%
 crypto/randomInt.js n=100000 max=100 min=-100 mode='async-parallel'                                  ***     39.74 %       ±5.70%   ±7.64%  ±10.06%
 crypto/randomInt.js n=100000 max=100 min=-100 mode='async-sequential'                                ***   1878.43 %      ±88.73% ±119.57% ±158.72%
 crypto/randomInt.js n=100000 max=100 min=-100 mode='sync'                                            ***   1892.60 %      ±96.45% ±129.98% ±172.55%
 crypto/randomInt.js n=100000 max=100 min=-140737488355327 mode='async-parallel'                      ***     40.54 %       ±4.49%   ±6.03%   ±7.96%
 crypto/randomInt.js n=100000 max=100 min=-140737488355327 mode='async-sequential'                    ***   3335.10 %     ±234.14% ±315.55% ±418.91%
 crypto/randomInt.js n=100000 max=100 min=-140737488355327 mode='sync'                                ***   3140.61 %     ±170.90% ±230.32% ±305.77%
 crypto/randomInt.js n=100000 max=140737488355328 min=-10000 mode='async-parallel'                    ***     47.40 %       ±4.92%   ±6.60%   ±8.71%
 crypto/randomInt.js n=100000 max=140737488355328 min=-10000 mode='async-sequential'                  ***   3300.80 %     ±182.10% ±245.41% ±325.80%
 crypto/randomInt.js n=100000 max=140737488355328 min=-10000 mode='sync'                              ***   2980.74 %     ±144.38% ±194.58% ±258.31%
 crypto/randomInt.js n=100000 max=140737488355328 min=-100 mode='async-parallel'                      ***     48.80 %       ±5.10%   ±6.86%   ±9.06%
 crypto/randomInt.js n=100000 max=140737488355328 min=-100 mode='async-sequential'                    ***   3385.05 %     ±175.80% ±236.92% ±314.53%
 crypto/randomInt.js n=100000 max=140737488355328 min=-100 mode='sync'                                ***   3041.96 %     ±176.29% ±237.58% ±315.41%
 crypto/randomInt.js n=100000 max=140737488355328 min=-140737488355327 mode='async-parallel'          ***     21.46 %       ±4.38%   ±5.86%   ±7.71%
 crypto/randomInt.js n=100000 max=140737488355328 min=-140737488355327 mode='async-sequential'        ***   2011.00 %     ±146.63% ±197.62% ±262.35%
 crypto/randomInt.js n=100000 max=140737488355328 min=-140737488355327 mode='sync'                    ***   1863.61 %      ±92.30% ±124.39% ±165.14%
 crypto/randomInt.js n=1000 max=10000 min=-10000 mode='async-parallel'                                ***     52.00 %      ±11.51%  ±15.44%  ±20.35%
 crypto/randomInt.js n=1000 max=10000 min=-10000 mode='async-sequential'                              ***    159.33 %      ±10.13%  ±13.58%  ±17.88%
 crypto/randomInt.js n=1000 max=10000 min=-10000 mode='sync'                                          ***    545.42 %      ±38.66%  ±52.05%  ±69.01%
 crypto/randomInt.js n=1000 max=10000 min=-100 mode='async-parallel'                                  ***     38.86 %       ±6.89%   ±9.18%  ±11.99%
 crypto/randomInt.js n=1000 max=10000 min=-100 mode='async-sequential'                                ***    156.91 %      ±13.86%  ±18.61%  ±24.56%
 crypto/randomInt.js n=1000 max=10000 min=-100 mode='sync'                                            ***    605.82 %      ±54.76%  ±73.78%  ±97.89%
 crypto/randomInt.js n=1000 max=10000 min=-140737488355327 mode='async-parallel'                      ***     78.15 %       ±5.72%   ±7.61%   ±9.90%
 crypto/randomInt.js n=1000 max=10000 min=-140737488355327 mode='async-sequential'                    ***    331.03 %      ±20.68%  ±27.83%  ±36.86%
 crypto/randomInt.js n=1000 max=10000 min=-140737488355327 mode='sync'                                ***    516.57 %      ±42.54%  ±57.30%  ±76.01%
 crypto/randomInt.js n=1000 max=100 min=-10000 mode='async-parallel'                                  ***     36.48 %       ±8.30%  ±11.06%  ±14.44%
 crypto/randomInt.js n=1000 max=100 min=-10000 mode='async-sequential'                                ***    152.74 %      ±10.06%  ±13.46%  ±17.65%
 crypto/randomInt.js n=1000 max=100 min=-10000 mode='sync'                                            ***    566.73 %      ±35.87%  ±48.30%  ±64.04%
 crypto/randomInt.js n=1000 max=100 min=-100 mode='async-parallel'                                    ***     31.18 %       ±6.98%   ±9.30%  ±12.11%
 crypto/randomInt.js n=1000 max=100 min=-100 mode='async-sequential'                                  ***    161.54 %      ±11.41%  ±15.28%  ±20.10%
 crypto/randomInt.js n=1000 max=100 min=-100 mode='sync'                                              ***    543.65 %      ±36.10%  ±48.61%  ±64.46%
 crypto/randomInt.js n=1000 max=100 min=-140737488355327 mode='async-parallel'                        ***     97.30 %      ±10.66%  ±14.31%  ±18.89%
 crypto/randomInt.js n=1000 max=100 min=-140737488355327 mode='async-sequential'                      ***    353.64 %      ±29.03%  ±39.09%  ±51.84%
 crypto/randomInt.js n=1000 max=100 min=-140737488355327 mode='sync'                                  ***    494.78 %      ±20.94%  ±28.15%  ±37.24%
 crypto/randomInt.js n=1000 max=140737488355328 min=-10000 mode='async-parallel'                      ***     82.55 %      ±10.74%  ±14.38%  ±18.90%
 crypto/randomInt.js n=1000 max=140737488355328 min=-10000 mode='async-sequential'                    ***    325.50 %      ±17.69%  ±23.80%  ±31.51%
 crypto/randomInt.js n=1000 max=140737488355328 min=-10000 mode='sync'                                ***    525.51 %      ±45.32%  ±61.05%  ±80.97%
 crypto/randomInt.js n=1000 max=140737488355328 min=-100 mode='async-parallel'                        ***     80.98 %      ±11.91%  ±15.94%  ±20.93%
 crypto/randomInt.js n=1000 max=140737488355328 min=-100 mode='async-sequential'                      ***    325.43 %      ±15.49%  ±20.82%  ±27.53%
 crypto/randomInt.js n=1000 max=140737488355328 min=-100 mode='sync'                                  ***    483.61 %      ±36.11%  ±48.61%  ±64.44%
 crypto/randomInt.js n=1000 max=140737488355328 min=-140737488355327 mode='async-parallel'            ***     47.12 %       ±7.40%   ±9.88%  ±12.92%
 crypto/randomInt.js n=1000 max=140737488355328 min=-140737488355327 mode='async-sequential'          ***    164.94 %      ±12.99%  ±17.42%  ±22.93%
 crypto/randomInt.js n=1000 max=140737488355328 min=-140737488355327 mode='sync'                      ***    601.63 %      ±45.89%  ±61.83%  ±82.04%

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

puzpuzpuz · 2021-01-19T06:06:55Z

lib/internal/crypto/random.js

+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) {


An idea for potential enhancement: consider adding options arg with disableEntropyCache setting to allow disabling the random cache. This way will make the API consistent with randomUUID (see #36729).

puzpuzpuz · 2021-01-19T06:10:18Z

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);


Consider using secureBuffer here to integrate with secure heap (see randomUUID's random cache and #36779).

Thanks for the suggestion. We could do that, but UUIDs and random ints always result in insecure memory allocations anyway that mostly destroy any security guarantees. JavaScript is terrible for secure memory management.

jasnell · 2021-02-22T22:29:58Z

@mcollina ... please take another look

lib/internal/crypto/random.js

mcollina

lgtm

tniessen · 2021-04-02T16:10:21Z

I believe this was technically ready to land a while ago, but I just addressed @aduh95's comments regarding primordials.

nodejs-github-bot · 2021-04-03T12:25:18Z

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

tniessen · 2021-04-03T13:31:05Z

Benchmark results still look good:

Unformatted output

15:15:01                                                                                               confidence improvement accuracy (*)     (**)    (***)
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-10000 mode='async-parallel'                              ***     24.11 %       ±3.79%   ±5.04%   ±6.57%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-10000 mode='async-sequential'                            ***   1854.25 %     ±100.81% ±135.85% ±180.34%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-10000 mode='sync'                                        ***   1692.53 %      ±86.05% ±115.97% ±153.96%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-100 mode='async-parallel'                                ***     26.16 %       ±4.40%   ±5.86%   ±7.66%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-100 mode='async-sequential'                              ***   1908.77 %     ±126.68% ±170.72% ±226.62%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-100 mode='sync'                                          ***   1681.47 %      ±83.93% ±113.11% ±150.16%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-140737488355327 mode='async-parallel'                    ***     20.07 %       ±5.60%   ±7.53%   ±9.95%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-140737488355327 mode='async-sequential'                  ***   3343.76 %     ±182.99% ±246.62% ±327.41%
15:15:01  crypto/randomInt.jsn=100000 max=10000 min=-140737488355327 mode='sync'                              ***   2995.21 %     ±184.03% ±248.02% ±329.26%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-10000 mode='async-parallel'                                ***     28.82 %       ±4.26%   ±5.68%   ±7.42%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-10000 mode='async-sequential'                              ***   1854.69 %      ±91.38% ±123.14% ±163.47%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-10000 mode='sync'                                          ***   1725.99 %      ±72.64%  ±97.89% ±129.94%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-100 mode='async-parallel'                                  ***     20.69 %       ±3.74%   ±4.98%   ±6.50%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-100 mode='async-sequential'                                ***   1849.93 %      ±89.96% ±121.23% ±160.92%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-100 mode='sync'                                            ***   1624.21 %      ±42.71%  ±57.56%  ±76.40%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-140737488355327 mode='async-parallel'                      ***     21.91 %       ±5.37%   ±7.21%   ±9.54%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-140737488355327 mode='async-sequential'                    ***   3501.94 %     ±202.95% ±273.51% ±363.11%
15:15:01  crypto/randomInt.jsn=100000 max=100 min=-140737488355327 mode='sync'                                ***   3044.37 %     ±186.10% ±250.80% ±332.97%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-10000 mode='async-parallel'                    ***     22.47 %       ±6.02%   ±8.10%  ±10.71%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-10000 mode='async-sequential'                  ***   3494.36 %     ±215.78% ±290.80% ±386.06%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-10000 mode='sync'                              ***   3017.06 %     ±166.74% ±224.71% ±298.33%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-100 mode='async-parallel'                      ***     29.59 %       ±4.95%   ±6.63%   ±8.74%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-100 mode='async-sequential'                    ***   3281.68 %     ±179.42% ±241.80% ±320.99%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-100 mode='sync'                                ***   2985.85 %     ±242.80% ±327.22% ±434.41%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-140737488355327 mode='async-parallel'          ***     -6.84 %       ±3.26%   ±4.34%   ±5.65%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-140737488355327 mode='async-sequential'        ***   1861.71 %      ±83.21% ±112.12% ±148.83%
15:15:01  crypto/randomInt.jsn=100000 max=140737488355328 min=-140737488355327 mode='sync'                    ***   1652.79 %     ±101.12% ±136.28% ±180.92%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-10000 mode='async-parallel'                                ***     55.56 %       ±6.94%   ±9.29%  ±12.21%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-10000 mode='async-sequential'                              ***    184.54 %      ±14.83%  ±19.90%  ±26.24%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-10000 mode='sync'                                          ***    491.12 %      ±24.91%  ±33.46%  ±44.18%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-100 mode='async-parallel'                                  ***     39.02 %       ±6.75%   ±8.98%  ±11.70%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-100 mode='async-sequential'                                ***    198.34 %      ±14.60%  ±19.62%  ±25.95%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-100 mode='sync'                                            ***    500.96 %      ±26.60%  ±35.77%  ±47.35%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-140737488355327 mode='async-parallel'                      ***     85.51 %      ±10.84%  ±14.54%  ±19.18%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-140737488355327 mode='async-sequential'                    ***    363.90 %      ±16.79%  ±22.56%  ±29.80%
15:15:01  crypto/randomInt.jsn=1000 max=10000 min=-140737488355327 mode='sync'                                ***    704.50 %      ±57.53%  ±77.49% ±102.80%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-10000 mode='async-parallel'                                  ***     51.90 %      ±12.02%  ±16.13%  ±21.27%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-10000 mode='async-sequential'                                ***    200.13 %      ±17.37%  ±23.32%  ±30.80%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-10000 mode='sync'                                            ***    499.05 %      ±30.34%  ±40.84%  ±54.13%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-100 mode='async-parallel'                                    ***     36.25 %       ±9.41%  ±12.52%  ±16.30%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-100 mode='async-sequential'                                  ***    191.84 %      ±16.23%  ±21.81%  ±28.84%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-100 mode='sync'                                              ***    465.86 %      ±19.74%  ±26.53%  ±35.09%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-140737488355327 mode='async-parallel'                        ***     93.88 %      ±12.16%  ±16.32%  ±21.52%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-140737488355327 mode='async-sequential'                      ***    388.58 %      ±27.68%  ±37.28%  ±49.46%
15:15:01  crypto/randomInt.jsn=1000 max=100 min=-140737488355327 mode='sync'                                  ***    772.60 %      ±59.93%  ±80.74% ±107.14%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-10000 mode='async-parallel'                      ***     76.59 %       ±8.42%  ±11.20%  ±14.58%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-10000 mode='async-sequential'                    ***    384.21 %      ±29.08%  ±39.17%  ±51.95%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-10000 mode='sync'                                ***    662.43 %      ±42.43%  ±57.11%  ±75.66%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-100 mode='async-parallel'                        ***     72.87 %       ±8.36%  ±11.13%  ±14.50%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-100 mode='async-sequential'                      ***    384.58 %      ±30.60%  ±41.19%  ±54.58%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-100 mode='sync'                                  ***    658.07 %      ±39.90%  ±53.70%  ±71.14%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-140737488355327 mode='async-parallel'            ***     40.61 %       ±6.91%   ±9.19%  ±11.96%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-140737488355327 mode='async-sequential'          ***    203.08 %      ±20.27%  ±27.28%  ±36.14%
15:15:01  crypto/randomInt.jsn=1000 max=140737488355328 min=-140737488355327 mode='sync'                      ***    521.38 %      ±40.28%  ±54.24%  ±71.94%
15:15:01  
15:15:01  Be aware that when doing many comparisons the risk of a false-positive
15:15:01  result increases. In this case, there are 54 comparisons, you can thus
15:15:01  expect the following amount of false-positive results:
15:15:01    2.70 false positives, when considering a   5% risk acceptance (*, **, ***),
15:15:01    0.54 false positives, when considering a   1% risk acceptance (**, ***),
15:15:01    0.05 false positives, when considering a 0.1% risk acceptance (***)

PR-URL: #35110 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

tniessen · 2021-04-03T13:38:51Z

Landed in 5dae7d6, thanks for reviewing.

PR-URL: #35110 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Sep 8, 2020

tniessen added benchmark Issues and PRs related to the benchmark subsystem. performance Issues and PRs related to the performance of Node.js. labels Sep 8, 2020

tniessen added the review wanted PRs that need reviews. label Sep 14, 2020

tniessen mentioned this pull request Jan 3, 2021

crypto: implement randomuuid #36729

Closed

4 tasks

ChALkeR self-requested a review January 3, 2021 14:35

mcollina requested changes Jan 3, 2021

View reviewed changes

jasnell approved these changes Jan 3, 2021

View reviewed changes

puzpuzpuz reviewed Jan 3, 2021

View reviewed changes

lib/internal/crypto/random.js Outdated Show resolved Hide resolved

puzpuzpuz reviewed Jan 3, 2021

View reviewed changes

lib/internal/crypto/random.js Outdated Show resolved Hide resolved

tniessen mentioned this pull request Jan 12, 2021

crypto: fix randomInt bias #36894

Closed

tniessen force-pushed the crypto-add-randomint-buffering branch from efd2b91 to 499bed1 Compare January 18, 2021 13:17

puzpuzpuz approved these changes Jan 19, 2021

View reviewed changes

aduh95 reviewed Feb 22, 2021

View reviewed changes

lib/internal/crypto/random.js Outdated Show resolved Hide resolved

lib/internal/crypto/random.js Outdated Show resolved Hide resolved

lib/internal/crypto/random.js Outdated Show resolved Hide resolved

mcollina approved these changes Feb 23, 2021

View reviewed changes

guybedford force-pushed the master branch from dc5a5da to 8e46568 Compare March 29, 2021 21:33

tniessen removed the review wanted PRs that need reviews. label Apr 2, 2021

tniessen force-pushed the crypto-add-randomint-buffering branch from 499bed1 to fe80c8e Compare April 2, 2021 12:48

crypto: add buffering to randomInt

2cdb569

tniessen force-pushed the crypto-add-randomint-buffering branch from fe80c8e to 2cdb569 Compare April 2, 2021 13:25

This comment has been minimized.

Sign in to view

aduh95 approved these changes Apr 2, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

tniessen closed this Apr 3, 2021

MylesBorins mentioned this pull request Apr 4, 2021

v15.14.0 proposal #38084

Merged

targos added the backport-blocked-v14.x label May 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto: add buffering to randomInt #35110

crypto: add buffering to randomInt #35110

tniessen commented Sep 8, 2020

nodejs-github-bot commented Sep 8, 2020

bnoordhuis commented Sep 10, 2020

tniessen commented Sep 14, 2020

aduh95 commented Nov 8, 2020

mcollina Jan 3, 2021

ronag Jan 18, 2021

jasnell left a comment

tniessen commented Jan 13, 2021

puzpuzpuz commented Jan 18, 2021

tniessen commented Jan 18, 2021 •

edited

puzpuzpuz Jan 19, 2021

puzpuzpuz Jan 19, 2021

tniessen Apr 2, 2021

jasnell commented Feb 22, 2021

mcollina left a comment

tniessen commented Apr 2, 2021

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Apr 3, 2021

tniessen commented Apr 3, 2021

tniessen commented Apr 3, 2021

crypto: add buffering to randomInt #35110

crypto: add buffering to randomInt #35110

Conversation

tniessen commented Sep 8, 2020

Checklist

nodejs-github-bot commented Sep 8, 2020

bnoordhuis commented Sep 10, 2020

tniessen commented Sep 14, 2020

aduh95 commented Nov 8, 2020

mcollina Jan 3, 2021

Choose a reason for hiding this comment

ronag Jan 18, 2021

Choose a reason for hiding this comment

jasnell left a comment

Choose a reason for hiding this comment

tniessen commented Jan 13, 2021

puzpuzpuz commented Jan 18, 2021

tniessen commented Jan 18, 2021 • edited

puzpuzpuz Jan 19, 2021

Choose a reason for hiding this comment

puzpuzpuz Jan 19, 2021

Choose a reason for hiding this comment

tniessen Apr 2, 2021

Choose a reason for hiding this comment

jasnell commented Feb 22, 2021

mcollina left a comment

Choose a reason for hiding this comment

tniessen commented Apr 2, 2021

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Apr 3, 2021

tniessen commented Apr 3, 2021

tniessen commented Apr 3, 2021

tniessen commented Jan 18, 2021 •

edited