Skip to content
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

Add Zstd support as optional and compile-time low ratio report #25

Merged
merged 16 commits into from
Nov 3, 2023

Conversation

kkent030315
Copy link
Contributor

@kkent030315 kkent030315 commented Oct 18, 2023

Zstd (ZStandard) is a fast real-time compression algorithm.

I've added Zstd support in compression/decompression as optional. Users can select which one to use by specifying feature flags: zstd and deflate.

# Cargo.toml
[dependencies]
include-flate = { version = "...", default-features = false, features = ["zstd"] }
# or
include-flate = { version = "...", default-features = false, features = ["deflate"] } # this is same as default.

In summary, while DEFLATE is a venerable and widely adopted algorithm, Zstd provides significant advantages in speed and compression ratio, would make it a strong choice for crates or binaries that can support it.

Testing this PR

Since I've modified Cargo.toml to not use unnecessary dependency, multiple vectors of tests needed:

cargo test --all
cargo test --no-default-features --features deflate
cargo test --no-default-features --features zstd

@kkent030315 kkent030315 changed the title add zstd support as optional add Zstd support as optional Oct 18, 2023
@kkent030315 kkent030315 changed the title add Zstd support as optional Add Zstd support as optional Oct 18, 2023
@SOF3
Copy link
Owner

SOF3 commented Oct 19, 2023

in that case, consider using the CI pipeline for different features too?

src/lib.rs Outdated Show resolved Hide resolved
@kkent030315
Copy link
Contributor Author

kkent030315 commented Oct 21, 2023

Well, It's done and awaiting for your @SOF3 review.

Changelog

  • Added: Zstd support for compression.
    • I've added custom encoders, FlateEncoder and FlateDecoder, for each of the current and future compression methods that the encoders/decoders need to wrap. That means, it is just to write an wrapper for both FlateEncoder and FlateDecoder and adding some values into some enums are enough to add custom compression method in further. And of course, I've improved tests, so additional compression methods could be tested more precisely.
  • Added: include-flate-compress in compress folder in the root directory of this repository, for commonality.
  • Added: FlateArgs for extendable/descriptive TokenStream input.
  • Improved: Entire tests are improved for additional checks regarding compression.
  • Improved: test_util.rs has been reworked for reliable code.
  • Improved: Better error message in decompression failure in include_flate::decode function.

The compression features are selectable for each flate! macros, or DEFLATE by default:

flate!(pub static DATA: [u8] from "assets/009f.dat"); // default, DEFLATE
flate!(pub static DATA: [u8] from "assets/009f.dat" with zstd); // Use Zstd for this file spcifically
flate!(pub static DATA: [u8] from "assets/009f.dat" with deflate); // Explicitly use DEFLATE.

@kkent030315
Copy link
Contributor Author

kkent030315 commented Oct 21, 2023

in that case, consider using the CI pipeline for different features too?

As of now, I've pivoted to not use Cargo feature, CI changes shouldn't required. (Reverted in 957c2bf)

@kkent030315
Copy link
Contributor Author

kkent030315 commented Oct 21, 2023

I’ve also added compile-time warning for less compression binaries, so they have a visibility for compression ration for each binaries and can consider another compression method if it is not they're looking for.

Also for the users want to ignore this warning, I've added no-compression-warnings feature, which is disabled by default.

image

@kkent030315 kkent030315 changed the title Add Zstd support as optional Add Zstd support as optional and compile-time low ratio report Oct 21, 2023
codegen/src/lib.rs Outdated Show resolved Hide resolved
codegen/src/lib.rs Outdated Show resolved Hide resolved
codegen/src/lib.rs Outdated Show resolved Hide resolved
kkent030315 and others added 2 commits October 30, 2023 23:45
Co-authored-by: Jonathan Chan Kwan Yin <sofe2038@gmail.com>
@kkent030315
Copy link
Contributor Author

kkent030315 commented Oct 30, 2023

@SOF3 For the compression ratio warning, we should have discussion about:

  1. Should it be disabled by default? I think so.
  2. What do you think the best threshold is? Currently, it is set to 10%.

@SOF3
Copy link
Owner

SOF3 commented Oct 31, 2023

10% sounds right to me. Users should be aware that include_flate does not just increase executable size — it also increases the runtime memory by doubling the required storage and hence only reasonable to use when compression ratio is sufficiently high.

Might be useful to provide a deflate_if in a future (separate) feature too.

@kkent030315
Copy link
Contributor Author

Okay, l will work on fixing CI.

@kkent030315
Copy link
Contributor Author

It should be ok for now. please run the CI.

compress/Cargo.toml Outdated Show resolved Hide resolved
@SOF3 SOF3 merged commit 232fedb into SOF3:master Nov 3, 2023
19 checks passed
@SOF3
Copy link
Owner

SOF3 commented Nov 3, 2023

Thanks! Will do a release later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants