Skip to content

Commit d4750c9

Browse files
joshlfjoshlf
authored
[ci] Various performance improvements (#2829)
* [ci] Cache MSRV's cargo index to improve performance This seems to speed things up significantly, resulting e.g. in [1] taking 3m34s (with a cache hit, skipping the "Generate cache" step). [1] https://github.com/google/zerocopy/actions/runs/19657114158 * [ci] Avoid downloading cache unnecessarily When generating the cache, if there's a cache hit, don't download the cache. Previously, we would download the cache and *then* check for a cache hit. * [ci] Calculate code coverage in parallel * Make cargo-zerocopy smarter about caching Modify the `cargo-zerocopy` tool: - To set `CARGO_TARGET_DIR` to a toolchain-specific directory inside of `target/` so that different toolchains won't clobber each others' artifacts - To auto-install toolchains and toolchain components via `rustup` without prompting for user input when running from a GitHub Actions environment This allows us to remove an explicit install step during CI.
1 parent 0b7b338 commit d4750c9

File tree

4 files changed

+134
-109
lines changed

4 files changed

+134
-109
lines changed

‎.github/actions/cache/action.yml‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,55 @@
77
# those terms.
88

99
name: Cache
10-
description: 'Caches cargo dependencies'
11-
outputs:
12-
cache-hit:
13-
description: "Cache Hit"
14-
value: ${{ steps.cache.outputs.cache-hit }}
10+
description: "Caches cargo and rustup dependencies"
11+
inputs:
12+
mode:
13+
description: "restore or generate"
14+
default: "restore"
15+
run:
16+
description: "Commands to run to populate cache (only for generate mode)"
17+
default: ""
1518
runs:
1619
using: composite
1720
steps:
18-
- uses: actions/cache@v4
21+
- name: Restore Cache
22+
if: inputs.mode == 'restore'
23+
uses: actions/cache@v4
1924
id: cache
2025
with:
21-
path: |
22-
~/.cargo/
26+
path: ~/.cargo/
2327
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}
2428
restore-keys: |
2529
${{ runner.os }}-cargo-
2630
31+
- name: Check Cache (Generate Mode)
32+
if: inputs.mode == 'generate'
33+
id: check
34+
shell: bash
35+
env:
36+
RUNNER_OS: ${{ runner.os }}
37+
CARGO_TOML_HASH: ${{ hashFiles('**/Cargo.toml') }}
38+
GH_TOKEN: ${{ github.token }}
39+
run: |
40+
CACHE_KEY="${RUNNER_OS}-cargo-${CARGO_TOML_HASH}"
41+
if gh cache list --key "$CACHE_KEY" --limit 1 | grep -q "$CACHE_KEY"; then
42+
echo "cache-hit=true" >> $GITHUB_OUTPUT
43+
else
44+
echo "cache-hit=false" >> $GITHUB_OUTPUT
45+
fi
46+
47+
- name: Run Commands (Generate Mode)
48+
if: inputs.mode == 'generate' && steps.check.outputs.cache-hit != 'true'
49+
shell: bash
50+
env:
51+
RUN_COMMANDS: ${{ inputs.run }}
52+
run: |
53+
echo "$RUN_COMMANDS" > run_commands.sh
54+
source run_commands.sh
55+
56+
- name: Save Cache (Generate Mode)
57+
if: inputs.mode == 'generate' && steps.check.outputs.cache-hit != 'true'
58+
uses: actions/cache/save@v4
59+
with:
60+
path: ~/.cargo/
61+
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}

‎.github/workflows/ci.yml‎

