Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add benchmark #430

Merged
merged 9 commits into from May 1, 2020
Merged

test: add benchmark #430

merged 9 commits into from May 1, 2020

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented Apr 30, 2020

Add a simple benchmark to be able to catch order-of-magnitude
performance regressions.

@ctavan
Copy link
Member Author

ctavan commented Apr 30, 2020

It does not yet have browser support but I believe that if at all, uuid performance might be critical only on the server (think of a system that generates events at a very high rate and uses uuid as event IDs).

I have no proof, but I would assume that such high-frequency use cases for uuid are much less likely in the browser.

@awwit
Copy link
Contributor

awwit commented Apr 30, 2020

@ctavan I think performance testing in the browser should also occur locally. If we delegate this task to a third-party service, then distortions in the results may occur. Since we cannot know what tasks are performed in parallel on third-party machines…

@ctavan
Copy link
Member Author

ctavan commented Apr 30, 2020

Adding browser support was easy enough so I just added it:
image

Add a simple benchmark to be able to catch order-of-magnitude
performance regressions.
@ctavan
Copy link
Member Author

ctavan commented Apr 30, 2020

Agree @awwit. I think if at all, this benchmark will only allow us to compare before/after of a certain change on the same machine.

I don't think we can get comparable results over time and I also don't intend to do this.

@broofa
Copy link
Member

broofa commented May 1, 2020

FWIW, here's the results for node 12.10.0 on my laptop (same laptop I used for the other results I posted)

uuidv1() x 1,472,274 ops/sec ±0.62% (91 runs sampled)
uuidv1() fill existing array x 4,389,392 ops/sec ±0.44% (87 runs sampled)
uuidv4() x 348,408 ops/sec ±0.75% (87 runs sampled)
uuidv4() fill existing array x 390,717 ops/sec ±1.32% (84 runs sampled)
uuidv3() DNS x 137,150 ops/sec ±0.49% (88 runs sampled)
uuidv3() URL x 126,044 ops/sec ±0.75% (89 runs sampled)
uuidv3() MY_NAMESPACE x 129,816 ops/sec ±0.45% (92 runs sampled)
uuidv5() DNS x 135,630 ops/sec ±0.68% (88 runs sampled)
uuidv5() URL x 123,146 ops/sec ±1.03% (90 runs sampled)
uuidv5() MY_NAMESPACE x 127,863 ops/sec ±0.67% (84 runs sampled)
Fastest is uuidv1() fill existing array

Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

A few suggestions, otherwise looks good. Thanks!

README_js.md Outdated
@@ -353,6 +353,17 @@ import { v4 as uuidv4 } from 'uuid';
Workers](https://caniuse.com/#feat=cryptography) and we are not aware of a polyfill (let us know if
you find one, please).

## Developing
Copy link
Member

Choose a reason for hiding this comment

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

This isn't of interest to most readers. Move to PR template (preferably the hidden .github/pull_request_template.md file).

examples/benchmark/benchmark.js Outdated Show resolved Hide resolved
examples/benchmark/benchmark.js Outdated Show resolved Hide resolved
examples/benchmark/benchmark.js Outdated Show resolved Hide resolved
examples/benchmark/benchmark.js Show resolved Hide resolved
@ctavan ctavan requested a review from broofa May 1, 2020 12:23
@ctavan ctavan merged commit 2b79686 into master May 1, 2020
@ctavan ctavan deleted the benchmark branch May 1, 2020 12:34
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.

None yet

3 participants