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

Cache the hashids object. #40

Merged
merged 1 commit into from
May 28, 2020
Merged

Cache the hashids object. #40

merged 1 commit into from
May 28, 2020

Conversation

alexkiro
Copy link
Contributor

Creating the Hashids and then encoding the id every time appears to take ~3x more time than having the Hashids object cached and ready to encode a new id.

A change like this does seem to improve performance quite a bit for scenarios where a lot of objects are being serialized.


I've made the changes backwards compatible (I think), but let me know if something else would be preferred instead.

Creating the Hashids and then encoding the id
every time appears to take 3x more time than
having the Hashids object cached and ready to
encode a new id.
@nshafer nshafer self-assigned this May 27, 2020
@nshafer nshafer merged commit cdc6b7d into nshafer:master May 28, 2020
@nshafer
Copy link
Owner

nshafer commented May 28, 2020

Thanks, Alex... there was one small problem with the arguments in rest that was causing tests to fail, but I fixed that. I also redid the Hashid class slightly to pull the details out of the passed Hashid instance so that __reduce__ works properly (for pickling). Finally, I was curious if this actually made much of a difference for performance and was quite pleased to see it more than doubled performance, and also reduced memory usage (though that was negligible to begin with).

In [14]: timeit('Hashid(123, salt="asdf", min_length=7)', setup='from hashid_field.hashid import Hashid', numb
    ...: er=10_000)                                                                                           
Out[14]: 0.5867186750001565

In [15]: timeit('Hashid(123, hashids=hashids)', setup='from hashid_field.hashid import Hashid; from hashids im
    ...: port Hashids; hashids=Hashids(salt="asdf", min_length=7)', number=10_000)                            
Out[15]: 0.22478430900082458

In [16]: timeit('Hashid(123, salt="asdf", min_length=7)', setup='from hashid_field.hashid import Hashid', numb
    ...: er=100_000)                                                                                          
Out[16]: 5.921704006999789

In [17]: timeit('Hashid(123, hashids=hashids)', setup='from hashid_field.hashid import Hashid; from hashids im
    ...: port Hashids; hashids=Hashids(salt="asdf", min_length=7)', number=100_000)                           
Out[17]: 2.2223077329999796

In [18]: timeit('Hashid(123, salt="asdf", min_length=7)', setup='from hashid_field.hashid import Hashid', numb
    ...: er=1_000_000)                                                                                        
Out[18]: 58.524623629000416

In [19]: timeit('Hashid(123, hashids=hashids)', setup='from hashid_field.hashid import Hashid; from hashids im
    ...: port Hashids; hashids=Hashids(salt="asdf", min_length=7)', number=1_000_000)                         
Out[19]: 22.343947568000658

Anyways, it's live in 3.1.2.

@alexkiro
Copy link
Contributor Author

Thanks, Alex... there was one small problem with the arguments in rest that was causing tests to fail, but I fixed that.

Sorry about that. I did try to run a test suite, but I think I must have missed it.

Finally, I was curious if this actually made much of a difference for performance and was quite pleased to see it more than doubled performance, and also reduced memory usage (though that was negligible to begin with).

Neat! Thanks for your awesome work on this library!

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

2 participants