-
Notifications
You must be signed in to change notification settings - Fork 50
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
Switch from adler32 to adler crate #83
Conversation
This seems reasonable, will include this with the 0.4 release. Will this change work as is with |
Yes, it should. |
As for performance differences, adler32 calculation is very fast in general, so maybe any difference is not significant enough to show in most comparisons. Maybe you could try compressing with custom settings and use only stored blocks, RLE or huffman-only to clearer show the difference. |
FWIW I did see some cases where adler32 calculated dominated the execution time: #76 Past me made the mistake of not including the testcases in the bug report 😞 |
Hmm, My PR for Since both |
I'm not adding unsafe code for fun, the PR you linked does actually improve performance considerably. The only reason it uses unsafe is because |
I'm not questioning whether the addition of unsafe code right for |
Ah, I see.
The only reason for using MIT/Apache is to ease integration into libstd. adler also allows usage under 0BSD, and that is normally my preferred license, but I was asked to use MIT/Apache to simplify the legal process. I agree this shouldn't be necessary, but oh well. |
I would be very wary of depending on anything using Alternatively, maybe you could add it as an optional feature or something. |
|
The adler crate is my clean-room reimplementation of Adler-32. The adler32 crate uses the Zlib license, while adler uses MIT/Apache-2.0 (or 0BSD), which makes integration into the standard library easier.
adler is also faster in the benchmarks I've made so far, although that isn't reflected in the miniz_oxide benchmarks (which in fact seemed to slightly regress on my system, but that might just be noise).