Skip to content

Commit

Permalink
chore: introduce --profile prod (#7923)
Browse files Browse the repository at this point in the history
This solves two problems:

* Makes it easy to produce a fully-optimized binary when doing lto-sensitive benchmarking
* Makes it simpler for the estimator to build the binary with the right optimizations, by avoiding hard-coding config into the estimator.

Ideally, we'd change the Makefile as well, but we'd rather not break existing `./target/release` layout.

In other words, before we had two sources of truth: Makefile and estimator. Now they are Makefile and Cargo.toml, and we also gained a nice cli flag for building fully optimized version:

$ cargo b --profile prod -p neard --bin neard

cc #6226

This PR is prompted by the recent confusion about lto: https://near.zulipchat.com/#narrow/stream/345766-pagoda.2Fstorage.2Fflat-storage/topic/Migration.20plan/near/305801512

I don't think this solves the problem, but hopefully existence of `--profile prod` would make it more obvious? 


Not super sure though, this is more like an RFC than "yes, I think we should have this"
  • Loading branch information
matklad authored and nikurt committed Nov 9, 2022
1 parent 81686c8 commit 21af417
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 20 deletions.
13 changes: 11 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,21 @@ xz2 = "0.1.6"
# "test" profile inherits from "dev" profile.
# https://doc.rust-lang.org/cargo/reference/profiles.html#test

[profile.dev]
panic = 'abort'

[profile.release]
overflow-checks = true
panic = 'abort'
lto = "fat"
codegen-units = 1

# A much faster to compile version of `release`.
[profile.quick-release]
inherits = "release"
lto = false
codegen-units = 16

[profile.dev]
panic = 'abort'

# Compile some dependencies with optimizations to speed up tests.
[profile.dev.package.hex]
Expand Down
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
export CARGO_PROFILE_RELEASE_CODEGEN_UNITS = 1
export CARGO_PROFILE_RELEASE_LTO = fat
export DOCKER_BUILDKIT = 1
export CARGO_BUILD_RUSTFLAGS = -D warnings
export NEAR_RELEASE_BUILD = no
Expand Down
20 changes: 11 additions & 9 deletions docs/practices/fast_builds.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,17 @@ contains a section on compilation time as well!

## Release Builds and Link Time Optimization

Obviously, `cargo build --release` is slower than `cargo build`. What's not
entirely obvious is that `cargo build -r` is not as slow as it could be: our
`--release` profile is somewhat optimized for fast builds, as it doesn't enable
full LTO.

When building production binaries, we use `lto=true` and `codegen-units=1`
options, which make the build significantly slower (but the resulting binary
somewhat faster). Keep this in mind when running benchmarks or parameter
estimation.
Obviously, `cargo build --release` is slower than `cargo build`. We enable full
lto (link time optimization), so our `-r` builds are very slow, use a lot of
RAM, and don't take advantage full of parallelism.

As debug builds are much to slow at runtime for many purposes, we have a custom
profile `--profile quick-release` which is equivalent to `-r`, but doesn't do
`lto`.

Use `--profile quick-release` when doing comparative benchmarking, or when
connecting a locally build node to a network. Use `-r` if you want to get
absolute performance numbers.

## Linker

Expand Down
13 changes: 6 additions & 7 deletions runtime/runtime-params-estimator/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ fn main_docker(
json_output: bool,
debug: bool,
) -> anyhow::Result<()> {
let profile = if full { "release" } else { "quick-release" };
exec("docker --version").context("please install `docker`")?;

let project_root = project_root();
Expand Down Expand Up @@ -340,15 +341,17 @@ fn main_docker(
#[cfg(feature = "nightly_protocol")]
buf.push_str(",nightly_protocol");

buf.push_str(" --release;");
buf.push_str(" --profile ");
buf.push_str(profile);
buf.push_str(";");

let mut qemu_cmd_builder = QemuCommandBuilder::default();

if debug {
qemu_cmd_builder = qemu_cmd_builder.plugin_log(true).print_on_every_close(true);
}
let mut qemu_cmd =
qemu_cmd_builder.build("/host/nearcore/target/release/runtime-params-estimator")?;
let mut qemu_cmd = qemu_cmd_builder
.build(&format!("/host/nearcore/target/{profile}/runtime-params-estimator"))?;

qemu_cmd.args(&["--home", "/.near"]);
buf.push_str(&format!("{:?}", qemu_cmd));
Expand Down Expand Up @@ -404,10 +407,6 @@ fn main_docker(
if debug_shell || !json_output {
cmd.args(&["--interactive", "--tty"]);
}
if full {
cmd.args(&["--env", "CARGO_PROFILE_RELEASE_LTO=fat"])
.args(&["--env", "CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1"]);
}
cmd.arg(tagged_image);

if debug_shell {
Expand Down

0 comments on commit 21af417

Please sign in to comment.