A few days ago ZSTD v 1.5.6 was released[1], but we still have two years old ZSTD v1.5.2 in the base. Zstandard is developed under Facebook's eye so likely it's backdoor-prone. Delaying updates for such a long time is not optimal considering speed improvements and other benefits from the upgrade. 1. https://github.com/facebook/zstd/releases/tag/v1.5.6
Somewhat related: there is an even older version of zstd bundled with OpenZFS. The bundled copy (except the wrapper) should be removed and we should have only one copy in the source tree.
PR was submitted on April 1 and after this date, all suggestions but backdoor pronity are still legitimate.
(In reply to Xin LI from comment #1) ZFS intentionally keeps older versions of compressors (decompressors can be updated) to produce identical results when needed (L2ARC on system with uncompressed ARC is the most known).
“pronity” is not a word, and I don't think “prone” means what you think it means: https://www.merriam-webster.com/dictionary/prone
(In reply to Dag-Erling Smørgrav from comment #4) >“pronity” is not a word, and I don't think “prone” means what you think it >means: https://www.merriam-webster.com/dictionary/prone Maybe it's not a word but it's still a thing[1], anyway, it was partially April Fool's PR, so please exclude this sentence from the original bug report as a post-factum errata: "Zstandard is developed under Facebook's eye so likely it's backdoor-prone." (In reply to Alexander Motin from comment #3) >ZFS intentionally keeps older versions of compressors My report refers to sys/contrib/zstd. Bytes under sys/contrib/openzfs/module/zstd are derived from OpenZFS and most ZFS users know that they probably have a good reason for denying updates. 1. https://en.wiktionary.org/wiki/pronity
(In reply to Alexander Motin from comment #3) For future reference in case someone wanted to give this a shot, the concerns at OpenZFS[1] was that different versions of zstd may [2] generate different output for the same data. When compressed ARC is disabled (enabled by default), ARC would only have a copy of uncompressed data, but the MAC was calculated against compressed data, so arc_hdr_authenticate() would see a mismatch because it's now using a newer version of zstd to compress the data (for performance reasons, because re-compression is faster than reading the data back from disk). [1] https://github.com/openzfs/zfs/pull/11367#pullrequestreview-559645117 further explained in https://github.com/openzfs/zfs/pull/11367#issuecomment-753517958 [2] https://github.com/facebook/zstd/issues/999#issuecomment-359538229
(In reply to Dag-Erling Smørgrav from comment #4) > “pronity” is not a word I think it's a perfectly cromulent word, and at any rate if Shakespeare could make up his own words then so can Marek.
pronity: noun Proneness; propensity. It *is* a word. It's been in use since 1913. If you happen to have a copy of the Websters dictionary with a print/publishing date of 1913. You'll find it there. Do Note; it will need to be an American-English dictionary. Sorry. I couldn't resist. :) --Chris
Zstandard (zstd) version 1.5.7 was released last week, introducing significant performance enhancements and bug fixes[1] We have zstd 1.5.2 in the base system. It was released 3+ years ago and there are 1772 commits to dev since this release. 1.https://github.com/facebook/zstd/releases/tag/v1.5.7
Colin, you want to upgrade this before 15-RELEASE?
The Zstandard compressor included in the base system still appears to be quite efficient. Using the same compression level, when part of an ISO file from an old FreeBSD release was used as a test sample, the base system’s version (1.5.2) compressed the file faster than the one from /usr/local (1.5.7), though at the cost of a slightly reduced compression ratio. The tests were repeated several times and produced consistent results. $ /usr/bin/time /usr/local/bin/zstdmt -f --long -19 plik200m.nor plik200m.nor : 25.37% ( 200 MiB => 50.7 MiB, plik200m.nor.zst) 33,50 real 85,75 user 0,69 sys $ ll plik200m.nor.zst -rw-r--r-- 1 mzar wheel 53199624 7 paź 10:40 plik200m.nor.zst $ /usr/bin/time /usr/bin/zstdmt -f --long -19 plik200m.nor plik200m.nor : 25.79% ( 200 MiB => 51.6 MiB, plik200m.nor.zst) 28,80 real 77,36 user 0,64 sys $ ll plik200m.nor.zst -rw-r--r-- 1 mzar wheel 54075469 7 paź 10:40 plik200m.nor.zst
If no one is actively working on it (I see that the newest vendor branch still only has 1.5.2 in it), I can take up the task. I would also like to get zstd in base to the latest version. Let me know if I'm conflicting with anyone else's WIP and I'll hand it over :)
I made a patch for 1.5.5 long time ago (https://reviews.freebsd.org/D43240). If no one is taking this, I'm fine with Siva doing the work since he has a commit bit.
I've imported zstd 1.5.7 to -CURRENT and done some diff reduction against upstream. ZFS bundled copy untouched for now. Ideally, we want the ZFS bundled copy to be removed, but upstream (OpenZFS) have concerns with output stability (same data would be compressed in different ways, making it problematic for persistent L2ARC; although compressed data can be decompressed without problem)
Sounds good. By the way, this happens to be the second time in the past few days you took over a WIP patchset of mine, please notify me next time before doing that so I can stop working on it and focus on something else. I would be happy to delegate the work. I will close my stack of reviews here https://reviews.freebsd.org/D55836, but I still think the XList narrowing will be worth it.
(In reply to Siva Mahadevan from comment #15) Hi, sorry I should have searched earlier. I'll take a look at your change and integrate it.