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 release and develop flags to configure #17665

Closed
nrc opened this issue Sep 30, 2014 · 18 comments
Closed

Add release and develop flags to configure #17665

nrc opened this issue Sep 30, 2014 · 18 comments

Comments

@nrc
Copy link
Member

nrc commented Sep 30, 2014

The set of flags for building rustc is getting bigger and the configuration for a release build versus something useful for developing is diverging.

We should add two flags to make --release and --develop which are umbrella flags of sensible sets of flags. A release build should be make the optimal compiler and develop should balance compile time, debug info, and optimisation.

--develop should not be no-debug (i.e., include logging), should build jemalloc in debug mode, have all asserts on in llvm, should be an O2 build (I think, cc @epdtry), and should use parallel builds with 4 threads.

--release should be no-debug (see #17496), should build release versions of jemalloc and llvm, should be O3, and should not use parallel builds.

Setting any individual flags should override these settings.

Open questions: should --develop include debuginfo by default? (Does this even work at the moment, I know there have been bugs recently. cc @michaelwoerister)

Are there other flags that should be set?

Can we do this as proposed? I assume configure can set different defaults for make. (cc @alexcrichton)

Which should be the default? (Or should the user be forced to specify one or the other). I think @thestinger has made a good argument elsewhere that --release should be the default since it is more unfortunate if a packager makes a mistake than a developer.

@brson
Copy link
Contributor

brson commented Sep 30, 2014

I think a lot of systems (incl LLVM) call these options 'release' and 'debug' (not 'development').

The naming of the current --debug is misleading since it does not give you debuginfo.

Can we fold the current --debug and --disable-optimize flag into one of these, or vice versa, or do we need all these things to be togglable independently (i.e. are 'release' and 'development' mode umbrella options that just turn on and off others)?

--release-channel is a related flag that currently only controls the versioning and package naming. Is that going to tie into this or be independent?

The naming of --release and --development flags do not fit into the current scheme used by configure where everything is either --enable-foo or --disable-foo.

Under the current configure script how do you even turn on -g? I don't see it.

Even if we turn off debug logging and asserts for release builds we should keep it on for snapshots. Let's please not land any changes here without updating the automation.

@brson
Copy link
Contributor

brson commented Sep 30, 2014

Data point: LLVM (and I assume transitively clang since clang is part of the LLVM build) builds 'Release+Asserts' by default (not 'Release') which is more or less equivalent to what we do now.

@thestinger
Copy link
Contributor

LLVM uses cmake which has a standard way of doing release vs. development builds. Using -DCMAKE_BUILD_TYPE=Release will be standard in any package dealing with it.

@alexcrichton
Copy link
Member

I would vote for this state of affairs:

  • These options can be disable/enabled via configure:
    • debug - on == --cfg ndebug, off = -g, default on. This flag applies to bundled C libs as well
    • optimize - on == -O, off == nothing, default on. This flag applies to bundled C libs as well.
    • parallel-codegen - on == -C codegen-units=4, off == nothing, default on

I believe that the vast majority of builders of rust will want a very fast build and will not be distributing the binaries. It's much easier to recommend that one package maintainer turns on one or two flags as opposed to recommending that all users who download rust pass some set of many flags.

Additionally, given the defaults above, our automation would do this:

  • nightlies/releases - --disable-debug --disable-parallel-codegen
  • snapshots - --disable-parallel-codegen

The idea here is that the output should be as fast as possible so parallel codegen is disabled. On nightlies/releases we can removing debuginfo/debug logging. Note that whatever flags we use on nightlies/releases are also the flags that we would recommend to packagers.

I personally don't think that --release and --develop (to the configure script hypothetically) would be pulling their weight today, so I'm not sure if it's worth it to add them.

If we pursue this strategy, then the changes from today would be:

  • Add a parallel-codegen option.
  • Modify nightly builders to pass --disable-debug.

@alexcrichton
Copy link
Member

I also think that -g is stable enough now to turn on by default. Cargo turns it on by default for everyone and I haven't heard of a segfault in awhile, and I know @michaelwoerister has been working tirelessly to weed out all the really nasty bugs, and his work definitely shows!

@thestinger
Copy link
Contributor

Using -g slows down compile-time quite a bit, as does having debug_assert! and debug! logging enabled. I don't think a 15-20% compile-time hit for rustc on Windows from debug logging by default is good for anyone, whether they are a developer or a user building it for themselves.

Developers working on Rust itself can be expected to pass non-default flags. You can't really expect packagers for countless distributions to know which non-standard flags they should pass. It doesn't matter if you document it somewhere obscure, they will use the defaults. There will also always be users building it for themselves, and you can't really expect them to know that they're going to get a poorly performing Rust by default.

@michaelwoerister
Copy link
Member

Regarding debuginfo, apart from #14930 which needs to be tested again, I don't know of a crash bug at the moment. Just compiling with -g should be OK if you don't enable optimizations, LTO, and debuginfo at the same time and have some crates with and some crates without debuginfo. Using the debuginfo (especially on Windows), that's another story :)

@nrc
Copy link
Member Author

nrc commented Oct 2, 2014

@brson I'm not attached to names, I'd be happy with --debug or making debug the default and just add --release.

--debug is a make flag not a configure one, right? I think this has to be a configure thing to get the right versions of jemalloc and llvm. I think we might want to toggle things independently (e.g., to debug an optimised build), I was envisioning these things being umbrella flags. Not sure how to pass -g from configure to make, by assume it can't be too hard to make a way.

There are a bunch of configure flags which are not --enable/--disable prefixed - libdir, llvm-root, etc. Although they do all take parameters and --release would not. --enable-release doesn't really make sense though, and --disable-debug is kind of wrong because it does more than just not emit debug info etc.

Agree about the snapshots

@nrc
Copy link
Member Author

nrc commented Oct 2, 2014

@alexcrichton with debuginfo as well as the flags you list, that is four flags that need to be toggled to get an optimum build, it seems to me that having either a release or debug umbrella flag is worth while (assuming the default is the other configuration).

Infra would then use:

  • nightlies/releases: --release
  • snapshots: --release --enable-debug

@nrc
Copy link
Member Author

nrc commented Oct 2, 2014

So the tricky question is the default. I agree with @alexcrichton that the majority of people who build rustc will want the develop/debug config, but also with @thestinger that packagers shouldn't have to mess around with flags and are likely to use the default.

@thestinger are packagers likely to use a single release flag if we advertise it well? I assume they must do that for llvm, does it work in practice?

@nrc
Copy link
Member Author

nrc commented Oct 2, 2014

Actually, @alexcrichton what is the motivation for having debug=on for snapshots? I never look at logging for that stage nor does the compiler really use debug-only asserts? If we get a big speed up without it, perhaps we could leave it off?

@thestinger
Copy link
Contributor

Both debug assertions and debug logging are toggled via the same configuration flag. Since debug assertions are enabled by default, there is very little usage in the standard libraries. Extensive assertions like jemalloc would have too much cost unless they were disabled by default.

@wizeman
Copy link
Contributor

wizeman commented Oct 2, 2014

So the tricky question is the default. I agree with @alexcrichton
that the majority of people who build rustc will want the develop/debug
config, but also with @thestinger that packagers shouldn't have to mess
around with flags and are likely to use the default.

@thestinger are packagers likely to use a single release flag if we advertise
it well? I assume they must do that for llvm, does it work in practice?

If you want to make sure people use the right config, why not make
./configure always explicitly require either --develop/debug or --release,
and fail if the user does not specify one of those 2 options?

@nrc
Copy link
Member Author

nrc commented Oct 2, 2014

@wizeman I would like to do this

@nrc
Copy link
Member Author

nrc commented Oct 7, 2014

We discussed this in the weekly meeting. The conclusions were:

  • we will add --enable-debug and --enable-release flags to configure
  • if neither is specified then we will use --enable-release, a message will be displayed explaining what the flags do and the benefits of using --enable-debug if you are a developer. The message will not be displayed if you actually use --enable-release.

@nrc
Copy link
Member Author

nrc commented Oct 22, 2014

#18213 has enabled parallel codegen without the configure flags.

@thestinger
Copy link
Contributor

Note that parallel code generation had to be disabled for now because it was breaking the build on systems with an up-to-date binutils.

Manishearth added a commit to Manishearth/rust that referenced this issue Apr 9, 2015
 This makes the default configuration fully optimized, with no debugging options, no llvm asserts, renames --enable-debug to --enable-debug-assertions, and adds --enable-debug as a blanket option that toggles various things, per rust-lang#17665. It does not add a `--enable-release` flag since that would be a no-op.

cc @nrc

Fixes rust-lang#22390
Fixes rust-lang#17081
Partially addresses rust-lang#17665
Manishearth added a commit to Manishearth/rust that referenced this issue Apr 9, 2015
 This makes the default configuration fully optimized, with no debugging options, no llvm asserts, renames --enable-debug to --enable-debug-assertions, and adds --enable-debug as a blanket option that toggles various things, per rust-lang#17665. It does not add a `--enable-release` flag since that would be a no-op.

cc @nrc

Fixes rust-lang#22390
Fixes rust-lang#17081
Partially addresses rust-lang#17665
bors added a commit that referenced this issue Apr 9, 2015
This makes the default configuration fully optimized, with no debugging options, no llvm asserts, renames --enable-debug to --enable-debug-assertions, and adds --enable-debug as a blanket option that toggles various things, per #17665. It does not add a `--enable-release` flag since that would be a no-op.

cc @nrc

Fixes #22390
Fixes #17081
Partially addresses #17665
@brson brson closed this as completed Aug 22, 2016
@brson
Copy link
Contributor

brson commented Aug 22, 2016

Closing because old. If somebody ever wants to pursue this again just start over.

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

No branches or pull requests

6 participants