Add performance regression tests in CI (#4701)

kaukabrizvi · web-flow · commit 87f4a0585dc3 · 2024-08-19T10:52:27.000-07:00
diff --git a/.github/workflows/regression_ci.yml 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
@@ -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
@@ -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<String> = 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) {