Lines changed: 71 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -598,51 +598,34 @@ jobs:
598598
matrix.target != 'wasm32-unknown-unknown' &&
599599
env.ZC_SKIP_CARGO_SEMVER_CHECKS != '1'
600600
601-
# TODO(#453): Doing this as a matrix step is a hack that allows us to depend
602-
# on the fact that toolchains have already been installed. We currently only
603-
# run this on a single matrix combination, but doing it outside of the
604-
# matrix would require us to replicate the toolchain resolution and
605-
# installation logic. We should either:
606-
# - Figure out how to factor out the toolchain resolution and installation
607-
# logic (see #1275) for an attempt
608-
# - Support multiple matrix combinations (which we intend to do as part of
609-
# #453 eventually anyway) so that this location is justified
610-
- name: Generate code coverage
611-
env:
612-
TOOLCHAIN: ${{ matrix.toolchain }}
613-
CRATE: ${{ matrix.crate }}
614-
TARGET: ${{ matrix.target }}
615-
FEATURES: ${{ matrix.features }}
616-
run: |
617-
set -eo pipefail
618-
./cargo.sh +nightly install cargo-llvm-cov
619-
620-
./cargo.sh +$TOOLCHAIN llvm-cov \
621-
--package $CRATE \
622-
--target $TARGET \
623-
$FEATURES \
624-
--doctests \
625-
--lcov \
626-
--output-path lcov.info \
627-
--verbose \
628-
-- \
629-
--skip ui
630-
if: |
631-
matrix.crate == 'zerocopy' &&
632-
matrix.features == '--all-features' &&
633-
matrix.toolchain == 'nightly' &&
634-
matrix.target == 'x86_64-unknown-linux-gnu'
635-
636-
- name: Upload coverage to Codecov
637-
uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1
638-
with:
639-
token: ${{ secrets.CODECOV_TOKEN }}
640-
files: lcov.info
641-
if: |
642-
matrix.crate == 'zerocopy' &&
643-
matrix.features == '--all-features' &&
644-
matrix.toolchain == 'nightly' &&
645-
matrix.target == 'x86_64-unknown-linux-gnu'
601+
coverage:
602+
runs-on: ubuntu-latest
603+
name: Generate code coverage
604+
needs: generate_cache
605+
steps:
606+
- uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0
607+
with:
608+
persist-credentials: false
609+
- name: Populate cache
610+
uses: ./.github/actions/cache
611+
- name: Generate code coverage
612+
run: |
613+
set -eo pipefail
614+
./cargo.sh +nightly install cargo-llvm-cov
615+
./cargo.sh +nightly llvm-cov \
616+
--package zerocopy \
617+
--target x86_64-unknown-linux-gnu \
618+
--all-features \
619+
--doctests \
620+
--lcov \
621+
--output-path lcov.info \
622+
--verbose \
623+
-- --skip ui
624+
- name: Upload coverage to Codecov
625+
uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1
626+
with:
627+
token: ${{ secrets.CODECOV_TOKEN }}
628+
files: lcov.info
646629

647630
kani:
648631
runs-on: ubuntu-latest
@@ -832,47 +815,49 @@ jobs:
832815
with:
833816
persist-credentials: false
834817

835-
- id: populate-cache
818+
- name: Populate cache
836819
uses: ./.github/actions/cache
837-
838-
- name: Download dependencies
839-
if: steps.populate-cache.outputs.cache-hit != 'true'
840-
run: |
841-
# Ensure all dependencies are downloaded - both for our crates and for
842-
# tools we use in CI. We don't care about these tools succeeding for
843-
# two reasons: First, this entire job is best-effort since it's just a
844-
# performance optimization. Second, there may be failures due to
845-
# issues other than failing to download dependencies (e.g., `cargo
846-
# metadata` called with a malformed `Cargo.toml`, build failure in our
847-
# own crate or in dependencies, etc). For those reasons, we discard
848-
# stderr and ignore status codes.
849-
#
850-
# For downloading our crates' dependencies in particular, note that
851-
# there is no support for doing this directly [1], so we just check
852-
# all crates using --tests.
853-
#
854-
# We background all jobs and then wait for them so that they can run
855-
# in parallel.
856-
#
857-
# [1] https://stackoverflow.com/a/42139535/836390
858-
859-
# See comment on "Pin syn dependency" job for why we do this. It needs
860-
# to happen before the subsequent `cargo check`, so we don't
861-
# background it.
862-
#
863-
# TODO(#1595): Debug why this step is still necessary after #1564 and
864-
# maybe remove it.
865-
cargo add -p zerocopy-derive 'syn@=2.0.46' &> /dev/null
866-
867-
cargo check --workspace --tests &> /dev/null &
868-
cargo metadata &> /dev/null &
869-
cargo install cargo-readme --version 3.2.0 &> /dev/null &
870-
cargo install --locked action-validator --version 0.8.0 &> /dev/null &
871-
cargo install --locked kani-verifier &> /dev/null &
872-
cargo install cargo-nextest &> /dev/null &
873-
cargo kani setup &> /dev/null &
874-
875-
wait
820+
with:
821+
mode: generate
822+
run: |
823+
# Ensure all dependencies are downloaded - both for our crates and for
824+
# tools we use in CI. We don't care about these tools succeeding for
825+
# two reasons: First, this entire job is best-effort since it's just a
826+
# performance optimization. Second, there may be failures due to
827+
# issues other than failing to download dependencies (e.g., `cargo
828+
# metadata` called with a malformed `Cargo.toml`, build failure in our
829+
# own crate or in dependencies, etc). For those reasons, we discard
830+
# stderr and ignore status codes.
831+
#
832+
# For downloading our crates' dependencies in particular, note that
833+
# there is no support for doing this directly [1], so we just check
834+
# all crates using --tests.
835+
#
836+
# We background all jobs and then wait for them so that they can run
837+
# in parallel.
838+
#
839+
# [1] https://stackoverflow.com/a/42139535/836390
840+
841+
# See comment on "Pin syn dependency" job for why we do this. It needs
842+
# to happen before the subsequent `cargo check`, so we don't
843+
# background it.
844+
#
845+
# TODO(#1595): Debug why this step is still necessary after #1564 and
846+
# maybe remove it.
847+
cargo add -p zerocopy-derive 'syn@=2.0.46' &> /dev/null
848+
849+
cargo check --workspace --tests &> /dev/null &
850+
# On our MSRV toolchain, updating the Cargo index takes a long time, so
851+
# it is worth specifically caching the MSRV index.
852+
./cargo.sh +msrv check --workspace --tests &> /dev/null &
853+
cargo metadata &> /dev/null &
854+
cargo install cargo-readme --version 3.2.0 &> /dev/null &
855+
cargo install --locked action-validator --version 0.8.0 &> /dev/null &
856+
cargo install --locked kani-verifier &> /dev/null &
857+
cargo install cargo-nextest &> /dev/null &
858+
cargo kani setup &> /dev/null &
859+
860+
wait
876861
877862
check-all-toolchains-tested:
878863
runs-on: ubuntu-latest
@@ -966,7 +951,7 @@ jobs:
966951
# https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks
967952
if: failure()
968953
runs-on: ubuntu-latest
969-
needs: [build_test, kani, check_be_aarch64, check_avr_atmega, check_fmt, check_actions, check_readme, check_versions, check_msrv_is_minimal, generate_cache, check-all-toolchains-tested, check-job-dependencies, check-todo, run-git-hooks, zizmor]
954+
needs: [build_test, coverage, kani, check_be_aarch64, check_avr_atmega, check_fmt, check_actions, check_readme, check_versions, check_msrv_is_minimal, generate_cache, check-all-toolchains-tested, check-job-dependencies, check-todo, run-git-hooks, zizmor]
970955
steps:
971956
- name: Mark the job as failed
972957
run: exit 1

