Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Feature - Expose ZLib crc32() function #7213

Closed
JavaScriptDude opened this issue Feb 28, 2014 · 14 comments
Closed

Feature - Expose ZLib crc32() function #7213

JavaScriptDude opened this issue Feb 28, 2014 · 14 comments

Comments

@JavaScriptDude
Copy link

I have a need to do cross platform simple file validation using Node.js. I am presently building a backup file distribution and synchronization system using Node.js and need a solid and fast way of generating a check sum of the files on the sender and receiver sides.

The files are on servers I own so I don't need crypto hashes for my purpose.

I know that zlib has a crc32 function under the hood and it would be great to have this available in JavaScript.

I know that others have written crc32 implementations in JavaScript for Node but IMO, that is not the correct approach.

Any reason why this should not be implemented?

@indutny
Copy link
Member

indutny commented Mar 1, 2014

Hello!

There is only one reason, it could be implemented in user-land without any pain: https://www.npmjs.org/search?q=crc32 . And you get often updates and choice for free! :)

Cheers,
Fedor.

P.S.

Generally this is our policy for the most of proposed features: if they could be implemented in user-land - avoid putting them in core.

@indutny indutny closed this as completed Mar 1, 2014
@JavaScriptDude
Copy link
Author

I need to calculate CRC's on very large files. I have not tested yet but I would guess that the implementing CRC32 in JavaScript would be significantly slower than native C code.

I will set up some tests but I have not yet found any userland CRC modules designed to handle streams so I have a bit of hacking to do.

@JavaScriptDude
Copy link
Author

After a partial re-write of the crc32 package, I was able to do some good testing with file streams. I tested with a large file of approximately 750MB and crc32 algorithm in JavaScript took about 10 seconds on my laptop. rhash crc32 calculation took about 2.5 seconds.

This performance delta is completely expected.

The big question is why not expose the zlib.crc32() function to enable a native crc32 function call to allow for native file checksum processing that in a high performance and cross platform manner?

@TooTallNate
Copy link

Frankly, if zlib offers it, I see no reason to not expose a binding.

@indutny
Copy link
Member

indutny commented Mar 1, 2014

@TooTallNate it has not the fastest implementation of it. @JavaScriptDude could you please try benchmarking https://www.npmjs.org/package/sse4_crc32 or https://www.npmjs.org/package/crc32c ?

@Mithgol
Copy link

Mithgol commented Mar 1, 2014

These two packages are addon-containing, i.e. npm install won't work on Windows at all (crc32c says it's Linux-only) or would require Visual Studio to be installed beforehand. Core modules are more comfortable.

if they could be implemented in user-land — avoid putting them in core.

You can't avoid putting zlib's crc32 implementation in the core. It's already there. The only remaining work is exposing it.

If documenting and supporting and testing zlib's crc32 implementation seems tedious, make the exposed implementation unofficial and non-documented (such as require('util')._extend() currently is). It would still help the community, people would just write their own tests (such as that one) to see for themselves if anything breaks.

@indutny
Copy link
Member

indutny commented Mar 1, 2014

Then I could say only that Pull Requests for this feature would be welcome :)

@JavaScriptDude
Copy link
Author

Cool. I may take a stab at this.

Not sure of the standard procedures for your issue tracking but should this ticket be re-opened so others can see it as an open item?

@indutny indutny reopened this Mar 1, 2014
@indutny
Copy link
Member

indutny commented Mar 1, 2014

Done!

@seishun
Copy link

seishun commented Mar 26, 2014

@JavaScriptDude any progress on this? If not, I could take a stab at this myself, since it's relevant to my interests.

@Mithgol
Copy link

Mithgol commented Mar 26, 2014

@seishun Go for it! I hope it's not rude for me to point out that the calendar on the page of @JavaScriptDude's GitHub profile does not display any activity in this March, and that's a good reason to assume that the answer (about the progress) is negative (unless something was developed in secret and outside of GitHub).

@JavaScriptDude
Copy link
Author

@seishun please proceed.

I did start some initial design work offline but got stumped with direction and learning curve with C and was then distracted by other higher priority initiatives.

Below are some of my thoughts on this.

The challenge is to design it so that it is both fast and useful. To be useful, it should allow for generating a CRC from a stream from JavaScript side. To do this, we would need to use zlib's crc32.c::crc32() and ideally store the running checksum in C++ side rather than marshaling the running checksum back and forth from JavaScript side. The JavaScript side would only be pushing buffers of data through to crc32.c::crc32() from a stream on the JavaScript (userland) side.

There are use cases where I would like it to act against a file directly and it would be interesting to see how much faster it would be to implement a simple wrapper for minizip.c::getFileCrc(). This should be the fastest way to go and could be a good litmus test to see how much extra compute time is taken by the JavaScript and marshaling side of Node.js and expose opportunities for optimization in the stream based approach.

@JavaScriptDude
Copy link
Author

@seishun Was just following your thread and commits. Thanks for your work on this :)

@JavaScriptDude
Copy link
Author

For the record, I have added a new issue to open a discussion to Incorporate suite of hash utilities into node #2444

@Trott Trott closed this as completed Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants