Skip to content

Conversation

Eomm
Copy link
Member

@Eomm Eomm commented Jan 18, 2022

Not sure about the randomUUID() performance.
I would use something more basic as Math.random() or crypto.randomBytes.

Need to try it

Copy link
Member

@zekth zekth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Math.random can have collision AFAIK

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you share the benchmark?

@Eomm
Copy link
Member Author

Eomm commented Jan 18, 2022

Running the script on Node.js 16

randomInt x 121,083,649 ops/sec ±0.35% (595 runs sampled)
randomInt + now x 9,471,249 ops/sec ±0.07% (596 runs sampled)
uuid x 24,935,836 ops/sec ±0.74% (589 runs sampled)
math.random x 113,739,051 ops/sec ±1.56% (566 runs sampled)
math.random + now x 6,165,940 ops/sec ±0.44% (591 runs sampled)

Math.random can have collision AFAIK

True, but I'm not so lucky and I never hit this case.
For this, I tried the + now() concatenation but it is the worse performance.

const Benchmark = require('benchmark')
Benchmark.options.minSamples = 500

const crypto = require('crypto')

Benchmark.Suite()
  .add('randomInt', function () { crypto.randomInt(10e6) })
  .add('randomInt + now', function () { return `${Date.now()}${crypto.randomInt(10e6)}` })
  .add('uuid', function () { crypto.randomUUID() })
  .add('math.random', function () { return Math.random() * 10e6 })
  .add('math.random + now', function () { return `${Date.now()}${Math.random() * 10e6}` })
  .on('cycle', function (event) { console.log(String(event.target)) })
  .on('complete', function () {})
  .run()

@RafaelGSS
Copy link
Member

Running the script on Node.js 16

randomInt x 121,083,649 ops/sec ±0.35% (595 runs sampled)
randomInt + now x 9,471,249 ops/sec ±0.07% (596 runs sampled)
uuid x 24,935,836 ops/sec ±0.74% (589 runs sampled)
math.random x 113,739,051 ops/sec ±1.56% (566 runs sampled)
math.random + now x 6,165,940 ops/sec ±0.44% (591 runs sampled)

This benchmark in comparison to the master has degraded the performance? I mean, I could not compare with the result in the README or my local benchmarks.

@mcollina
Copy link
Member

IMHO randomUUID() is fast enough to not cause problems in practice.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Eomm Eomm merged commit 318cc44 into master Jan 18, 2022
@Eomm Eomm deleted the Eomm-patch-1 branch January 18, 2022 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants