Skip to content

Commit 2a15f08

Browse files
committed
Update check regression script
This commit updates the regression check script to skip the initial value in all BenchmarkTimes array of the benchmark results json file to calculate a new p90. We use this new p90 to identify regression, this change was made to combat the skewed p90 metrics we were seeing due to the slow starts of the benchmark pull times which were affecting the overall regression comparison. Skipping the first value allows us to have a more uniform comparison, remove github environment noise and we'd be able to identify true regression in our code Signed-off-by: Arjun Raja Yogidas <[email protected]>
1 parent 8c6880c commit 2a15f08

File tree

1 file changed

+20
-9
lines changed

1 file changed

+20
-9
lines changed

scripts/check_regression.sh

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ compare_stat_p90() {
2222
local current_value="$2"
2323
local stat_name="$3"
2424

25-
# Calculate 110% of the past value
25+
# Calculate 125% of the past value
2626
local threshold=$(calculate_threshold "$past_value")
2727

2828
# Compare the current value with the threshold
29-
if (( $(awk 'BEGIN {print ("'"$current_value"'" > "'"$threshold"'")}') )); then
30-
echo "ERROR: $stat_name - Current P90 value ($current_value) exceeds the 110% threshold ($threshold) of the past P90 value ($past_value)"
29+
if (( $(echo "$current_value > $threshold" |bc -l) )); then
30+
echo "ERROR: $stat_name - Current P90 value ($current_value) exceeds the 125% threshold ($threshold) of the past P90 value ($past_value)"
3131
return 1
3232
fi
3333

@@ -36,7 +36,18 @@ compare_stat_p90() {
3636

3737
calculate_threshold() {
3838
local past_value="$1"
39-
awk -v past="$past_value" 'BEGIN { print past * 1.1 }'
39+
awk -v past="$past_value" 'BEGIN { print past * 1.25 }'
40+
}
41+
42+
calculate_p90_after_skip() {
43+
local times_array="$1"
44+
local num_entries=$(echo "$times_array" | jq 'length')
45+
local times=$(echo "$times_array" | jq -r '.[1:] | .[]')
46+
local sorted_times=$(echo "$times" | tr '\n' ' ' | xargs -n1 | sort -g)
47+
local index=$((num_entries * 90 / 100))
48+
49+
local p90=$(echo "$sorted_times" | sed -n "${index}p")
50+
echo "$p90"
4051
}
4152

4253
# Loop through each object in past.json and compare P90 values with current.json for all statistics
@@ -52,8 +63,10 @@ compare_p90_values() {
5263
for test_name in $test_names; do
5364
echo "Checking for regression in '$test_name'"
5465
for stat_name in "fullRunStats" "pullStats" "lazyTaskStats" "localTaskStats"; do
55-
local past_p90=$(echo "$past_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.pct90')
56-
local current_p90=$(echo "$current_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.pct90')
66+
local past_p90_array=$(echo "$past_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.BenchmarkTimes')
67+
local past_p90=$(calculate_p90_after_skip "$past_p90_array")
68+
local current_p90_array=$(echo "$current_json" | jq -r --arg test "$test_name" '.benchmarkTests[] | select(.testName == $test) | .'"$stat_name"'.BenchmarkTimes')
69+
local current_p90=$(calculate_p90_after_skip "$current_p90_array")
5770

5871
# Call the compare_stat_p90 function
5972
compare_stat_p90 "$past_p90" "$current_p90" "$stat_name" || regression_detected=1
@@ -64,8 +77,6 @@ compare_p90_values() {
6477
return $regression_detected
6578
}
6679

67-
# ... (remaining code)
68-
6980
# Call compare_p90_values and store the exit code in a variable
7081
compare_p90_values "$past_data" "$current_data"
7182
exit_code=$?
@@ -78,4 +89,4 @@ else
7889
fi
7990

8091
# Set the final exit code to indicate if any regression occurred
81-
exit $exit_code
92+
exit $exit_code

0 commit comments

Comments
 (0)