Feature request: compile build.rs scripts in release mode even when doing a debug build #2424

TyOverby · 2016-03-01T17:42:14Z

Right now, cargo build compiles build.rs in debug mode, while cargo build --release compiles build.rs in release mode. However build.rs scripts can potentially be very computationally intensive, and I would like the option to run my build.rs script in release mode even when doing a debug compile of the rest of my crate.

As a motivating example, nikomatsakis/lalrpop is a parser generator that does source generation during the custom build step. Just the build.rs step takes 6m30s when compiling in debug, and 33s when compiling in release for a complex grammar.

The text was updated successfully, but these errors were encountered:

alexcrichton · 2016-03-01T18:46:03Z

cc @nikomatsakis

I've certainly wanted this in the past as well. The tricky parts I can think of are:

We need some sort of "nice syntax" for configuring this. We may not want to use [profile] as this may want to be a per-package instead of a per-application setting.
It's not just the build script that needs to be optimized here, but also the dependencies. Sometimes the dependencies can be re-used with the original application, so we need to rationalize that as well.

nikomatsakis · 2016-03-02T21:04:55Z

On Tue, Mar 01, 2016 at 10:46:06AM -0800, Alex Crichton wrote:

cc @nikomatsakis

I've certainly wanted this in the past as well. The tricky parts I can think of are:

We need some sort of "nice syntax" for configuring this. We may not want to use [profile] as this may want to be a per-package instead of a per-application setting.

It's not just the build script that needs to be optimized here, but also the dependencies. Sometimes the dependencies can be re-used with the original application, so we need to rationalize that as well.

Yeah, I think the main reason I'd not yet opened an issue requesting
this was not having an answer to these questions. It's also unclear to
me what the defaults ought to be: I'd EXPECT that build.rs typically
wants to be "release by default", but I don't know.

I could also imagine this not being something that one configures, but
rather part of the cargo build command. e.g., perhaps build.rs
defaults always to a release build, but you can run cargo build --preprocessor debug or something when you actually want to debug
your build.rs script.

sfackler · 2016-03-03T17:17:02Z

The story for lalrpop is kind of interesting here. It's fairly expensive to compile, particularly in release mode (~minutes). For small grammars, total build script time is dominated by compiling lalrpop-snap, in which case you'd probably want a release build, but for larger grammars, the situation inverts and you really need an optimized version of lalrpop.

TyOverby · 2016-03-03T17:45:36Z

cargo build --preprocessor debug

I'd rather not have this. In the scenario where I have

user application depends on my library which uses lalrpop

I'd like for my library to use a release preprocessor always because I know that the trade off is worth it for my library. I don't want to have to tell people "oh, and if you depend on my library, you should really run cargo build --preprocessor=release.

nikomatsakis · 2016-03-09T23:16:13Z

@TyOverby

I don't want to have to tell people "oh, and if you depend on my library, you should really run cargo build --preprocessor=release.

My proposal was that release would be the default. You would use debug if you knew you wanted to debug the build.rs script (which would presumably not be the common case). But the point is this is a decision that only the end-user is really in a position to make, not any particular dependency.

nikomatsakis · 2016-03-09T23:17:03Z

@sfackler

The story for lalrpop is kind of interesting here. It's fairly expensive to compile, particularly in release mode (~minutes).

I'd much rather always have release mode. Sure, the first compile is slow, but after that it doesn't change, so who cares? In general I feel like the first time I compile anything in cargo (or virtually any build system) it is slow, because it has to run off and grab a bunch of deps.

TyOverby · 2016-03-09T23:44:10Z

@nikomatsakis Ok yeah, I agree with this. Running build scripts in release sounds like a good default.

alexcrichton · 2016-03-10T00:57:15Z

There's a few concerns I'd have about having a default release for build dependencies:

If we still have the option to switch to debug mode, we still have the problem of inventing a syntax for such an operation. Arguably the same syntax at that point could be used for "this should be optimized"
The problem of dependencies is sort of two-fold. A crate can be used both for a build script and for the final artifact generation. In this case do we optimize it or do we not? Unfortunately either choice we make can be the wrong choice depending on how you look at it.

Nemikolh · 2016-03-10T08:20:24Z

