Rust build caching improvements & fixes (#3197)

Caches are getting too big and we are exceeding the 10GB limit, leading to cache churning. 1. Try to make the caches smaller by using `Swatinem/rust-cache`, which is smarter about what gets cached. - After doing this it turns out we don't really need `sccache` any more, it has very little impact upon compile times as the cache hit ratio is low. So remove it, to reduce complexity of build and size of build caches. 2. Also fix artifact caching which had been broken by a version format change (4956cf5).
microsoft · Jun 19, 2023 · 28e36ef · 28e36ef
1 parent b1b7cf0
commit 28e36ef
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 90 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -16,9 +16,7 @@ concurrency:
 
 env:
   CARGO_TERM_COLOR: always
-  SCCACHE_DIR: ${{github.workspace}}/sccache/
-  SCCACHE_CACHE_SIZE: 1G
-  ACTIONS_CACHE_KEY_DATE: 2023-04-19
+  ACTIONS_CACHE_KEY_DATE: 2023-06-19
   CI: true
 
 jobs:
@@ -54,10 +52,12 @@ jobs:
       - name: Get Rust version & build version
         shell: bash
         run: |
+          set -x
           echo "RUST_VERSION=$(rustc --version)" >> $GITHUB_OUTPUT
           VERSION=$(src/ci/get-version.sh)
-          # it's a release build if version doesn't have a hyphen in it
-          IS_RELEASE_BUILD=$(if [[ "$VERSION" =~ '-' ]]; then echo 'false'; else echo 'true'; fi)
+          # it's a release build if version doesn't have a plus in it
+          # NB: this should stay in sync with version generation in get-version.sh
+          IS_RELEASE_BUILD=$(if [[ "$VERSION" =~ '+' ]]; then echo 'false'; else echo 'true'; fi)
           echo "RELEASE_BUILD=$IS_RELEASE_BUILD" >> $GITHUB_OUTPUT
         id: rust-version
       - name: Rust artifact cache
@@ -71,35 +71,22 @@ jobs:
           path: artifacts
           key: agent-artifacts|${{ join(matrix.os, ':') }}|${{steps.rust-version.outputs.RUST_VERSION}}|${{ env.ACTIONS_CACHE_KEY_DATE }}|${{ hashFiles('src/agent/**/*') }}|${{hashFiles('src/ci/agent.sh')}}
           # note: also including the ACTIONS_CACHE_KEY_DATE to rebuild if the Prereq Cache is invalidated
+      - name: Rust build cache
+        id: rust-build-cache
+        uses: Swatinem/rust-cache@v2
+        if: steps.cache-agent-artifacts.outputs.cache-hit != 'true'
+        with:
+          key: ${{env.ACTIONS_CACHE_KEY_DATE}} # additional key for cache-busting
+          workspaces: src/agent
       - name: Linux Prereqs
         if: runner.os == 'Linux' && steps.cache-agent-artifacts.outputs.cache-hit != 'true'
         run: |
           sudo apt-get -y update
           sudo apt-get -y install libssl-dev libunwind-dev build-essential pkg-config
-      - name: Rust Prereq Cache
-        if: steps.cache-agent-artifacts.outputs.cache-hit != 'true'
-        uses: actions/cache@v3
-        id: cache-rust-prereqs
-        with:
-          path: |
-            ~/.cargo/registry
-            ~/.cargo/git
-            ~/.cargo/bin
-          key: rust|${{ join(matrix.os, ':') }}|${{steps.rust-version.outputs.RUST_VERSION}}|${{ env.ACTIONS_CACHE_KEY_DATE }}
       - name: Install Rust Prereqs
-        if: steps.cache-rust-prereqs.outputs.cache-hit != 'true' && steps.cache-agent-artifacts.outputs.cache-hit != 'true'
+        if: steps.rust-build-cache.outputs.cache-hit != 'true' && steps.cache-agent-artifacts.outputs.cache-hit != 'true'
         shell: bash
         run: src/ci/rust-prereqs.sh
-      - name: Rust Compile Cache
-        if: steps.cache-agent-artifacts.outputs.cache-hit != 'true'
-        uses: actions/cache@v3
-        with:
-          path: |
-            sccache
-            src/agent/target
-          key: agent|${{ join(matrix.os, ':') }}|${{steps.rust-version.outputs.RUST_VERSION}}|${{ env.ACTIONS_CACHE_KEY_DATE }}|${{ hashFiles('src/agent/Cargo.lock') }}
-          restore-keys: |
-            agent|${{ join(matrix.os, ':') }}|${{steps.rust-version.outputs.RUST_VERSION}}|${{ env.ACTIONS_CACHE_KEY_DATE }}|
       - run: src/ci/agent.sh
         if: steps.cache-agent-artifacts.outputs.cache-hit != 'true'
         shell: bash
@@ -237,29 +224,16 @@ jobs:
     runs-on: ubuntu-20.04
     steps:
       - uses: actions/checkout@v3
-      - name: Rust Prereq Cache
-        uses: actions/cache@v3
-        id: cache-rust-prereqs
+      - name: Rust build cache
+        id: rust-build-cache
+        uses: Swatinem/rust-cache@v2
         with:
-          path: |
-            ~/.cargo/registry
-            ~/.cargo/git
-            ~/.cargo/bin
-          key: rust-${{ runner.os }}-${{ env.ACTIONS_CACHE_KEY_DATE }}
+          key: ${{env.ACTIONS_CACHE_KEY_DATE}} # additional key for cache-busting
+          workspaces: src/proxy-manager
       - name: Install Rust Prereqs
-        if: steps.cache-rust-prereqs.outputs.cache-hit != 'true'
+        if: steps.rust-build-cache.outputs.cache-hit != 'true'
         shell: bash
         run: src/ci/rust-prereqs.sh
-      - name: Rust Compile Cache
-        uses: actions/cache@v3
-        with:
-          path: |
-            sccache
-            src/proxy-manager/target
-          key: proxy-${{ runner.os }}-${{ hashFiles('src/proxy-manager/Cargo.lock') }}-${{ env.ACTIONS_CACHE_KEY_DATE }}
-          restore-keys: |
-            proxy-${{ runner.os }}-${{ hashFiles('src/proxy-manager/Cargo.lock') }}-
-            proxy-${{ runner.os }}-
       - run: src/ci/proxy.sh
       - uses: actions/upload-artifact@v3
         with:
@@ -301,6 +275,7 @@ jobs:
 
       - name: Build Service
         run: |
+          set -x
           version=$(./src/ci/get-version.sh)
 
           cd src/ApiService/
@@ -310,10 +285,12 @@ jobs:
           echo ${GITHUB_SHA} | tee ApiService/onefuzzlib/git.version
 
           # stamp the build with version
-          # note that version might have a suffix of '-{sha}' from get-version.sh
-          if [[ "$version" =~ '-' ]]; then
+          # note that version might have a suffix of '+{sha}' from get-version.sh
+
+          # NB: ensure this stays in sync with get-version.sh
+          if [[ "$version" =~ '+' ]]; then
             # if it has a suffix, split it into two parts
-            dotnet build -warnaserror --configuration Release /p:VersionPrefix=${version%-*} /p:VersionSuffix=${version#*-}
+            dotnet build -warnaserror --configuration Release /p:VersionPrefix=${version%+*} /p:VersionSuffix=${version#*+}
           else
             dotnet build -warnaserror --configuration Release /p:VersionPrefix=${version}
           fi

diff --git a/src/agent/onefuzz/src/libfuzzer.rs b/src/agent/onefuzz/src/libfuzzer.rs
@@ -173,7 +173,11 @@ impl LibFuzzer {
         Ok(cmd)
     }
 
-    async fn verify_inner(&self, check_fuzzer_help: bool, inputs: &[&Path]) -> Result<()> {
+    pub(crate) async fn verify_once(
+        &self,
+        check_fuzzer_help: bool,
+        inputs: &[&Path],
+    ) -> Result<()> {
         if check_fuzzer_help {
             self.check_help().await?;
         }
@@ -222,7 +226,7 @@ impl LibFuzzer {
         let mut attempts = 1;
         loop {
             let result = self
-                .verify_inner(check_fuzzer_help, inputs.unwrap_or_default())
+                .verify_once(check_fuzzer_help, inputs.unwrap_or_default())
                 .await;
 
             match result {
@@ -519,22 +523,22 @@ mod tests {
 
         // verify catching bad exits with -help=1
         assert!(
-            fuzzer.verify(true, None).await.is_err(),
+            fuzzer.verify_once(true, &[]).await.is_err(),
             "checking false with -help=1"
         );
 
         // verify catching bad exits with inputs
         assert!(
             fuzzer
-                .verify(false, Some(&[temp_setup_dir.path()]))
+                .verify_once(false, &[temp_setup_dir.path()])
                 .await
                 .is_err(),
             "checking false with basic input"
         );
 
         // verify catching bad exits with no inputs
         assert!(
-            fuzzer.verify(false, None).await.is_err(),
+            fuzzer.verify_once(false, &[]).await.is_err(),
             "checking false without inputs"
         );
 
@@ -553,22 +557,22 @@ mod tests {
         );
         // verify good exits with -help=1
         assert!(
-            fuzzer.verify(true, None).await.is_ok(),
+            fuzzer.verify_once(true, &[]).await.is_ok(),
             "checking true with -help=1"
         );
 
         // verify good exits with inputs
         assert!(
             fuzzer
-                .verify(false, Some(&[temp_setup_dir.path()]))
+                .verify_once(false, &[temp_setup_dir.path()])
                 .await
                 .is_ok(),
             "checking true with basic inputs"
         );
 
         // verify good exits with no inputs
         assert!(
-            fuzzer.verify(false, None).await.is_ok(),
+            fuzzer.verify_once(false, &[]).await.is_ok(),
             "checking true without inputs"
         );
 

diff --git a/src/ci/agent.sh b/src/ci/agent.sh
@@ -11,25 +11,6 @@ exists() {
     [ -e "$1" ]
 }
 
-SCCACHE=$(which sccache || echo '')
-if [ -n "$SCCACHE" ]; then
-    # only set RUSTC_WRAPPER if sccache exists
-    export RUSTC_WRAPPER=$SCCACHE
-    # incremental interferes with (disables) sccache
-    export CARGO_INCREMENTAL=0
-else
-    # only set CARGO_INCREMENTAL on non-release builds
-    #
-    # This speeds up build time, but makes the resulting binaries slightly slower.
-    # https://doc.rust-lang.org/cargo/reference/profiles.html?highlight=incremental#incremental
-    if [ "${GITHUB_REF}" != "" ]; then
-        TAG_VERSION=${GITHUB_REF#refs/tags/}
-        if [ ${TAG_VERSION} == ${GITHUB_REF} ]; then
-            export CARGO_INCREMENTAL=1
-        fi
-    fi
-fi
-
 platform=$(uname --kernel-name --machine)
 platform=${platform// /-} # replace spaces with dashes
 rel_output_dir="artifacts/agent-$platform"
@@ -45,11 +26,6 @@ cargo clippy --version
 cargo fmt --version
 cargo license --version
 
-# unless we're doing incremental builds, start clean during CI
-if [ X${CARGO_INCREMENTAL} == X ]; then
-    cargo clean
-fi
-
 cargo fmt -- --check
 
 cargo deny -L error check
@@ -69,10 +45,6 @@ cargo llvm-cov nextest --all-targets --locked --workspace --lcov --output-path "
 # TODO: once Salvo is integrated, this can get deleted
 cargo build --release --locked --manifest-path ./onefuzz-telemetry/Cargo.toml --all-features
 
-if [ -n "$SCCACHE" ]; then
-    sccache --show-stats
-fi
-
 echo "Checking dependencies of binaries"
 
 "$script_dir/check-dependencies.sh"

diff --git a/src/ci/get-version.sh b/src/ci/get-version.sh
@@ -10,6 +10,9 @@ BASE_VERSION=$(cat ${SCRIPT_DIR}/../../CURRENT_VERSION)
 BRANCH=$(git rev-parse --abbrev-ref HEAD)
 GIT_HASH=$(git rev-parse HEAD)
 
+# NB: ensure this code stays in sync the with version test in 
+# .github/workflows/ci.yml
+
 if [ "${GITHUB_REF}" != "" ]; then
     TAG_VERSION=${GITHUB_REF#refs/tags/}
 

diff --git a/src/ci/proxy.sh b/src/ci/proxy.sh
@@ -5,8 +5,6 @@
 
 set -ex
 
-#export RUSTC_WRAPPER=$(which sccache)
-
 mkdir -p artifacts/proxy
 
 cd src/proxy-manager

diff --git a/src/ci/rust-prereqs.sh b/src/ci/rust-prereqs.sh
@@ -5,7 +5,4 @@
 
 set -ex
 
-cargo install --locked sccache cargo-license@0.4.2 cargo-llvm-cov cargo-deny cargo-insta cargo-nextest
-
-# sccache --start-server
-# export RUSTC_WRAPPER=$(which sccache)
+cargo install --locked cargo-license@0.4.2 cargo-llvm-cov cargo-deny cargo-insta cargo-nextest