gh-115704: Improve DJBX33A hash algorithm#115705
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
1 similar comment
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
77a701a to
d951214
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
1 similar comment
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
picnixz
left a comment
There was a problem hiding this comment.
I'd like you to address Serhiy's concern on the issue as well. And could we have the actual assembly code being generated actually (and it's comparison)?
| hash = hash * 33 * 33 * 33 * 33 + | ||
| p[0] * 33 * 33 * 33 + | ||
| p[1] * 33 * 33 + | ||
| p[2] * 33 + | ||
| p[3]; |
There was a problem hiding this comment.
I think you can directly compute those values (namely the powers of 33), although the compiler should be able to optimize it as well. You could however add a small comment.
There was a problem hiding this comment.
Thank you for your review. Done.
| if (len >= 2) { | ||
| if (len > 2) { | ||
| hash = hash * 33 * 33 * 33 + | ||
| p[0] * 33 * 33 + | ||
| p[1] * 33 + | ||
| p[2]; | ||
| } | ||
| else { | ||
| hash = hash * 33 * 33 + p[0] * 33 + p[1]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Does the compiler behaves differently if you use
if (len > 2) { ... }
else if (len == 2) { ... }
else if (len) { ... }instead?
|
this PR is quite fundamentally different from the Lemire post/the PHP change. In PHP, the change was done in the implementation of the hash function for arbitrary lengths. In this PR, only the code for computing the hash of bytes with This might still be a worthwhile change, but there should be really strong benchmarking results that show this. |
|
I support it only to clean up a little by getting rid of the ugly (IMO) C fall-through code and replacing it with a simple loop. I don't think the compiler will generate anything significantly different. |
Accelerating python hash algorithm by "unoptimizing" it when using DJBX33A as hash algorithm. See Daniel Lemire's blog post: https://lemire.me/blog/2016/07/21/accelerating-php-hashing-by-unoptimizing-it/ Signed-off-by: PeterYang12 <yuhan.yang@intel.com>
bf9cfa8 to
0364811
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
The following commit authors need to sign the Contributor License Agreement: |
|
This PR is stale because it has been open for 30 days with no activity. |
|
I experimented a bit with the approach here (using godbolt), but found no real speedups. Also note that by default the code is not used as For the siphash13 that is used there is an optimization possible by pre-computing the v0, v1, v2, v3 from k0 and k1 (the k0 and k1 are set at initialization time). For small strings it makes an 5% difference. If anyone is interested, feel free to pursue that. PR can be closed afaic |
|
Thanks for checking @eendebakpt, additionally the CLA is unsigned and no benchmarks have been provided as requested, so I'm closing. |
Accelerating python hash algorithm by "unoptimizing" it when using DJBX33A as hash algorithm.
See Daniel Lemire's blog post:
https://lemire.me/blog/2016/07/21/accelerating-php-hashing-by-unoptimizing-it/
This idea has already been implemented in the PHP interpreter.