Considering cross-compilation with cargo, isn't already a thing that build dependencies are compiled in a very different way than crate ones?

Having this scenario in mind, the fact that build script are compiled in release mode with the triplet target of the host is a default that make more sense than being similar to the crate compilation profile.

In case a crate is used for both dependencies, for a cross-compilation use case, you would have to compile it twice anyway so it doesn't seems crazy.

nikomatsakis · 2016-03-10T14:34:40Z

If we still have the option to switch to debug mode, we still have the
problem of inventing a syntax for such an operation. Arguably the same
syntax at that point could be used for "this should be optimized"

I contend that this is the wrong default -- I think the number of
people writing custom logic in their build.rs that they plan to debug
pales in comparison to the numbe of people injecting logic written by
others.

That said, it may be that the right fix here is to stop using
build.rs altogether. That is, maybe what we want is a way to declare a
"preprocess-dependency" that compiles and executes the crate as a
preprocessing step automatically, without the need for a user to write
their own build.rs script. We could then make the default for that
kind of dependency be release, and keep build.rs as it is today.

This also makes using LALRPOP, syntex, and other similar preprocessors
more ergonomic: you only have to add one line to Cargo.toml, instead
of following several distinct steps.

The problem of dependencies is sort of two-fold. A crate can be used both
for a build script and for the final artifact generation. In this case do
we optimize it or do we not? Unfortunately either choice we make can be
the wrong choice depending on how you look at it.

My assumption was that the "build-dependencies" are distinct from the
"main" dependencies. That is, build.rs is a distinct target, so it's
not clear why it has (or should) to use the same build of the crate?

(Feels a lot like workspaces. :)

alexcrichton · 2016-03-11T06:36:57Z

Yeah this may indeed be a different class of dependencies (preprocessors), but those also sound a lot like plugins...

When you're compiling for the host target (e.g. you didn't pass --target) then if the application depends on a a crate which is also depended on by a build script, we'll only compile that crate once (to avoid unnecessary compilations). I guess we could just decide to compile it twice though

nikomatsakis · 2016-03-11T18:49:43Z

Yeah this may indeed be a different class of dependencies (preprocessors), but those also sound a lot like plugins...

Is this surprising? I guess you're saying that adding something more "first class" than build.rs doesn't feel right to you, given that we will (eventually) have some kind of plugin system? I could understand that. I think it may well be that we continue to want "preprocessors" of the current form, which generate "all new" source rather than being triggered by some macro use embedded in a .rs file, but I'm not entirely sure about that. It's not clear why that would necessarily be better than some kind of #![invoke_preprocessor] attribute (or maybe even something that triggers automatically when given an extern crate).

alexcrichton · 2016-03-11T21:31:25Z

Hm yeah I guess they're different enough to warrant different systems. Seems plausible as becoming a thing!

jdub · 2016-10-08T03:40:54Z

Is the idea of a host profile for build dependencies ridiculously naive? The defaults could be same as the release profile, but you could change it however you like:

[profile.host]
opt-level = 1
debug = true

I would love to not rebuild all the host build dependencies when using other profiles.

alexcrichton · 2016-10-10T15:26:11Z

@jdub that's plausible, yeah! The only question would be what to do for a crate which needs to be compiled by the host profile and the target profile. My guess would be the target takes precedent, but in general doesn't matter too too much.

