-
Notifications
You must be signed in to change notification settings - Fork 177
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
Thread safety #29
Comments
We use it in millions of calls every minute in a multithreaded app using a static singleton instance. It works fine as-is. There are lots of other performance improvements that we made for allocations though in our internal fork but you should be fine as far as thread-safety. |
@manigandham thanks for chiming in, that's reassuring. For the library maintainers I suggest creating some test cases for thread safety of all the methods just be sure, the performance of a single instance is just too good just to limit this library to always create a new instance. It will only add to the awesomeness of this library. |
@Exocomp there are no maintainers in plural, only me sadly. I would however be very happy if someone would make a PR with those test cases. I recently had a kid and have had rather limited spare time.. I will get there and create those tests, but maybe not as soon as you'd like! |
@ullmark thank you for creating this library it is super useful! Congrats on your child and I absolutely understand regarding time. I'm super busy also but hopefully sometime in the future we can find some time. |
@manigandham any chance of upstreaming some of those performance updates into this repo? |
So, the deal is that I don't really use windows much these last years, I'm going to see if I can get it running in macOs and .net core. Otherwise I might be able to remote to a windows computer. Its great to hear @manigandham, but are there a way to really test thread safety? When this ticket came in I tried googling how to verify that in .net, but couldn't really find anything good? I would gladly accept someone to contribute to this library and give push/publish access as well, as I've asked earlier. |
You can test by creating new threads that call the same instance and use Also the older target frameworks can probably be dropped since they're end of life already, and .NET Framework 4.8 is the last/latest Windows framework which already supports I can help as a contributor, let me know |
@manigandham I'll give you access! 👍 |
@ullmark Got it, thanks. I'll see what I can do in the next few days |
HashIds store simple data types inside and don't modify it. So, yes, it is thread safe. |
hey @ullmark I merged a bunch of bug fixes and improvements, updated tests, added the github actions workflow, and updated library version to 2.0.0. It should be ready for publishing to nuget. Let me know. |
Hi, @manigandham, final optimizations: #43 |
@manigandham let me just briefly explain my thoughts on versioning of this library. This is initially a port of a javascript (or PHP whichever was first) library that now exists in 40+ languages. One of my friends built the Ruby version so I would say the code base is influenced by his port. I tried to make build it by taking influences from JS to make sure that any changes to the algorithm could be made to .net version as easy as possible, ensuring that a hash created in one language could be used in another given the settings are the same. At first I followed the versioning of the other libraries, but as where I added features like "long"-support that made no sense in for instance javascript, I started bumping minor version when I added non breaking stuff. As for these updates I see no reason for updating major version since it doesn't have any breaking changes or big new features? |
@manigandham going to see if I can give you access to publish on nuget as well. |
Impressive results from @Daramant
Build and tests all pass: https://github.com/ullmark/hashids.net/runs/2411647475 @ullmark all set to release version 1.4.0 |
hi @ullmark are you still planning on publishing? anything I can do to help? |
hi @manigandham. A question; Is there any way of using github for that, seeing a couple of new panels. Otherwise I could add so you have access publishing on nuget, what's your account there? |
seeing microsoft now owns github right, it should be possible to setup something that deploys directly to nuget? (since I don't really use windows anymore... ) |
There's nothing automatic in github. It supports package hosting but separately from nuget. There are github actions that can push to nuget but I havent set that up before. My nuget username is https://www.nuget.org/profiles/manigandham |
@manigandham I created a new Organization called "hashids.net" on nuget where I also invited you as a collaborator. I removed myself as owner of the package and let the organization be the only owner. I believe you should be able to publish new versions of the package now. Feel free to email me at [email protected] in the future if there is something you want to discuss. There is also a "hashids" organization on Github, I can ask them to invite you, but to be honest, not much is happening there 😄 |
Hey everyone - the latest release 1.4.0 is now live: https://www.nuget.org/packages/Hashids.net/1.4.0 |
On a quad core/4hz machine with .NET Core 3.1 using static vs creating a new instance is about 110 times slower (using encode/decode) testing with 1M integers.
Using a single instance is super fast, so due to performance gains I'd like to use a single instance and share it across threads. So therefore it brings into question thread safety.
I've been testing encode/decode with Parallel.For and I don't see any issues. No locks and ids generated are valid compared to creating new instances.
I'd appreciate if you can confirm thread safety for this library, due to performance gains its worth using a single instance instead of just creating a new instance every time.
The text was updated successfully, but these errors were encountered: