search

rurban / smhasher

Hash function quality and speed tests
https://rurban.github.io/smhasher/
Other
1.85k stars 178 forks source link

Fix #191 #268

Closed o0101 closed 1 year ago

o0101 commented 1 year ago

Hi @rurban

Long time! Hope you're doing well. I see smhasher has continued to grow and become so much more! That's great!

I think this fixes the memory alignment issues of discohash (#191). If it does, turns out the fix was quite simple, just using alignas.

Also, just for interest: I tried using CXX=clang++ and found it produces results that are about 15% faster, so I'm throwin' it it out there!

I know you're super busy so no worries if you have no time to get to this PR, just thanks for reading this far!

o0101 commented 1 year ago

@rurban Alright man I'll jump on that!

I tried adding -DCMAKE_BUILD_TYPE=Debug but then running SMHasher BEBB4185 said BEBB4185 was an unknown hash. I couldn't find the place to change that so I just assumed it was taken out of circulation for Debug due to its undefined behavior maybe! :)

o0101 commented 1 year ago

@rurban done!

o0101 commented 1 year ago

@rurban done! Changed build.sh back to normal and also aligned the input void *key 😵 !

o0101 commented 1 year ago

@rurban RAA! It is DONE! ✌🏻

o0101 commented 1 year ago

@rurban hey man I'm sorry this will probably give you a lot of trouble, so I'm sorry to bug you with this request, but i was wondering, can we retire the name BEBB4185 and now call it discohash ? This is inline with https://github.com/dosyago/discohash and I think it's easier to catch if it uses a real human English name not a hex value! :)

o0101 commented 1 year ago

Yes sir! 🫡

o0101 commented 1 year ago

Alright, @rurban it is TRULY NOW DONE THIS TIME! :)

Would it be a terrible idea to re-run the hash quality tests? I noticed it's faster!

rurban commented 1 year ago

At home it is slower. Will test at the benchmark machine on mondays

o0101 commented 1 year ago

@rurban makes sense, because it added alignment. I only tested on a new machine, was surprised at the speed. Didn't benchmark the old tho!