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

Store shuffled alphabets so that they don't have to be calculated again. #105

Merged
merged 3 commits into from
Feb 20, 2018

Conversation

jwpage
Copy link
Contributor

@jwpage jwpage commented Feb 20, 2018

This changes the Hashids::bench function so that it will store a shuffled alphabet once it's been created. This increases the performance of the function, especially when calculating large numbers of hashes.

Here are a couple of blackfire.io profiles for encoding 50,000 hashes.

@vinkla vinkla requested review from vinkla and jd327 February 20, 2018 20:54
@vinkla vinkla self-assigned this Feb 20, 2018
@vinkla vinkla removed their request for review February 20, 2018 20:54
@vinkla
Copy link
Owner

vinkla commented Feb 20, 2018

Thanks for the pull request @jwpage! This pull request looks good to me.

@ivanakimov what do you say?

@jd327
Copy link
Collaborator

jd327 commented Feb 20, 2018

@vinkla @jwpage this is super awesome.

@jwpage thanks for providing benchmarks as well! I was trying to think of any scenarios where this might break anything, but I guess besides memory consumption if the tests are good, this should be a nice optimization.

@vinkla if looks good to you, let's get it out!

@vinkla vinkla merged commit d83428a into vinkla:master Feb 20, 2018
@vinkla
Copy link
Owner

vinkla commented Feb 20, 2018

Merged! This will be part of version 2.1 which will be released in the coming weeks. For now, you could set minimum-stability to dev and start using version 2.1 in development today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants