image-rs / image

Encoding and decoding images in Rust
Apache License 2.0
4.93k stars 610 forks source link

Proposal: more aggressive feature flags #2244

Open LGFae opened 4 months ago

LGFae commented 4 months ago

Hello. I've used image extensively in numerous projects. In most of them, I only needed to decode an image into a series of pixels. This means I only needed the decode functionality from the image crate. Everything else (most noticeably, encoding and imageops), was superfluous.

I would like to be able to select the precise functionality (decode and/or encode) I need from image. This would entail creating a lot more features flags than image currently has.

The main advantage this would bring is a decrease in dependencies, and sub-sequentially compilation times. image is often the most time-consuming crate to compile in my projects, though I can sometimes work around that by only enabling a few formats I care about.

The main disadvantage is that it would be a lot of work, and it might hurt readability/maintainability with all these #[cfg] annotations thrown everywhere.

Draft

If this is accepted, I would suggesting starting as simple as possible. Just annotating the top level image crate with encode and decode features for each image format. It would be a lot of work, but (from what I can tell) mostly simple, grunt-like stuff.

Of course, ideally, we would like to have these flags all the way upstream (so, for example, the gif crate should also have an encode and decode flag). But I believe we can do that in small steps after we've patched image itself.

Furthermore, I believe an imageops feature flags could also be created, though that may be rendered useless if #2238 is addressed.

Finally, note this would be a breaking change downstream, since anyone who's using default-features=false would have to rework their Cargo.toml. Overall, I believe it would be a simple fix, and still worth it.

fintelia commented 4 months ago

I took a look at the build timings for this crate, and one thing that stood out was that about 3/4 of total compilation time was consumed by only a few features:

LGFae commented 4 months ago

Indeed, just removing avif and rayon gave me roughly 20s faster --release compile times, and reduced my dependencies in Cargo.lock by more than 400 lines.

WorikWork commented 4 months ago

The main advantage this would bring is a decrease in dependencies, and sub-sequentially compilation times

For me the advantage stops at "decrease in dependencies"

I am looking to create a software that I deploy on my public facing (very valuable to me) production server. I want as few dependencies as I can get.

I came here browsing crates that can help me (scaling Jpeg/PNG and or stitching Jpeg/PNG) and a very important criterion is the quantity of dependencies.

OT It is a weakness in the Rust ecosystem that it is generally so liberal with pulling in arbitrary crates. It is a nightmare from my perspective as I do not want to expand the attack surface on my server more than I absolutely have to.

Love your work - I fully understand if my requirements do not mesh with yours.

fintelia commented 4 months ago

This split likely wouldn't decrease the number of dependencies by very much. The reason is that generally we use the same crate for both encoding and decoding. And even within those crates, a lot of the dependencies will be shared. Compression libraries generally provide both compression and decompression code paths, checksums are the same whether you're writing or validating, etc.

I totally understand where you're coming from in wanting to keep distinct dependencies to an absolute minimum. Given the extremely broad scope of the crate, that's something we unfortunately cannot fully cater towards. We do try to limit to reputable crates and a fair number of dependent crates are actually part of the same image-rs org (and thus don't add anyone new to trust). But that's only one factor to consider. We also care about functionality, performance, robustness, etc.