From 87f4a0585dc3056433f193b9305f587cff239be3 Mon Sep 17 00:00:00 2001 From: kaukabrizvi <100529019+kaukabrizvi@users.noreply.github.com> Date: Mon, 19 Aug 2024 10:52:27 -0700 Subject: [PATCH] Add performance regression tests in CI (#4701) --- .github/workflows/regression_ci.yml | 87 +++++++++++++++++++++++++++++ tests/regression/README.md | 2 +- tests/regression/src/lib.rs | 68 +++++++++++++++++----- 3 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/regression_ci.yml diff --git a/.github/workflows/regression_ci.yml b/.github/workflows/regression_ci.yml new file mode 100644 index 00000000000..4fb9b800de1 --- /dev/null +++ b/.github/workflows/regression_ci.yml @@ -0,0 +1,87 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: MIT-0 +name: Performance Regression Test + +on: + pull_request: + branches: + - main + paths-ignore: + - tests/regression/** + +jobs: + regression-test: + runs-on: ubuntu-latest + + steps: + # Checkout the code from the pull request branch + - name: Checkout code + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + # Install the stable Rust toolchain + - name: Install Rust toolchain + id: toolchain + run: | + rustup toolchain install stable + rustup override set stable + + # Update the package list on the runner + - name: Update package list + run: sudo apt-get update + + # Download and install Valgrind 3.23 from source + - name: Download Valgrind 3.23 Source + run: | + wget https://sourceware.org/pub/valgrind/valgrind-3.23.0.tar.bz2 + tar -xjf valgrind-3.23.0.tar.bz2 + cd valgrind-3.23.0 + ./configure + make + sudo make install + + # Generate the necessary bindings + - name: Generate + run: ${{env.ROOT_PATH}}bindings/rust/generate.sh --skip-tests + + # Run performance tests using Valgrind for current branch + - name: Run scalar performance test (curr) + env: + PERF_MODE: valgrind + run: cargo test --release --manifest-path=tests/regression/Cargo.toml + + # Switch to the main branch + - name: Switch to mainline + run: | + git fetch origin main + git switch main + + # Regenerate bindings for main branch + - name: Generate + run: ${{env.ROOT_PATH}}bindings/rust/generate.sh --skip-tests + + # Run performance tests using Valgrind for main branch + - name: Run scalar performance test (prev) + env: + PERF_MODE: valgrind + run: cargo test --release --manifest-path=tests/regression/Cargo.toml + + # Checkout pull request branch again + # This is required for cg_annotate diff to locate the changes in the PR to properly annotate the output diff file + - name: Checkout pull request branch + run: git checkout ${{ github.event.pull_request.head.sha }} + + # Run the differential performance test + - name: Run diff test + env: + PERF_MODE: diff + run: cargo test --release --manifest-path=tests/regression/Cargo.toml + + # Upload the performance output artifacts. This runs even if run diff test fails so debug files can be accessed + - name: Upload artifacts + if: ${{ always() }} + uses: actions/upload-artifact@v4 + with: + name: regression_artifacts + path: tests/regression/target/regression_artifacts diff --git a/tests/regression/README.md b/tests/regression/README.md index 8ee4ce3a707..e010620d123 100644 --- a/tests/regression/README.md +++ b/tests/regression/README.md @@ -24,7 +24,7 @@ The performance benchmarking framework utilizes CPU Instruction count across API Ensure you have the following installed: - Rust (with Cargo) -- Valgrind (for cachegrind instrumentation) +- Valgrind (for cachegrind instrumentation): Valgrind 3.23 or newer is required to run the tests, since cachegrind annotation is not included in earlier versions. If this version is not automatically downloaded by running `apt install valgrind`, it can be installed manually by following https://valgrind.org/downloads/ ## Running the Harnesses with Valgrind (scalar performance) To run the harnesses with Valgrind and store the annotated results, run: diff --git a/tests/regression/src/lib.rs b/tests/regression/src/lib.rs index 04334ce3745..5d0cf02b827 100644 --- a/tests/regression/src/lib.rs +++ b/tests/regression/src/lib.rs @@ -27,14 +27,37 @@ pub mod git { } pub fn extract_commit_hash(file: &str) -> String { - // input: "target/$commit_id/test_name.raw" + // input: "target/regression_artifacts/$commit_id/test_name.raw" // output: "$commit_id" - file.split("target/") + file.split("target/regression_artifacts/") .nth(1) .and_then(|s| s.split('/').next()) .map(|s| s.to_string()) .unwrap_or_default() // This will return an empty string if the Option is None } + + pub fn is_mainline(commit_hash: &str) -> bool { + // Execute the git command to check which branches contain the given commit. + let output = Command::new("git") + .args(["branch", "--contains", commit_hash]) + .output() + .expect("Failed to execute git branch"); + + // If the command fails, it indicates that the commit is either detached + // or does not exist in any branches. Meaning, it is not part of mainline. + if !output.status.success() { + return false; + } + + // Convert the command output to a string and check each line. + let branches = String::from_utf8_lossy(&output.stdout); + branches.lines().any(|branch| { + // Trim the branch name to remove any leading or trailing whitespace. + // The branch name could be prefixed with '*', indicating the current branch. + // We check for both "main" and "* main" to account for this possibility. + branch.trim() == "main" || branch.trim() == "* main" + }) + } } #[cfg(test)] @@ -117,7 +140,7 @@ mod tests { impl RawProfile { fn new(test_name: &str) -> Self { let commit_hash = git::get_current_commit_hash(); - create_dir_all(format!("target/{commit_hash}")).unwrap(); + create_dir_all(format!("target/regression_artifacts/{commit_hash}")).unwrap(); let raw_profile = Self { test_name: test_name.to_owned(), @@ -143,7 +166,10 @@ mod tests { } fn path(&self) -> String { - format!("target/{}/{}.raw", self.commit_hash, self.test_name) + format!( + "target/regression_artifacts/{}/{}.raw", + self.commit_hash, self.test_name + ) } // Returns the annotated profile associated with a raw profile @@ -154,8 +180,9 @@ mod tests { /// Return the raw profiles for `test_name` in "git" order. `tuple.0` is older than `tuple.1` /// /// This method will panic if there are not two profiles. + /// This method will also panic if both commits are on different logs (not mainline). fn query(test_name: &str) -> (RawProfile, RawProfile) { - let pattern = format!("target/**/*{}.raw", test_name); + let pattern = format!("target/regression_artifacts/**/*{}.raw", test_name); let raw_files: Vec = glob::glob(&pattern) .expect("Failed to read glob pattern") .filter_map(Result::ok) @@ -167,18 +194,28 @@ mod tests { test_name: test_name.to_string(), commit_hash: git::extract_commit_hash(&raw_files[0]), }; - let profile2 = RawProfile { test_name: test_name.to_string(), commit_hash: git::extract_commit_hash(&raw_files[1]), }; - if git::is_older_commit(&profile1.commit_hash, &profile2.commit_hash) { - (profile1, profile2) - } else if git::is_older_commit(&profile2.commit_hash, &profile1.commit_hash) { - (profile2, profile1) + // xor returns true if exactly one commit is mainline + if git::is_mainline(&profile1.commit_hash) ^ git::is_mainline(&profile2.commit_hash) { + // Return the mainline as first commit + if git::is_mainline(&profile1.commit_hash) { + (profile1, profile2) + } else { + (profile2, profile1) + } } else { - panic!("The commits are not in the same log"); + // Neither or both profiles are on the mainline, so return the older one first + if git::is_older_commit(&profile1.commit_hash, &profile2.commit_hash) { + (profile1, profile2) + } else if git::is_older_commit(&profile2.commit_hash, &profile1.commit_hash) { + (profile2, profile1) + } else { + panic!("The commits are not in the same log, are identical, or there are not two commits available"); + } } } } @@ -211,7 +248,10 @@ mod tests { } fn path(&self) -> String { - format!("target/{}/{}.annotated", self.commit_hash, self.test_name) + format!( + "target/regression_artifacts/{}/{}.annotated", + self.commit_hash, self.test_name + ) } fn instruction_count(&self) -> i64 { @@ -240,7 +280,7 @@ mod tests { assert_command_success(diff_output.clone()); // write the diff to disk - create_dir_all(format!("target/diff")).unwrap(); + create_dir_all("target/regression_artifacts/diff").unwrap(); let diff_content = String::from_utf8(diff_output.stdout) .expect("Invalid UTF-8 in cg_annotate --diff output"); write(diff_profile.path(), diff_content).expect("Failed to write to file"); @@ -249,7 +289,7 @@ mod tests { } fn path(&self) -> String { - format!("target/diff/{}.diff", self.test_name) + format!("target/regression_artifacts/diff/{}.diff", self.test_name) } fn assert_performance(&self, max_diff: f64) {