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

Improve build times. #311

Closed
ghost opened this issue Mar 1, 2015 · 13 comments · Fixed by #330
Closed

Improve build times. #311

ghost opened this issue Mar 1, 2015 · 13 comments · Fixed by #330

Comments

@ghost
Copy link

ghost commented Mar 1, 2015

I was wondering if there was any ideas on how the build times could be improved for this crate. It currently takes about a 1minute to compiler for a none release build.

Some ideas:

  • Move the image codecs into sub crates
  • Add feature flags to disable codecs that are not used
@TyOverby
Copy link
Contributor

TyOverby commented Mar 1, 2015

I really like this idea. You could have the default feature include ever codec, but if I have a crate that only uses PNG and JPEG, then I could just select those.

@nwin
Copy link
Contributor

nwin commented Mar 1, 2015

Good idea. I woud prefer feature flags. I don’t want to maintain 6 crates on crates.io

@bvssvni
Copy link
Contributor

bvssvni commented Mar 4, 2015

The problem with feature flag is they only allow one feature set at a time, which must be named. Example https://github.com/bvssvni/start_piston/blob/master/Cargo.toml#L90-L98

@bvssvni
Copy link
Contributor

bvssvni commented Mar 4, 2015

We can use a "only_jpeg_png" feature flag and leave out the other codecs when this is enabled.

@tomaka
Copy link
Contributor

tomaka commented Mar 4, 2015

No, please.

Let's say you have library A which depends on image without only_jpeg_png, and library B which depends on image with only_jpeg_png.

If you try to use both A and B at the same time, Cargo will compile image with only_jpeg_png as it assumes that activating features can only be beneficial. And then library A won't work.

@bvssvni
Copy link
Contributor

bvssvni commented Mar 4, 2015

We could move codecs into smaller crates, and make them depend on this library.

In the long term this is a better solution. We should figure out a way to make working with crates.io a less burden on the maintainers.

@ghost
Copy link
Author

ghost commented Mar 4, 2015

When I tried to at least add a feature flag I ran into problems with dynimage.rs. You can use a feature flag to remove enums, but table entries is not possible.

@bvssvni
Copy link
Contributor

bvssvni commented Mar 4, 2015

Also, see rust-lang/cargo#1169 If Cargo get this, then working with multiple crates could be easier.

@bvssvni
Copy link
Contributor

bvssvni commented Mar 4, 2015

It seems that multiple features are supported, there is a bug in the command line, but works in Cargo.toml. I opened rust-lang/cargo#1378

@hauleth
Copy link
Contributor

hauleth commented Mar 9, 2015

IMHO the best way to improve compilation times (and improve implementation of new formats) is to split format encoders and decoders to separate crates (possibly in the same repo) and compile them only when needed. Just add "meta crate" that will contain common traits.

@nwin
Copy link
Contributor

nwin commented Mar 9, 2015

The build times really degraded. Since we did not change much it must be partially a rustc thing…

@nwin
Copy link
Contributor

nwin commented Mar 9, 2015

A month ago a release build took 1 minute (30s for image crate) on my computer. Now it is 2. From this two minutes image takes 1:30. That means the build times of image trippelt.

huonw added a commit to huonw/image that referenced this issue Mar 16, 2015
The default continues to have all codecs activated, but a user can
opt-in to only supporting specific ones, via features, e.g. depending on
`image`, while only supporting JPEG and GIF:

    [dependencies.image]
    version = "0.2"
    default-features = false
    features = ["jpeg", "gif"]

On my computer `cargo build` takes ~95s, while `cargo build --features
jpeg --no-default-features` (only JPEG) takes ~28s.

This patch also updates the travis file to also compile the library with
each feature, to ensure they continue to compile. These codec features
are all independent, so testing each one individually is sufficient.

Closes image-rs#311.
huonw added a commit to huonw/image that referenced this issue Mar 16, 2015
The default continues to have all codecs activated, but a user can
opt-in to only supporting specific ones, via features, e.g. depending on
`image`, while only supporting JPEG and GIF:

    [dependencies.image]
    version = "0.2"
    default-features = false
    features = ["jpeg", "gif"]

On my computer `cargo build` takes ~95s, while `cargo build --features
jpeg --no-default-features` (only JPEG) takes ~28s.

This patch also updates the travis file to also compile the library with
each feature, to ensure they continue to compile. These codec features
are all independent, so testing each one individually is sufficient.

Closes image-rs#311.
huonw added a commit to huonw/image that referenced this issue Mar 16, 2015
The default continues to have all codecs activated, but a user can
opt-in to only supporting specific ones, via features, e.g. depending on
`image`, while only supporting JPEG and GIF:

    [dependencies.image]
    version = "0.2"
    default-features = false
    features = ["jpeg", "gif"]

On my computer `cargo build` takes ~95s, while `cargo build --features
jpeg --no-default-features` (only JPEG) takes ~28s.

This patch also updates the travis file to also compile the library with
each feature, to ensure they continue to compile. These codec features
are all independent, so testing each one individually is sufficient.

Closes image-rs#311.
@nwin nwin closed this as completed in #330 Mar 16, 2015
@nwin
Copy link
Contributor

nwin commented May 3, 2015

Ok, I think it wasn’t our fault that the compile times exploded: rust-lang/rust#25069

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

Successfully merging a pull request may close this issue.

5 participants