crypto: add buffering to randomInt by tniessen · Pull Request #35110 · nodejs/node

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 (3988798fa4bdbd3ade4cb23289ec0ae05621d438 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!

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

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

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

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

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

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

Comment threadlib/internal/crypto/random.js Outdated

puzpuzpuz reviewed Jan 3, 2021
View reviewed changes

Comment threadlib/internal/crypto/random.js Outdated

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 499bed1Compare January 18, 2021 13:17

puzpuzpuz approved these changes Jan 19, 2021
View reviewed changes

aduh95 reviewed Feb 22, 2021
View reviewed changes

Comment threadlib/internal/crypto/random.js Outdated
Comment threadlib/internal/crypto/random.js Outdated
Comment threadlib/internal/crypto/random.js Outdated

mcollina approved these changes Feb 23, 2021
View reviewed changes

guybedford force-pushed the master branch from dc5a5da to 8e46568Compare 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 fe80c8eCompare April 2, 2021 12:48

crypto: add buffering to randomInt
2cdb569

tniessen force-pushed the crypto-add-randomint-buffering branch from fe80c8e to 2cdb569Compare 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

Uh oh!

Uh oh!

Conversation

tniessen commented Sep 8, 2020

Checklist

Uh oh!

nodejs-github-bot commented Sep 8, 2020

Uh oh!

bnoordhuis commented Sep 10, 2020

Uh oh!

tniessen commented Sep 14, 2020

Uh oh!

aduh95 commented Nov 8, 2020

Uh oh!

Uh oh!

Uh oh!

mcollinaJan 3, 2021

Choose a reason for hiding this comment

Uh oh!

ronagJan 18, 2021

Choose a reason for hiding this comment

Uh oh!

jasnell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tniessen commented Jan 13, 2021

Uh oh!

puzpuzpuz commented Jan 18, 2021

Uh oh!

tniessen commented Jan 18, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

puzpuzpuzJan 19, 2021

Choose a reason for hiding this comment

Uh oh!

puzpuzpuzJan 19, 2021

Choose a reason for hiding this comment

Uh oh!

tniessenApr 2, 2021

Choose a reason for hiding this comment

Uh oh!

jasnell commented Feb 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

tniessen commented Apr 2, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Apr 3, 2021

Uh oh!

tniessen commented Apr 3, 2021

Uh oh!

tniessen commented Apr 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tniessen commented Jan 18, 2021•
edited
Loading