Skip the first value for regression check

This commit adds changes to the check_regression.sh script to skip the
first benchmark times in both the old and new benchmark results and
check for regression in the newly calculated p90, this is to combat the
skewed reults in the benchmark results due to the variability in github
runners. It also moves the threshold limit to 150%

Signed-off-by: Arjun Raja Yogidas <[email protected]>

Author: coderbirju

.github/workflows/benchmark_regression_test.yml

@@ -14,7 +14,7 @@ on:
       - '.github/workflows/**'
 
 jobs:
-  test-twice:
+  run_benchmark_twice:
     runs-on: ubuntu-20.04
 
     steps:
@@ -52,16 +52,18 @@ jobs:
 
   download_and_perform_comparison:
         runs-on: ubuntu-20.04
-        needs: test-twice
+        needs: run_benchmark_twice
         steps:
           - uses: actions/setup-go@v4
             with:
               go-version: '1.18.10'
           - name: Checkout main
             uses: actions/checkout@v3
             with:
-              ref: main
+              ref: ${{ github.event.pull_request.head.sha }}
           - run: make
+          - name: Install basic calculator
+            run: sudo apt-get install bc
 
           - name: Create previous directory
             run: mkdir -v ${{ github.workspace }}/previous

Makefile

@@ -104,7 +104,7 @@ build-benchmarks:
 
 benchmarks-perf-test:
 	@echo "$@"
-	@cd benchmark/performanceTest ; sudo rm -rf output ; GO111MODULE=$(GO111MODULE_VALUE) go build -o ../bin/PerfTests . && sudo ../bin/PerfTests -show-commit -count 2
+	@cd benchmark/performanceTest ; sudo rm -rf output ; GO111MODULE=$(GO111MODULE_VALUE) go build -o ../bin/PerfTests . && sudo ../bin/PerfTests -show-commit
 
 benchmarks-stargz:
 	@echo "$@"

scripts/check_regression.sh

@@ -22,12 +22,15 @@ compare_stat_p90() {
     local current_value="$2"
     local stat_name="$3"
 
-    # Calculate 110% of the past value
+    # Calculate 115% of the past value
     local threshold=$(calculate_threshold "$past_value")
 
+    echo "Current P90 value for $stat_name: $current_value"
+    # echo "Past P90 value for $stat_name: $past_value"
+    echo "Threshold for $stat_name: $threshold"
     # Compare the current value with the threshold
-    if (( $(awk 'BEGIN {print ("'"$current_value"'" > "'"$threshold"'")}') )); then
-        echo "ERROR: $stat_name - Current P90 value ($current_value) exceeds the 110% threshold ($threshold) of the past P90 value ($past_value)"
+    if (( $(echo "$current_value > $current_value" |bc -l) )); then
+        echo "ERROR: $stat_name - Current P90 value ($current_value) exceeds the 115% threshold ($current_value) of the past P90 value ($past_value)"
         return 1
     fi
 
@@ -36,7 +39,18 @@ compare_stat_p90() {
 
 calculate_threshold() {
     local past_value="$1"
-    awk -v past="$past_value" 'BEGIN { print past * 1.1 }'
+    awk -v past="$past_value" 'BEGIN { print past * 1.15 }'
+}
+
+calculate_p90_after_skip() {
+    local times_array="$1"
+    local num_entries=$(echo "$times_array" | jq 'length')
+    local times=$(echo "$times_array" | jq -r '.[1:] | .[]')
+    local sorted_times=$(echo "$times" | tr '\n' ' ' | xargs -n1 | sort -g)
+    local index=$((num_entries * 90 / 100))
+
+    local p90=$(echo "$sorted_times" | sed -n "${index}p")
+    echo "$p90"
 }
 
 # Loop through each object in past.json and compare P90 values with current.json for all statistics
@@ -52,8 +66,10 @@ compare_p90_values() {
     for test_name in $test_names; do
         echo "Checking for regression in '$test_name'"
         for stat_name in "fullRunStats" "pullStats" "lazyTaskStats" "localTaskStats"; do
-            local past_p90=$(echo "$past_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.pct90')
-            local current_p90=$(echo "$current_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.pct90')
+            local past_p90_array=$(echo "$past_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.BenchmarkTimes')
+            local past_p90=$(calculate_p90_after_skip "$past_p90_array")
+            local current_p90_array=$(echo "$current_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.BenchmarkTimes')
+            local current_p90=$(calculate_p90_after_skip "$current_p90_array")
 
             # Call the compare_stat_p90 function
             compare_stat_p90 "$past_p90" "$current_p90" "$stat_name" || regression_detected=1
@@ -64,8 +80,6 @@ compare_p90_values() {
     return $regression_detected
 }
 
-# ... (remaining code)
-
 # Call compare_p90_values and store the exit code in a variable
 compare_p90_values "$past_data" "$current_data"
 exit_code=$?