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

Remove generated [profile.release] from new projects #369

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexcrichton
Copy link
Member

This commit effectively reverts #289 and removes the generated [profile.release] section from Cargo.toml when executing cargo component new. The PR mentions that a default release build decreases from 1.6M to 52KB and that was likely due to stripping DWARF debugging information rather than many of the other settings configured. Nowadays for example a default release build (with no extra configuration and no profile.release) is around 50KB because rustc/cargo have started stripping debuginfo by default in release mode. That means that the original motivation is still solved, the "default" build is not an alarmingly large 1.6M.

Otherwise the various settings configured here can have negative effects which aren't always easy to debug if you're not familiar with them, for example:

  • lto = true and codegen-units = 1 can massively increase the time needed for a build. This can be surprising to newcomers who may have heard about Rust compile times being bad but this is an order of magnitude worse than the default settings. While these two options are good rules-of-thumb for a minimally-sized build not everyone falls into the category of needing a minimally sized build, and users can always configure this themselves.

  • debug = false is not necessary as it's already the default of the profile.release in Cargo.

  • strip = true is no longer necessary in recent versions of rustc/Cargo because this is done by default for debuginfo. This is otherwise somewhat harmful as it strips the name custom section which hinders any after-the-fact analysis of a binary because there is no symbol information to profile with, for example.

  • opt-level = "s" is not universally smaller than the default opt-level = "3" and it can also perform significantly worse. An example component I was working with today had a 100% slowdown using opt-level = "s" relative to opt-level = "3".

Overall I think it's best to inherit Cargo's and Rust's defaults for these settings by default. There's no one-size-fits-all for all projects which is why Cargo allows configuring things, but for a template I think it's best to inherit default settings since that'll ideally cause the least surprise for users who aren't otherwise familiar with these settings.

This commit effectively reverts bytecodealliance#289 and removes the generated
`[profile.release]` section from `Cargo.toml` when executing `cargo
component new`. The PR mentions that a default release build decreases
from 1.6M to 52KB and that was likely due to stripping DWARF debugging
information rather than many of the other settings configured. Nowadays
for example a default release build (with no extra configuration and no
`profile.release`) is around 50KB because rustc/cargo have started
stripping debuginfo by default in release mode. That means that the
original motivation is still solved, the "default" build is not an
alarmingly large 1.6M.

Otherwise the various settings configured here can have negative effects
which aren't always easy to debug if you're not familiar with them, for
example:

* `lto = true` and `codegen-units = 1` can massively increase the time
  needed for a build. This can be surprising to newcomers who may have
  heard about Rust compile times being bad but this is an order of
  magnitude worse than the default settings. While these two options are
  good rules-of-thumb for a minimally-sized build not everyone falls
  into the category of needing a minimally sized build, and users can
  always configure this themselves.

* `debug = false` is not necessary as it's already the default of the
  `profile.release` in Cargo.

* `strip = true` is no longer necessary in recent versions of
  rustc/Cargo because this is done by default for debuginfo. This is
  otherwise somewhat harmful as it strips the `name` custom section
  which hinders any after-the-fact analysis of a binary because there is
  no symbol information to profile with, for example.

* `opt-level = "s"` is not universally smaller than the default
  `opt-level = "3"` and it can also perform significantly worse. An
  example component I was working with today had a 100% slowdown using
  `opt-level = "s"` relative to `opt-level = "3"`.

Overall I think it's best to inherit Cargo's and Rust's defaults for
these settings by default. There's no one-size-fits-all for all projects
which is why Cargo allows configuring things, but for a template I think
it's best to inherit default settings since that'll ideally cause the
least surprise for users who aren't otherwise familiar with these settings.
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

Successfully merging this pull request may close these issues.

2 participants