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

Missing asm.js annotations #1

Closed
romgrk opened this issue Aug 28, 2024 · 3 comments
Closed

Missing asm.js annotations #1

romgrk opened this issue Aug 28, 2024 · 3 comments

Comments

@romgrk
Copy link

romgrk commented Aug 28, 2024

I've been benchmarking hashing algos and I noticed that your implementation is missing a few asm.js annotations to appease the JS engines, details here: cristianbote/goober#588 (comment)

@cgiosy
Copy link
Owner

cgiosy commented Nov 5, 2024

Hello. Sorry for the delayed response!
I've looked at your benchmark code, and haven't found any significant differences without that it uses an Int32Array of a pre-allocated buffer. (Additionally, DataView is faster than Int32Array in my experiments.)
This change seems difficult to apply to this library, which takes a buffer as input directly. It seems like a good idea to add a function that takes text as input.
Could you explain in more detail what other parts need to be changed?

@cgiosy
Copy link
Owner

cgiosy commented Nov 5, 2024

Here are the results for the modified benchmark (on my M1 Macbook Air):

### node: v22.6.0 ###
CASE: murmur2_unmodified: ##################................................   37.57% (426 ops) (2.347 ms/op)
CASE: murmur2:            ############################......................   56.53% (641 ops) (1.562 ms/op)
CASE: murmur2_dataview:   ############################......................   57.94% (657 ops) (1.524 ms/op)
CASE: xxh:                ##########################################........   84.04% (953 ops) (1.049 ms/op)
CASE: xxh_dataview:       ##################################################  100.00% (1,134 ops) (0.8818 ms/op)
CASE: fnv-1a:             ###################...............................   38.80% (440 ops) (2.275 ms/op)
CASE: djb2a:              #######################...........................   46.74% (530 ops) (1.887 ms/op)
CASE: goober:             #####################.............................   42.42% (481 ops) (2.081 ms/op)

@romgrk
Copy link
Author

romgrk commented Nov 5, 2024

Good to know, I'll close the issue then. The few modifications that exist in the benchmark repo might explain why the annotations make a difference there (the V8 disassembly was clear that they were needed though).

Thanks for sharing about the DataView, I'm in the process of implementing a light replacement for EmotionCSS at MUI, and I'll probably be picking your implementation of xxh for the hashing (unless I can find a way to avoid, hashing in JS is painful).

@romgrk romgrk closed this as completed Nov 5, 2024
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

No branches or pull requests

2 participants