From b07a8f593325efe97ddb526c2725d45d480bf7e6 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 21 Nov 2023 14:59:21 -0800 Subject: [PATCH] [meta] a few changes to prevent duplicate dep builds (#4535) 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 ` 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). --- .config/nextest.toml | 4 +- Cargo.toml | 122 ++++++++++++++++++++++++------------------- 2 files changed, 70 insertions(+), 56 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index 79774e3658..ef296d7ef8 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -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 diff --git a/Cargo.toml b/Cargo.toml index fb220ba53d..f3da0381df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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", @@ -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. @@ -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 #