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

Add CRC32C API #972

Closed
mattbishop opened this issue Jun 14, 2021 · 4 comments
Closed

Add CRC32C API #972

mattbishop opened this issue Jun 14, 2021 · 4 comments
Labels
suggestion a suggestion yet to be agreed

Comments

@mattbishop
Copy link

I have been using a node wrapper of a rust CRC32C implementation: https://github.com/napi-rs/node-rs/tree/main/packages/crc32

This capability makes sense to be in the Deno std lib because it takes advantage of CPU-specific hardware instructions to calculate CRC values. It supports a wide variety of architectures.

@wperron wperron added the suggestion a suggestion yet to be agreed label Jun 14, 2021
@FujiHaruka
Copy link
Contributor

I took a glance at hash/_wasm.

We can add a hash algorithm in rust by adding to this list in lib.rs.

https://github.com/denoland/deno_std/blob/f4496b78ef3874bdc06f6a6de10cdadb97b6f2bb/hash/_wasm/src/lib.rs

All hash structs here implement Digest traits.

However, crc32fast, which is the most popular crc32 library in crates.io, doesn't implement Digest traits. We have something to do in order to import crc32fast to _wasm. (But I don't know much about rust).

@jeremyBanks
Copy link
Contributor

jeremyBanks commented Aug 1, 2021

This capability makes sense to be in the Deno std lib because it takes advantage of CPU-specific hardware instructions to calculate CRC values.

deno_std isn't able to take advantage of CPU-specific hardware instructions directly; it's implemented in TypeScript(/JavaScript/WASM). To take full advantage of something it would need to be added to the Deno runtime directly. However, I think WASM SIMD is now supported in Deno's v8, so a WASM implementation might be able to perform competitively? (See here for the change to _build.ts required to enable it, I think, I haven't tested it much: jeremyBanks@d9dd17f#diff-313a3ff5e24b77914cc23ee8c8d0c49b0406f3704ec36962551a2f68d315534cR47)

We can add a hash algorithm in rust by adding to this list in lib.rs.

Sorry, I probably should have left a note in that directory, but hash/_wasm is deprecated and being replaced with hash/crypto/hash/_wasm_crypto (see #1025). The new implementation doesn't require the Digest trait,
it's mostly a bunch of match statements (see https://github.com/denoland/deno_std/blob/main/_wasm_crypto/src/digest.rs), so if we wanted to add CRC support to that interface it should be pretty straightforward.

@iuioiua
Copy link
Contributor

iuioiua commented Nov 29, 2023

Hi guys, what are your current thoughts on this suggestion now that some time has passed? CC @mattbishop @jeremyBanks

@iuioiua
Copy link
Contributor

iuioiua commented Dec 13, 2023

I'm going to close this as I haven't seen sufficient demand for this. If anyone is still strongly for having this functionality, please feel free to re-open this issue. Either way, thank you for your input, guys.

@iuioiua iuioiua closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion a suggestion yet to be agreed
Projects
None yet
Development

No branches or pull requests

5 participants