Nullus157 / async-compression

Adaptors between compression crates and Rust's async IO types
https://docs.rs/async-compression
Apache License 2.0
410 stars 81 forks source link

Gzip compression level 0 #313

Closed eth3lbert closed 43 minutes ago

eth3lbert commented 8 hours ago

Currently, the compression level for Gzip is clamped between 1 (fast) and 9 (best). However, level 0 is supported by the flate2 crate and Python's gzip module. While changing the level range could be a breaking change, are there any alternative approaches to address this issue? For example, introducing a none level might be a viable solution?

NobodyXu commented 6 hours ago

I think that you can useLevel::Precise(0)?

The problem though is that we clamp it based on min/max supported by flate2

https://docs.rs/async-compression/latest/src/async_compression/lib.rs.html#216

And flate2 fastest uses compression level 1

https://docs.rs/flate2/latest/src/flate2/lib.rs.html#218-220

eth3lbert commented 6 hours ago

I think that you can useLevel::Precise(0)?

The problem though is that we clamp it based on min/max supported by flate2

https://docs.rs/async-compression/latest/src/async_compression/lib.rs.html#216

If I understand correctly, the Level::Precise(0) after clamp should result in flate2::Compression::new(1), but I'd prefer it to be the result of flate2::Compression::new(0).

The use case is that we have some test cases that explicitly disable compression, such as https://github.com/rust-lang/crates.io/blob/000197227fb963d91612d2bcdf360bea44b7bd34/src/tests/krate/publish/max_size.rs#L40:L41. However, while I tried to implement this using async-compression, I was unsuccessful 😢

NobodyXu commented 6 hours ago

Yes and that is because flate2::Compression::fast() uses level 1.

I believe it's better to update flate2 to use level 0?

eth3lbert commented 5 hours ago

Yeah, it would be great the Precise part could be changed to something similar to:

            Self::Precise(quality) => flate2::Compression::new(
                quality
                    .try_into()
                    .unwrap_or(0)
                    .clamp(flate2::Compression::none(), best.level()),
            ),
NobodyXu commented 5 hours ago

Hmmm I think this makes sense to make.

I would accept a PR for this.