Skip to content

Commit

Permalink
[meta] a few changes to prevent duplicate dep builds (#4535)
Browse files Browse the repository at this point in the history 
This PR has a few changes that make builds and test runs significantly faster:

1. Remove `xtask` from the list of default-members. This makes it so that `cargo nextest run` and `cargo nextest run -p <package>` use more dependency feature sets in common.
2. Move `opt-level` settings from `profile.test` to `profile.dev`. Again, this results in more cache hits.
3. Set `profile.dev.panic` to `unwind`. This is to unify build units across dev and test builds: tests are always built with `panic = "unwind"` so that proper backtraces can be printed out. Release builds stay as `abort`.
4. For a belt-and-suspenders approach, make the `crdb-seed` script use the `test` profile. If there are any divergences between `dev` and `test` in the future, then crdb-seed should share its build cache with the tests it was presumably invoked for.
5. Set `profile.dev.build-override.debug` to `line-tables-only`. This, along with 3, means that target (normal/dev) and build (host) dependencies are now unified.

All of this comes together for a pretty sweet improvement.

See #4392 for more details and how I investigated this issue.

## Impact

With a fresh build on Linux with mold, I ran three commands in sequence:

1. `cargo nextest run --no-run`
2. `cargo nextest run -p nexus-db-queries`
3. `cargo build -p omicron-nexus`

The results were:

|               **command**               | **phase**         | **before** | **before, cumul.** | **after** | **after, cumul.** |
|-----------------------------------------|-------------------|-----------:|-------------------:|----------:|------------------:|
| `cargo nextest run`                     | build             |       173s |               173s |      158s |              158s |
| `cargo nextest run -p nexus-db-queries` | build             |        61s |               234s |       51s |              209s |
| `cargo nextest run -p nexus-db-queries` | `crdb-seed` build |        21s |               255s |        1s |              210s |
| `cargo build -p omicron-nexus`          | build             |        99s |               354s |       69s |              279s |

So the cumulative time spent on these three commands went from 354s to 279s. That's a 1.26x speedup. And this should also make other commands better as well (omicron-nexus is a bit of a weird case because it takes a very long time to compile by itself, and that 69s in the "after" column is entirely building omicron-nexus).
  • Loading branch information
@sunshowers
sunshowers authored Nov 21, 2023
1 parent 5c6ad08 commit b07a8f5
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 56 deletions.
4 changes: 3 additions & 1 deletion .config/nextest.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ setup = 'crdb-seed'
fail-fast = false

[script.crdb-seed]
command = 'cargo run -p crdb-seed'
# Use the test profile for this executable since that's how almost all
# invocations of nextest happen.
command = 'cargo run -p crdb-seed --profile test'

# The ClickHouse cluster tests currently rely on a hard-coded set of ports for
# the nodes in the cluster. We would like to relax this in the future, at which
Expand Down
122 changes: 67 additions & 55 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ default-members = [
"dev-tools/omdb",
"dev-tools/omicron-dev",
"dev-tools/thing-flinger",
"dev-tools/xtask",
# Do not include xtask in the list of default members, because this causes
# hakari to not work as well and build times to be longer.
# See omicron#4392.
"dns-server",
"gateway-cli",
"gateway-test-utils",
Expand Down Expand Up @@ -391,13 +393,27 @@ zeroize = { version = "1.6.0", features = ["zeroize_derive", "std"] }
zip = { version = "0.6.6", default-features = false, features = ["deflate","bzip2"] }
zone = { version = "0.3", default-features = false, features = ["async"] }

# NOTE: The test profile inherits from the dev profile, so settings under
# profile.dev get inherited. AVOID setting anything under profile.test: that
# will cause dev and test builds to diverge, which will cause more Cargo build
# cache misses.

[profile.dev]
panic = "abort"
# Note: This used to be panic = "abort" earlier, but that caused a lot of
# duplicate dependency builds. Letting panic be "unwind" causes dependencies
# across `cargo test` and `cargo run` to be unified. See omicron#4392.
panic = "unwind"

# See https://github.com/oxidecomputer/omicron/issues/4009 for some background context here.
# By reducing the debug level (though keeping enough to have meaningful
# backtraces), we reduce incremental build time and binary size significantly.
debug = "line-tables-only"

[profile.dev.build-override]
# Setting this to line-tables-only results in a large improvement in build
# times, because it allows target and host dependencies to be unified.
debug = "line-tables-only"

# `bindgen` is used by `samael`'s build script; building it with optimizations
# makes that build script run ~5x faster, more than offsetting the additional
# build time added to `bindgen` itself.
Expand Down Expand Up @@ -428,112 +444,108 @@ panic = "abort"
# proptest based test generation and shrinking is expensive. Let's optimize it.
[profile.dev.package.proptest]
opt-level = 3
[profile.test.package.proptest]
opt-level = 3

[profile.dev.package.bootstore]
opt-level = 3
[profile.test.package.bootstore]
opt-level = 3

# Crypto stuff always needs optimizations
[profile.test.package.sha3]
[profile.dev.package.sha3]
opt-level = 3
[profile.test.package.sha2]
[profile.dev.package.sha2]
opt-level = 3
[profile.test.package.hkdf]
[profile.dev.package.hkdf]
opt-level = 3
[profile.test.package.chacha20poly1305]
[profile.dev.package.chacha20poly1305]
opt-level = 3
[profile.test.package.chacha20]
[profile.dev.package.chacha20]
opt-level = 3
[profile.test.package.vsss-rs]
[profile.dev.package.vsss-rs]
opt-level = 3
[profile.test.package.curve25519-dalek]
[profile.dev.package.curve25519-dalek]
opt-level = 3
[profile.test.package.aead]
[profile.dev.package.aead]
opt-level = 3
[profile.test.package.aes]
[profile.dev.package.aes]
opt-level = 3
[profile.test.package.aes-gcm]
[profile.dev.package.aes-gcm]
opt-level = 3
[profile.test.package.bcrypt-pbkdf]
[profile.dev.package.bcrypt-pbkdf]
opt-level = 3
[profile.test.package.blake2]
[profile.dev.package.blake2]
opt-level = 3
[profile.test.package.blake2b_simd]
[profile.dev.package.blake2b_simd]
opt-level = 3
[profile.test.package.block-buffer]
[profile.dev.package.block-buffer]
opt-level = 3
[profile.test.package.block-padding]
[profile.dev.package.block-padding]
opt-level = 3
[profile.test.package.blowfish]
[profile.dev.package.blowfish]
opt-level = 3
[profile.test.package.constant_time_eq]
[profile.dev.package.constant_time_eq]
opt-level = 3
[profile.test.package.crypto-bigint]
[profile.dev.package.crypto-bigint]
opt-level = 3
[profile.test.package.crypto-common]
[profile.dev.package.crypto-common]
opt-level = 3
[profile.test.package.ctr]
[profile.dev.package.ctr]
opt-level = 3
[profile.test.package.cbc]
[profile.dev.package.cbc]
opt-level = 3
[profile.test.package.digest]
[profile.dev.package.digest]
opt-level = 3
[profile.test.package.ed25519]
[profile.dev.package.ed25519]
opt-level = 3
[profile.test.package.ed25519-dalek]
[profile.dev.package.ed25519-dalek]
opt-level = 3
[profile.test.package.elliptic-curve]
[profile.dev.package.elliptic-curve]
opt-level = 3
[profile.test.package.generic-array]
[profile.dev.package.generic-array]
opt-level = 3
[profile.test.package.getrandom]
[profile.dev.package.getrandom]
opt-level = 3
[profile.test.package.hmac]
[profile.dev.package.hmac]
opt-level = 3
[profile.test.package.lpc55_sign]
[profile.dev.package.lpc55_sign]
opt-level = 3
[profile.test.package.md5]
[profile.dev.package.md5]
opt-level = 3
[profile.test.package.md-5]
[profile.dev.package.md-5]
opt-level = 3
[profile.test.package.num-bigint]
[profile.dev.package.num-bigint]
opt-level = 3
[profile.test.package.num-bigint-dig]
[profile.dev.package.num-bigint-dig]
opt-level = 3
[profile.test.package.rand]
[profile.dev.package.rand]
opt-level = 3
[profile.test.package.rand_chacha]
[profile.dev.package.rand_chacha]
opt-level = 3
[profile.test.package.rand_core]
[profile.dev.package.rand_core]
opt-level = 3
[profile.test.package.rand_hc]
[profile.dev.package.rand_hc]
opt-level = 3
[profile.test.package.rand_xorshift]
[profile.dev.package.rand_xorshift]
opt-level = 3
[profile.test.package.rsa]
[profile.dev.package.rsa]
opt-level = 3
[profile.test.package.salty]
[profile.dev.package.salty]
opt-level = 3
[profile.test.package.signature]
[profile.dev.package.signature]
opt-level = 3
[profile.test.package.subtle]
[profile.dev.package.subtle]
opt-level = 3
[profile.test.package.tiny-keccak]
[profile.dev.package.tiny-keccak]
opt-level = 3
[profile.test.package.uuid]
[profile.dev.package.uuid]
opt-level = 3
[profile.test.package.cipher]
[profile.dev.package.cipher]
opt-level = 3
[profile.test.package.cpufeatures]
[profile.dev.package.cpufeatures]
opt-level = 3
[profile.test.package.poly1305]
[profile.dev.package.poly1305]
opt-level = 3
[profile.test.package.inout]
[profile.dev.package.inout]
opt-level = 3
[profile.test.package.keccak]
[profile.dev.package.keccak]
opt-level = 3

#
Expand Down

0 comments on commit b07a8f5

Please sign in to comment.