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

Fix and optimization for non-x86 architectures #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

buu700
Copy link

@buu700 buu700 commented May 5, 2021

Addresses #21. I wasn't sure based on that thread whether you'd prefer to remove sse4_crc32 entirely, or if we're keeping it then whether it should be above or below @node-rs/crc32, but I'm happy to make any changes you'd like before merging.

@Brooooooklyn
Copy link

Note, the @node-rs/crc32 can replace the sse4_crc32 completely, it use sse CPU instruction too, and faster than sse4_crc32, easier to install because it is without postinstall scripts and build dependencies.

@Brooooooklyn
Copy link

So I think the sse4_crc32 doesn't need any more, what do you think?

@buu700
Copy link
Author

buu700 commented Oct 30, 2021

I didn't see a need for it either, personally. @node-rs/crc32 claims to be faster and seems to be more actively maintained, but it's up to @ashi009.

@madebymichaelg
Copy link

Hey @ashi009 👋, is there a chance we can get this merged? This is a blocker in my organization

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

Successfully merging this pull request may close these issues.

4 participants