This commit includes two significant changes: 1. Patterns are bundled in their plain TeX form, and serialized at build time; this is preliminary to user-chosen normalization. 2. Patterns are generated by applying a binary serialization strategy directly to pattern and exception *tries*. Change 1 entails a significant increase in the time of first-build, partly owing to the limitation that `build.rs` cannot be compiled in release mode for debug builds. (Refer to [cargo #2424](rust-lang/cargo#2424).) The alternative to build-time serialization would be to package patterns in every normalization form, and then either bundle them as a whole, or isolate each as an optional data-only dependency. The former would unacceptably inflate package size (to roughly 40MB, as of writing); the latter I perceive as a misuse of Cargo crates. Change 2 offers modest improvements in deserialization times, at the expense of size: - Deserializing the interned trie is about ~20–25% faster than deserializing sequential JSON data and building the trie at runtime. - The interned data is approximately 25% larger. This may improve in the future, e.g. by compressing shared prefixes or suffixes.

tapeinosyne · 2016-11-18T20:26:54Z

I should like to offer another example: the hyphenation crate normalizes a corpus of UTF-8 text at build time, an operation that takes an order of magnitude longer in debug mode. This was cause of some perplexity upon first encounter.

Dependencies notwithstanding, for small libraries such as hyphenation the problem is easily remedied, but it would be nice to have a dedicated option.

joshtriplett · 2017-01-27T21:17:25Z

For another example, see rust-phf/rust-phf#104 , which provides an example where PHF's perfect hash generation takes 685 seconds when compiled for debug, or ~16.8 seconds when compiled for release.

alexcrichton · 2017-01-27T21:35:42Z

FWIW we don't really need any more examples of where this is helpful, it's clear that there's a well-motivated use case.

What we need now is a solution!

lilith · 2017-03-10T22:16:55Z

There are also cases where build.rs depends on crates that take minutes to compile in release mode.

Some way to turn the optimization level down a bit would greatly speed up the build. I've also found that opt-level=1 can sometimes offer the best of both worlds.

aturon · 2018-01-22T20:36:04Z

Possibly related to rust-lang/rfcs#2282.

Boscop · 2018-04-04T00:35:35Z

Any update on this? :)

Profile Overrides (RFC #2282 Part 1) Profile Overrides (RFC #2282 Part 1) WIP: Putting this up before I dig into writing tests, but should be mostly complete. I also have a variety of questions below. This implements the ability to override profiles for dependencies and build scripts. This includes a general rework of how profiles work internally. Closes #5298. Profile overrides are available with `profile-overrides` set in `cargo-features` in the manifest. Part 2 is to implement profiles in config files (to be in a separate PR). General overview of changes: - `Profiles` moved to `core/profiles.rs`. All profile selection is centralized there. - Removed Profile flags `test`, `doc`, `run_custom_build`, and `check`. - Removed `Profile` from `Unit` and replaced it with two enums: `CompileMode` and `ProfileFor`. This is the minimum information needed to compute profiles at a later stage. - Also removed `rustc_args`/`rustdoc_args` from `Profile` and place them in `Context`. This is currently not very elegant because it is a special case, but it works. An alternate solution I considered was to leave them in the `Profile` and add a special uber-override layer. Let me know if you think it should change. - Did some general cleanup in `generate_targets`. ## Misc Fixes - `cargo check` now honors the `--release` flag. Fixes #5218. - `cargo build --test` will set `panic` correctly for dependences. Fixes #5369. - `cargo check --tests` will no longer include bins twice (once as a normal check, once as a `--test` check). It only does `--test` check now. - Similarly, `cargo check --test name` no longer implicitly checks bins. - Examples are no longer considered a "test". (See #5397). Consequences: - `cargo test` will continue to build examples as a regular build (no change). - `cargo test --tests` will no longer build examples at all. - `cargo test --all-targets` will no longer build examples as tests, but instead build them as a regular build (now matches `cargo test` behavior). - `cargo check --all-targets` will no longer check examples twice (once as normal, once as `--test`). It now only checks it once as a normal target. ## Questions - Thumbs up/down on the general approach? - The method to detect if a package is a member of a workspace should probably be redone. I'm uncertain of the best approach. Maybe `Workspace.members` could be a set? - `Hash` and `PartialEq` are implemented manually for `Profile` only to avoid matching on the `name` field. The `name` field is only there for debug purposes. Is it worth it to keep `name`? Maybe useful for future use (like #4140)? - I'm unhappy with the `Finished` line summary that displays `[unoptimized + debuginfo]`. It doesn't actually show what was compiled. Currently it just picks the base "dev" or "release" profile. I'm not sure what a good solution is (to be accurate it would need to potentially display a list of different options). Is it ok? (See also #4140 for the wrong profile name being printed.) - Build-dependencies use different profiles based on whether or not `--release` flag is given. This means that if you want build-dependencies to always use a specific set of settings, you have to specify both `[profile.dev.build_override]` and `[profile.release.build_override]`. Is that reasonable (for now)? I've noticed some issues (like #1774, #2234, #2424) discussing having more control over how build-dependencies are handled. - `build --bench xxx` or `--benches` builds dependencies with dev profile, which may be surprising. `--release` does the correct thing. Perhaps print a warning when using `cargo build` that builds benchmark deps in dev mode? - Should it warn/error if you have an override for a package that does not exist? - Should it warn/error if you attempt to set `panic` on the `test` or `bench` profile? ## TODO - I have a long list of tests to add. - Address a few "TODO" comments left behind.

alercah · 2019-01-08T16:02:00Z

I believe RFC 2822 solves this feature request and this can be closed as a dupe of rust-lang/rust#48683.

alexcrichton · 2019-01-08T17:45:11Z

Indeed!

TyOverby mentioned this issue Mar 1, 2016

larger grammars generate a LOT of code lalrpop/lalrpop#65

Closed

alexcrichton mentioned this issue Mar 22, 2016

Allow dependency to forcibly override profile settings #2506

Closed

nikomatsakis mentioned this issue Aug 3, 2016

Improve compilation times of gluon_parser gluon-lang/gluon#69

Closed

joshtriplett mentioned this issue Jan 27, 2017

Generation performance doesn't match README rust-phf/rust-phf#104

Closed

alexcrichton mentioned this issue Apr 7, 2017

build.rs scripts are locked to host crate debug/release settings #3903

Closed

carols10cents added A-build-scripts Area: build.rs scripts A-profiles Area: profiles C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Sep 25, 2017

aturon added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Jan 22, 2018

This was referenced Mar 23, 2018

revisit how snapshot works lalrpop/lalrpop#348

Closed

make it easy for build.rs to use lalrpop executable lalrpop/lalrpop#349

Open

ehuss mentioned this issue Apr 18, 2018

Profile Overrides (RFC #2282 Part 1) #5384

Merged

alexcrichton closed this as completed Jan 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature request: compile build.rs scripts in release mode even when doing a debug build #2424

Feature request: compile build.rs scripts in release mode even when doing a debug build #2424

TyOverby commented Mar 1, 2016

alexcrichton commented Mar 1, 2016

nikomatsakis commented Mar 2, 2016

sfackler commented Mar 3, 2016

TyOverby commented Mar 3, 2016

nikomatsakis commented Mar 9, 2016

nikomatsakis commented Mar 9, 2016

TyOverby commented Mar 9, 2016

alexcrichton commented Mar 10, 2016

Nemikolh commented Mar 10, 2016

nikomatsakis commented Mar 10, 2016

alexcrichton commented Mar 11, 2016

nikomatsakis commented Mar 11, 2016

alexcrichton commented Mar 11, 2016

jdub commented Oct 8, 2016

alexcrichton commented Oct 10, 2016

tapeinosyne commented Nov 18, 2016

joshtriplett commented Jan 27, 2017

alexcrichton commented Jan 27, 2017

lilith commented Mar 10, 2017

aturon commented Jan 22, 2018

Boscop commented Apr 4, 2018

alercah commented Jan 8, 2019

alexcrichton commented Jan 8, 2019

Feature request: compile build.rs scripts in release mode even when doing a debug build #2424

Feature request: compile build.rs scripts in release mode even when doing a debug build #2424

Comments

TyOverby commented Mar 1, 2016

alexcrichton commented Mar 1, 2016

nikomatsakis commented Mar 2, 2016

sfackler commented Mar 3, 2016

TyOverby commented Mar 3, 2016

nikomatsakis commented Mar 9, 2016

nikomatsakis commented Mar 9, 2016

TyOverby commented Mar 9, 2016

alexcrichton commented Mar 10, 2016

Nemikolh commented Mar 10, 2016

nikomatsakis commented Mar 10, 2016

alexcrichton commented Mar 11, 2016

nikomatsakis commented Mar 11, 2016

alexcrichton commented Mar 11, 2016

jdub commented Oct 8, 2016

alexcrichton commented Oct 10, 2016

tapeinosyne commented Nov 18, 2016

joshtriplett commented Jan 27, 2017

alexcrichton commented Jan 27, 2017

lilith commented Mar 10, 2017

aturon commented Jan 22, 2018

Boscop commented Apr 4, 2018

alercah commented Jan 8, 2019

alexcrichton commented Jan 8, 2019