Upgrade zstd to ^0.9

[Dandandan]

Upgrade zstd a couple of versions

[jorgecarleitao]

Do we require the latest version of zstd? My understanding is that, for libraries, we should place the minimum version and maximum version we know to accept, and let cargo/consumers handle the resolution to the latest.

E.g. this is backward incompatible because it requires dependents to now use at least 0.9?

Since we seem to support 0.6 up to 0.9, wouldn't it make sense to use >= 0.6, < 0.10 instead?

[Dandandan]

facebook/zstd#731

Zstd (the format) is stable. The changes in the encoder/decoder are mainly performance improvements and fixes

[jorgecarleitao]

(I meant zstd the crate).

If someone writes ^0.6 in their crate to match ours to reduce the binary size and we now change it to ^0.9, that causes two zstd to be shipped in the binary (afaik cargo). By bracketing dependencies, we allow downstreams to depend on whatever version they want, and as long as it is compatible with our bracket, a single zstd is compiled and shipped (no?)

[Dandandan]

That's true. We don't expose the API of the zstd crate AFAIK, so a user is still free to choose an older version of zstd, it will appear twice with different versions in their dependency tree. It depends on how the other library is using zstd

However, I am not sure whether it's a good idea to also keep depending on an older version here. 0. releases are considered backwards incompatible, so you should keep testing the older release versions (in CI) in order to not break the compatibility. Also (large) performance improvements will not apply because the other library depends an older version.

[Dandandan]

https://github.com/facebook/zstd/releases

Performance differences of new versions are quite large btw, compression speed and decompression speed improved significantly in the latest release.

[codecov-commenter]

Codecov Report

Merging #31 (9c490d5) into main (20a1fb9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #31   +/-   ##
=======================================
  Coverage   68.92%   68.92%           
=======================================
  Files          61       61           
  Lines        3321     3321           
=======================================
  Hits         2289     2289           
  Misses       1032     1032           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20a1fb9...9c490d5. Read the comment docs.

[jorgecarleitao]

Thanks a lot, @Dandandan for the explanation.

I left a thread on Rust users to get some opinions on this, but it is not blocking; like you said, the perf improvements are massive.

[jorgecarleitao]

just for reference what you proposed is indeed the cargo way: https://users.rust-lang.org/t/how-to-declare-dependencies-in-cargo-without-cascading-effects/62958/5?u=jorgecarleitao . 👍