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

Move to NanoID #121

Merged
merged 1 commit into from Jul 8, 2018
Merged

Move to NanoID #121

merged 1 commit into from Jul 8, 2018

Conversation

@shashkovdanil
Copy link
Contributor

@shashkovdanil shashkovdanil commented Jul 7, 2018

Integrated nanoid for generating id

@ai
Copy link
Collaborator

@ai ai commented Jul 7, 2018

@shashkovdanil can you test distribution and use NanoID performance benchmark to prove that we improve shortid?

@shashkovdanil
Copy link
Contributor Author

@shashkovdanil shashkovdanil commented Jul 7, 2018

@ai, OK.

@shashkovdanil
Copy link
Contributor Author

@shashkovdanil shashkovdanil commented Jul 7, 2018

@ai, shortid was improved 🚀

has flat distribution test:

has flat distribution shortid with nanoid

    expect(received).toBeCloseTo(expected, precision)

    Precision: 1-digit
    Expected: 1
    Received: 1.3898666666666666


has flat distribution shortid original

    expect(received).toBeCloseTo(expected, precision)

    Precision: 1-digit
    Expected: 1
    Received: 2.3356444444444446

Benchmark test:

  • With nanoid
nanoid          149,530 ops/sec 
nanoid/generate 154,555 ops/sec 
uuid/v4         148,375 ops/sec 
shortid          12,988 ops/sec 
  • Original
nanoid          149,435 ops/sec 
nanoid/generate 148,073 ops/sec 
uuid/v4         152,058 ops/sec 
shortid          14,737 ops/sec 
@ai
Copy link
Collaborator

@ai ai commented Jul 7, 2018

@shashkovdanil in this test every symbol had different distribution, so you can't just show one number.

Post chars[k] content instead.

Also performance reducing if not do great.

@ai ai merged commit c15864c into dylang:master Jul 8, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ai
Copy link
Collaborator

@ai ai commented Jul 8, 2018

I improved performance:

shortid old        33,979 ops/sec 
shortid new        82,404 ops/sec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.