‎cargo.sh‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
set -eo pipefail
1010

11-
# Build `cargo-zerocopy` without any RUSTFLAGS set in the environment
12-
env -u RUSTFLAGS cargo +stable build --manifest-path tools/Cargo.toml -p cargo-zerocopy -q
11+
# Build `cargo-zerocopy` without any RUSTFLAGS or CARGO_TARGET_DIR set in the
12+
# environment
13+
env -u RUSTFLAGS -u CARGO_TARGET_DIR cargo +stable build --manifest-path tools/Cargo.toml -p cargo-zerocopy -q
1314
# Thin wrapper around the `cargo-zerocopy` binary in `tools/cargo-zerocopy`
1415
./tools/target/debug/cargo-zerocopy $@

‎tools/cargo-zerocopy/src/main.rs‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,19 +141,17 @@ fn is_toolchain_installed(versions: &Versions, name: &str) -> Result<bool, Error
141141
fn install_toolchain_or_exit(versions: &Versions, name: &str) -> Result<(), Error> {
142142
eprintln!("[cargo-zerocopy] missing either toolchain '{name}' or component 'rust-src'");
143143
if env::vars().any(|v| v.0 == "GITHUB_RUN_ID") {
144-
// If we're running in a GitHub action, then it's better to bail than to
145-
// hang waiting for input we're never going to get.
146-
process::exit(1);
147-
}
148-
149-
loop {
150-
eprint!("[cargo-zerocopy] would you like to install toolchain '{name}' and component 'rust-src' via 'rustup' (y/n)? ");
151-
let mut input = [0];
152-
io::stdin().read_exact(&mut input).unwrap();
153-
match input[0] as char {
154-
'y' | 'Y' => break,
155-
'n' | 'N' => process::exit(1),
156-
_ => (),
144+
eprint!("[cargo-zerocopy] detected GitHub Actions environment; auto-installing without waiting for confirmation");
145+
} else {
146+
loop {
147+
eprint!("[cargo-zerocopy] would you like to install toolchain '{name}' and component 'rust-src' via 'rustup' (y/n)? ");
148+
let mut input = [0];
149+
io::stdin().read_exact(&mut input).unwrap();
150+
match input[0] as char {
151+
'y' | 'Y' => break,
152+
'n' | 'N' => process::exit(1),
153+
_ => (),
154+
}
157155
}
158156
}
159157

@@ -235,6 +233,12 @@ fn delegate_cargo() -> Result<(), Error> {
235233

236234
let mut cmd = rustup(["run", version, "cargo"], Some(("RUSTFLAGS", &rustflags)));
237235

236+
if env::var("CARGO_TARGET_DIR").is_ok() {
237+
eprintln!("[cargo-zerocopy] WARNING: `CARGO_TARGET_DIR` is set - this may cause `cargo-zerocopy` to behave unexpectedly");
238+
} else {
239+
cmd.env("CARGO_TARGET_DIR", format!("target/by-toolchain/{}", name));
240+
}
241+
238242
// Computes the fully-qualified package name of workspace package `p`.
239243
let fqpn = |p| {
240244
// Generate a lockfile, if absent.

0 commit comments

Comments
 (0)