Provide ways to disable compression support?

[NobodyXu]

Upstream zstd provides build macro ZSTD_LIB_COMPRESSION to disable compression support, which will then significantly reduce the build time of the zstd-sys crate and reducing the size of the final binary.

However, adding a feature to zstd-sys will certainly be a breaking change even if it's enabled by default (could be opt-out via default-features = false) and it's likely other crates using this will just switch them on by default.

I'm thinking of a new environment flag ZSTD_SYS_DISABLE_COMPRESSION:

When enabled, it will enable ZSTD_LIB_COMPRESSION and change all the compression API to just return an error, signaling that is not supported.

But that sounds really hacky and I would rather like other solutions, like using gc-sections in #188 or using cross-languages LTO #191.

[NobodyXu]

Perhaps we should just let the users specify CFLAGS to disable compression support #192 ?

[nickbabcock]

I'm curious how large the size reduction is, as I'm testing out different compression algorithms in Wasm and as a point of comparison, the default brotli dictionary increases the Wasm size by 3.5x (from 400 KB to 1400 KB). I'm not expecting a similar drastic reduction, but I am curious what the impact would be, as any savings over the current 500 KB zstd wasm build may be greatly appreciated by the web community.

[NobodyXu]

@nickbabcock You can use CFLAG to do that for now.
Simply define ZSTD_LIB_COMPRESSION=0 and ZSTD_LIB_DICTBUILDER=0 as suggested by modular-build in zstd, you should be able to disable compression support, but linking might fail.

Alternatively, you can compile zstd yourself with these flags to try that out.

PR #189 should also reduce the size if you enable feature thin and once cc makes a new release containing rust-lang/cc-rs#757 , I will try re-enabling LTO again.

[nickbabcock]

Thanks for the tip. I may be a bit slow this morning, but I tried:

CFLAGS="-DZSTD_LIB_COMPRESSION=0" cargo build -vv --release

And I see lines like the following that make me think I'm communicating ZSTD_LIB_COMPRESSION correctly.

[zstd-sys 2.0.5+zstd.1.5.2] running: "cc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-DZSTD_LIB_COMPRESSION=0" "-I" "zstd/lib/" "-I" "zstd/lib/common" "-fvisibility=hidden" "-DZSTD_LIB_DEPRECATED=0" "-DXXH_PRIVATE_API=" "-DZSTDLIB_VISIBILITY=" "-DZSTDERRORLIB_VISIBILITY=" "-o" "/home/nick/projects/wasm-bench/target/release/build/zstd-sys-665d444381abd221/out/zstd/lib/compress/zstd_fast.o" "-c" "zstd/lib/compress/zstd_fast.c"

But I am not seeing any difference in output.

Maybe it's because the zstd-sys is always compiling files found in the zstd/lib/compress and including them in the output. I don't see ZSTD_LIB_COMPRESSION used in zstd outside the makefile.

[NobodyXu]

@nickbabcock You are absolutely right on this and that means #189 is actually changing the def to no-op.
Would open another PR to fix that.

[NobodyXu]

@nickbabcock In that case, I guess the only thing that couild help is cross language LTO #191

[nickbabcock]

@NobodyXu , my first thought would be to guard adding the files related to compression with a cargo feature (like zdict_builder):

zstd-rs/zstd-safe/zstd-sys/build.rs

Line 94 in bd75cb4

"zstd/lib/compress",

Which would emulate how the makefile interprets ZSTD_LIB_COMPRESSION=0

https://github.com/facebook/zstd/blob/88b7088d2e4686bb383e1691e28d02399ec1ff17/lib/Makefile#L32-L34

[gyscos]

It's just difficult to do in a backward compatible way.

[NobodyXu]

Yes, that's also my first thought.

Though TBH I'm worried about its adoption, since most libraries I use (tar, async-zip-rs) I use supports both building archive and extracting, so they likely will enable both at the same time.

[NobodyXu]

It's just difficult to do in a backward compatible way.

Yeah it definitely needs a breaking release to add new feature flags that disables existing functionalities.

[NobodyXu]

@gyscos Is it possible to have a new env/feature flag, when enabled, replace the zstd/lib/compress with an implementation that always returns an error for "not supported"?

I know this is also breaking change in terms of the API behavior, but at least it won't directly break the API hence don't need direct support in tar/async-zip-rs, etc, and as long as the compression API is not actually used, it will run just fine.

But that does sound like hacking together a solution instead of the proper one.

[gyscos]

Adding a "negative" feature (no-compress) to remove stuff is not technically a breaking change since no one could be using this feature at this point - it just goes against the additivity of features and could cause confusing build errors with conflicting sub-dependencies.

Also potentially breaking stuff at runtime sounds worse than a build error, especially if it can come from unusual causes like env variable or nested dependency.

It's best just to bump the version and bite the churn.

[NobodyXu]

Yes, I agree that an additive feature and build-time failure is better.

It's best just to bump the version and bite the churn.

I would prefer a new minor release after new version of cc released before having a new major release, which would likely quite some time before dependents adopt it.

[NobodyXu]

@nickbabcock cargo-bins/cargo-binstall#817 enabled cross-lang-fat-lto in cargo-binstall and is able to reduce >50% of the final binary size on linux.

We haven't figured out for MacOS and Windows yet.

I suppose the reduce of size mainly comes from removal of zstd compression related code since it is unused but there could be other stuff as well.

[nickbabcock]

Nice, I'm seeing a pure zstd decompressor in Wasm take up around 40-50 kB (gzipped), which agrees with the >50% figure you see too. Excellent job. I'm not sure how much more you think is left to shave off, but I think it's pretty close for one to be able to recommend zstd for the web without too many reservations.

For reference, I looked at the size of @oneidentity/zstd-js and the size of zstd-codec and they were 546 kB and 818 kB (both gzipped) respectively, so I think an implementation based on this could be enticing.

EDIT: @hpcc-js/wasm comes close with 117 kB

[NobodyXu]

Thanks!

lto is really the most effective way of shaving off unused bits, but I haven't figured out how to do this for MacOS and Windows yet...