-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[perf] WASM hashing function #3241
base: main
Are you sure you want to change the base?
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
800 bytes is still a sizeable % of the overall bundlesize IIRC. I also don't think we can assume that web assembly is available on the host platform so we'd have to include both versions to avoid breaking changes. cc @emmatown |
For more precise numbers, XXH is 519 bytes of binary, and murmur2 is 386 bytes of binary, so total 800 and 650 after wrapping & base64 is a good figure. |
I have improvements for the JS implementation but I have split them in #3242. |
How much faster is this WASM version than the improved JS function you prepared? Also, what would be the numbers for JS version of xxHash? |
The performance numbers are all in the top comment of that PR, but roughly 2x faster than the improved javascript implementation, which is itself roughly 2x faster than the current one (though I don't know for a JS xxHash, the only existing port I found, https://github.com/pierrec/js-xxhash, is written in an OOP style, so that won't fly for a performance improvement. I took a look at the source code but it's much more dense than murmur2 so I haven't tried porting it yet. On the same topic, would you be open to switch the algorithm if it increases the collision probability (reasonably)? I've been wondering if I could find a simpler algo that maintains a good enough collision rate. I'm not familiar with your codebase, how do you handle collisions? |
Before considering the WASM version, I'd love to see what could be squeezed out of it using a performant JS port of xxHash.
Potentially yes.
We pray they won't happen. |
I've found a performant port of xxh but it can't beat murmur2 in javascript:
Having looked at the implementation for a few hashing functions, I think it's going to be hard to find anything faster than murmur2 in JS. The reason why xxh can be faster than murmur2 in WASM is that its algorithm is easy to optimize & auto-vectorize, so it can use better instructions than murmur2 can, in particular SIMD instructions that can only be accessed via WASM in the browser. |
Follow-up of @emmatown comment in the other PR, the algo needs to be stable in case of server-rendering hydration, which means either murmur2 or xxh in both versions. You have all the numbers here, please let me know if/how you want to proceed. Both algos are neck and neck in JS, but xxh is substantially faster in WASM, so I would recommend switching to that one. |
Goober seems to use a really simple hash algorithm: https://github.com/cristianbote/goober/blob/master/src/core/to-hash.js, and seems to be really fast on the server: https://twind.dev/handbook/introduction.html#opportunities @cristianbote has been iterating on it for some time. But maybe it has a too high collision probability. |
I have been spending way too much time researching hashing functions so here is some data. PerformanceFirst, here are the performance results across engines. The So what I said above:
Is false, and xxh can beat murmur2 in javascript depending on the dataset. Anyway this still supports my previous recommendation to switch to xxh. The results on SpiderMonkey are however puzzling to me, but tbh the engine seems to have weird performance cliffs in many situations, I've given up on trying to understand why. Collision rateI have plotted the distribution of hashes for the "Numbers" dataset as described in this stackoverflow answer, for the different functions:
Nothing to add here, the plots speak for themselves. So summary is, goober not good for us. asm.js annotations@cristianbote Quick win for goober though, you can add asm.js annotations to speed it up, JSC seems to need those: // original
while (i < str.length) out = (101 * out + str.charCodeAt(i++)) >>> 0;
// asm.js
while (i < str.length) out = (Math.imul(101, out) + str.charCodeAt(i++) | 0) >>> 0; LinksAll benchmarking code here: https://github.com/romgrk/js-benchmark # benchmarks
node ./index.js ./benchmarks/hashing.js
# plots
node ./hash-testing.js |
Funny thought I had but I took the fast hex formatting function from mui/material-ui#43289 and replaced the final |
wow, nice! Love these insights @romgrk Took them for a spin locally on my end and I can confirm your results. Don't wanna side-track the conversation here about |
FYI, I'm on PTO this week. I can continue the conversation when I get back |
I work at MUI and I've been doing some performance optimization. I've noticed that
murmur2
is at the top of the stack traces for the test case benchmark I'm using, so I wanted to open a PR here to discuss options to improve the performance.I've extracted some actual style strings that are hashed at runtime by emotion (from the test case above) and created a benchmark to compare emotion's JS
murmur2
implementation to alternatives. I used rust+wasm to implement those alternatives, because the use-case is a good match for the performance benefits of wasm. Here is a summary of different versions compared to the current onePassing strings from JS to WASM implies encoding them in an array buffer, so the performance scales differently on the input string length. The original JS version performs about 25% faster than the rest for small strings (up until a length of around 64 characters), but after that the WASM versions take the lead. The table below shows the results for the strings extracted at runtime described above which is imho close enough to what happens in production for real-word users.
murmur2_original
murmur2_rust()
xxh_rust()
xxh3_rust()
city_hash_rust()
adaptive()
The fastest option is xxHash, which is newer and faster than murmur2, with a similar collision rate.
adaptive
is a combination of usingmurmur2_original
for small strings andxxh_rust
for large strings, but the additional branching instruction (if (string.length > 64) ...
) makes it slower.Using a WASM module also doesn't need to add any build/bundle time complexity for users, as we can ship the module inline, something like this. The compiled wasm module is around 500 bytes, so a realistic cost for the wrapper and base64 bytes would be around 800 bytes.
I have also benchmarked the MUI test case again after replacing the hashing function. The results aren't as impressive as I expected (I was hoping for a 70% reduction of hashing time based on the table above), but this change improves MUI's rendering performance by about 2-4%.
The code in this PR is a draft but I'd be happy to have an early review/opinion before doing more work.