Benchmarks workflow prints diff when changes found

.github/workflows/benchmarks.yml

@@ -74,8 +74,8 @@ jobs:
       - id: generate-matrix
         run: echo "benchmarks-matrix=$(curl -s --retry 3 https://raw.githubusercontent.com/apple/swift-nio/main/scripts/generate_matrix.sh | bash)" >> "$GITHUB_OUTPUT"
         env:
-          MATRIX_LINUX_COMMAND: "swift package --package-path ${{ inputs.benchmark_package_path }} ${{ inputs.swift_package_arguments }} benchmark baseline check --check-absolute-path ${{ inputs.benchmark_package_path }}/Thresholds/${SWIFT_VERSION}/"
-          MATRIX_LINUX_SETUP_COMMAND: "swift --version && apt-get update -y -q && apt-get install -y -q libjemalloc-dev"
+          MATRIX_LINUX_COMMAND: "curl -s https://raw.githubusercontent.com/apple/swift-nio/main/scripts/check_benchmark_thresholds.sh | BENCHMARK_PACKAGE_PATH=${{ inputs.benchmark_package_path }} bash"
+          MATRIX_LINUX_SETUP_COMMAND: "swift --version && apt-get update -y -q && apt-get install -y -q curl libjemalloc-dev && git config --global --add safe.directory /$(basename ${{ github.workspace }})"
           MATRIX_LINUX_5_9_ENABLED: ${{ inputs.linux_5_9_enabled }}
           MATRIX_LINUX_5_10_ENABLED: ${{ inputs.linux_5_10_enabled }}
           MATRIX_LINUX_6_0_ENABLED: ${{ inputs.linux_6_0_enabled }}

dev/thresholds-from-benchmark-output.sh

@@ -13,7 +13,7 @@
 ##
 ##===----------------------------------------------------------------------===##
 
-# This script allows you to consume swift package benchmark output and 
+# This script allows you to consume swift package benchmark output and
 # update JSON threshold files
 
 set -uo pipefail
@@ -22,196 +22,173 @@ log() { printf -- "** %s\n" "$*" >&2; }
 error() { printf -- "** ERROR: %s\n" "$*" >&2; }
 fatal() { error "$@"; exit 1; }
 
-pr_url="${FETCH_PR_URL:=""}"
-output_dir="${OUTPUT_DIRECTORY:=""}"
+url="${URL:=""}"
 
-if [ -z "$pr_url" ]; then
-  fatal "Pull request URL must be specified."
-fi
-
-if [ -z "$output_dir" ]; then
-  fatal "Output directory must be specified."
+if [ -z "$url" ]; then
+  fatal "Pull request or workflow run URL must be specified."
 fi
 
 # Check for required tools
 GH_BIN="${GH_BIN:-$(which gh)}" || fatal "GH_BIN unset and no gh on PATH"
 JQ_BIN="${JQ_BIN:-$(which jq)}" || fatal "JQ_BIN unset and no jq on PATH"
 YQ_BIN="${YQ_BIN:-$(which yq)}" || fatal "YQ_BIN unset and no yq on PATH"
 
-# Parsing constants
-readonly benchmark_title_name_prefix="than threshold for "
-readonly table_pipe="│"
-readonly table_current_run_heading="Current_run"
-
 fetch_checks_for_pr() {
     pr_url=$1
 
     "$GH_BIN" pr checks "$pr_url" | grep Benchmarks | grep -v Construct
 }
 
-fetch_check_logs() {
-    repo=$1
-    job=$2
-
-    # We use `gh api` rather than `gh run view --log` because of https://github.com/cli/cli/issues/5011.
-    # Look for the table outputted by the benchmarks tool if there is a discrepancy
-    "$GH_BIN" api "/repos/${repo}/actions/jobs/${job}/logs" | grep -e "$benchmark_title_name_prefix" -e "$table_pipe"
+parse_url() {
+    workflow_url=$1
+    # https://github.com/apple/swift-nio/actions/runs/15269806473
+    # https://github.com/apple/swift-nio/pull/3257
+    if echo "$url" | grep -q "pull"; then
+        type="PR"
+    elif echo "$url" | grep -q "actions/runs"; then
+        type="run"
+    else
+        fatal "Cannot parse URL: $url"
+    fi
+    echo "$url" | awk -v type="$type" -F '/' '{print $4, $5, type}'
 }
 
 parse_check() {
-    check_line=$1
+    type=$1
+    check_line=$2
 
-    # Something like:
-    # Benchmarks / Benchmarks / Linux (5.10)	pass	4m21s	https://github.com/apple/swift-nio-ssl/actions/runs/13793783082/job/38580234681
-    echo "$check_line" | sed -E 's/.*\(([^\)]+)\).*github\.com\/(.*)\/actions\/runs\/[0-9]+\/job\/([0-9]+)/\1 \2 \3/g'
-}
-
-parse_benchmark_header() {
-    line=$1
-
-    echo "$line" | grep "$table_pipe" | grep "$table_current_run_heading" > /dev/null || fatal "Unexpected line format when expecting benchmark table header: $line"
-
-    # Something like:
-    # │ Malloc (total) (#, %)                    │   p90 threshold │     Current_run │    Difference % │     Threshold % │
-    benchmark_header_name="$(echo "$line" | awk '{split($0,a,"│"); print a[2]}' | xargs)"
-
-    case "$benchmark_header_name" in
-        "Malloc (total) (#, Δ)")
-            benchmark_metric_name="mallocCountTotal"
-            scale=1
+    case "$type" in
+    "PR")
+        parse_check_for_pr "$check_line"
         ;;
 
-        "Malloc (total) (K, Δ)")
-            benchmark_metric_name="mallocCountTotal"
-            scale=1000
+    "run")
+        parse_check_for_workflow "$check_line"
         ;;
 
-        "Malloc / free Δ (#, Δ)")
-            benchmark_metric_name="memoryLeaked"
-            scale=1
+    *)
+        fatal "Unknown type '$type'"
+        # Add error handling commands here
         ;;
     esac
-    echo "$benchmark_metric_name" "$scale"
 }
 
-parse_benchmark_value() {
-    line=$1
-    echo "$line" | grep "$table_pipe" | grep -v "$table_current_run_heading" > /dev/null || fatal "Unexpected line format when expecting benchmark table values: $line"
-
+parse_check_for_workflow() {
+    check_line=$1
+
     # Something like:
-    # │ p90                                      │            8000 │            6000 │           -2000 │               0 │
-    echo "$line" | awk '{split($0,a,"│"); print a[4]}'
+    # ✓ Benchmarks / Benchmarks / Linux (5.10) in 5m10s (ID 42942543009)
+    echo "$check_line" | sed -En 's/.*ID ([0-9][0-9]*).*/\1/p'
 }
 
-parse_benchmark_title() {
-    line=$1 
-    echo "$line" | grep -q "$benchmark_title_name_prefix" || fatal "Unexpected line format when expecting threshold title: $line"
+parse_check_for_pr() {
+    check_line=$1
 
     # Something like:
-    # Deviations better than threshold for NIOCoreBenchmarks:WaitOnPromise
-    benchmark_title="$(echo "$line" | sed -E 's/.*than threshold for ([^:]*)\:(.*)$/\1.\2/g')"
-    threshold_file="${benchmark_title}.p90.json"
+    # Benchmarks / Benchmarks / Linux (5.10)	pass	4m21s	https://github.com/apple/swift-nio-ssl/actions/runs/13793783082/job/38580234681
+    echo "$check_line" | sed -E 's/.*\(([^\)]+)\).*github\.com\/(.*)\/actions\/runs\/[0-9]+\/job\/([0-9]+)/\3/g'
+}
+
+parse_workflow_url() {
+    workflow_url=$1
+    # https://github.com/apple/swift-nio/actions/runs/15269806473
+    echo "$workflow_url" | awk -F '/' '{print $8}'
+}
 
-    echo "$threshold_file"
+fetch_checks_for_workflow() {
+    repo=$1
+    run=$2
+
+    "$GH_BIN" --repo "$repo" run view "$run" | grep Benchmarks | grep ID | grep -v Construct
 }
 
-# States for the output parsing state machine
-readonly STATE_EXPECTING_BENCHMARK_TITLE=0
-readonly STATE_EXPECTING_BENCHMARK_HEADER_ROW=1
-readonly STATE_EXPECTING_BENCHMARK_VALUE_ROW=2
-readonly STATE_PROCESSED_VALUE_ROW=3
+fetch_check_logs() {
+    repo=$1
+    job=$2
 
-parse_benchmarks_output() {
+    log "Pulling logs for $repo job $job"
+    # We use `gh api` rather than `gh run view --log` because of https://github.com/cli/cli/issues/5011.
+    # Look for the table outputted by the benchmarks tool if there is a discrepancy
+    "$GH_BIN" api "/repos/${repo}/actions/jobs/${job}/logs"
+}
+
+scrape_benchmarks_output_diff() {
     lines=$1
     job=$2
-    swift_version=$3
-
-    # We can ignore the percentage difference rows (with '#, %' in the title)
-    lines="$(echo "$lines" | sed -e '/#, %/,+1d')"
-
-    state=$STATE_EXPECTING_BENCHMARK_TITLE
-    while read -r line; do
-        case "$state" in
-            "$STATE_EXPECTING_BENCHMARK_TITLE")
-                output_file="$(parse_benchmark_title "$line")"
-                output_path="${output_dir}/${swift_version}/${output_file}"
-                output=""
-                state=$STATE_EXPECTING_BENCHMARK_HEADER_ROW
-                ;;
-
-            "$STATE_EXPECTING_BENCHMARK_HEADER_ROW")
-                read -r benchmark_metric_name scale <<< "$(parse_benchmark_header "$line")"
-                output="${output}\"${benchmark_metric_name}\":"
-
-                state=$STATE_EXPECTING_BENCHMARK_VALUE_ROW
-                ;;
-
-            "$STATE_EXPECTING_BENCHMARK_VALUE_ROW")
-                benchmark_metric_value=$(parse_benchmark_value "$line")
-                benchmark_metric_scaled_value="$(( benchmark_metric_value * scale ))"
-
-                output="${output}${benchmark_metric_scaled_value},"
-
-                state=$STATE_PROCESSED_VALUE_ROW
-                ;;
-
-            "$STATE_PROCESSED_VALUE_ROW")
-                # next up is either another metric in the same benchmark or a new benchmark
-                if [[ "$line" =~ $table_current_run_heading ]]; then
-                    # this is a BENCHMARK_HEADER_ROW
-                    output="${output}\"$(parse_benchmark_header "$line")\":"
-                    state=$STATE_EXPECTING_BENCHMARK_VALUE_ROW
-                else
-                    # this is a new BENCHMARK_TITLE, finish-up the old benchmarK
-                    write_output "$output" "$output_path" "$job"
-
-                    # reset state and output JSON buffer
-                    output=""
-                    state=$STATE_EXPECTING_BENCHMARK_HEADER_ROW
-
-                    # new benchmark means a new output file
-                    output_file="$(parse_benchmark_title "$line")"
-                    output_path="${output_dir}/${swift_version}/${output_file}"
-                fi
-                ;;
-
-            *)
-                fatal "Unexpected state: $state"
-                ;;
-        esac
-    done <<< "$lines"
-
-    write_output "$output" "$output_path" "$job"
+
+    log "Scraping diff from log"
+    # Trim out everything but the diff
+    git_diff="$(echo "$lines" | sed '1,/=== BEGIN DIFF ===/d' | sed '/Post job cleanup/,$d' | sed 's/^[0-9][0-9][0-9][0-9]-.*Z //')"
+
+    echo "$git_diff"
 }
 
-write_output() {
-    output=$1
-    output_path=$2
-    job=$3
+apply_benchmarks_output_diff() {
+    git_diff=$1
+
+    if [ -z "$git_diff" ]; then
+        log "No git diff found to apply"
+        return
+    fi
 
-    log "Updating: $output_path job:$job"
-    # go via `yq` to clean up the trailing comma
-    echo "{$output}" | "$YQ_BIN" . | "$JQ_BIN" . > "$output_path"
+    log "Applying git diff"
+    echo "$git_diff" | git apply
+}
+
+filter_thresholds_to_allowlist() {
+    git_diff=$1
+
+    log "Filtering thresholds"
+    for thresholds_file in $(echo "$git_diff" | grep "+++" | sed -E 's/^\+\+\+ b\/(.*)$/\1/g'); do
+        jq 'with_entries(select(.key | in({mallocCountTotal:1, memoryLeaked:1})))' "$thresholds_file" > temp.json && mv -f temp.json "$thresholds_file"
+    done
 }
 
 ####
 
-check_lines="$(fetch_checks_for_pr "$pr_url")"
+read -r repo_org repo_name type <<< "$(parse_url "$url")"
+repo="$repo_org/$repo_name"
+log "URL is of type $type in $repo"
 
-if [ -z "$check_lines" ]; then
-    fatal "Could not locate benchmark checks on PR: $pr_url"
-fi
+case "$type" in
+"PR")
+    log "Fetching checks for $url"
+    check_lines="$(fetch_checks_for_pr "$url")"
+
+    if [ -z "$check_lines" ]; then
+        fatal "Could not locate benchmark checks on PR: $url"
+    fi
+    ;;
+
+"run")
+    run="$(parse_workflow_url "$url")"
+
+    log "Fetching checks for $repo run $run"
+    check_lines="$(fetch_checks_for_workflow "$repo" "$run")"
+
+    if [ -z "$check_lines" ]; then
+        fatal "Could not locate benchmark checks on workflow run: $url"
+    fi
+    ;;
+
+*)
+    fatal "Unknown type '$type'"
+    ;;
+esac
 
 while read -r check_line; do
-    read -r swift_version repo job <<< "$(parse_check "$check_line")"
+    job="$(parse_check "$type" "$check_line")"
 
     lines=$(fetch_check_logs "$repo" "$job")
 
     if [ -z "$lines" ]; then
-        log "Nothing to update: $repo $swift_version job:$job"
+        log "Nothing to update: $repo job: $job"
         continue
     fi
 
-    parse_benchmarks_output "$lines" "$job" "$swift_version"
+    git_diff="$(scrape_benchmarks_output_diff "$lines" "$job")"
+    apply_benchmarks_output_diff "$git_diff"
+
+    filter_thresholds_to_allowlist "$git_diff"
 
 done <<< "$check_lines"

scripts/check_benchmark_thresholds.sh

@@ -0,0 +1,46 @@
+#!/bin/bash
+##===----------------------------------------------------------------------===##
+##
+## This source file is part of the SwiftNIO open source project
+##
+## Copyright (c) 2025 Apple Inc. and the SwiftNIO project authors
+## Licensed under Apache License v2.0
+##
+## See LICENSE.txt for license information
+## See CONTRIBUTORS.txt for the list of SwiftNIO project authors
+##
+## SPDX-License-Identifier: Apache-2.0
+##
+##===----------------------------------------------------------------------===##
+
+set -uo pipefail
+
+log() { printf -- "** %s\n" "$*" >&2; }
+error() { printf -- "** ERROR: %s\n" "$*" >&2; }
+fatal() { error "$@"; exit 1; }
+
+# Parameter environment variables
+if [ -z "$SWIFT_VERSION" ]; then
+  fatal "SWIFT_VERSION must be specified."
+fi
+
+benchmark_package_path="${BENCHMARK_PACKAGE_PATH:-"."}"
+swift_version="${SWIFT_VERSION:-""}"
+
+# Any parameters to the script are passed along to SwiftPM
+swift_package_arguments=("$@")
+
+#"swift package --package-path ${{ inputs.benchmark_package_path }} ${{ inputs.swift_package_arguments }} benchmark baseline check --check-absolute-path ${{ inputs.benchmark_package_path }}/Thresholds/${SWIFT_VERSION}/"
+swift package --package-path "$benchmark_package_path" "${swift_package_arguments[@]}" benchmark thresholds check --format metricP90AbsoluteThresholds --path "${benchmark_package_path}/Thresholds/${swift_version}/"
+rc="$?"
+
+# Benchmarks are unchanged, nothing to recalculate
+if [[ "$rc"  == 0 ]]; then
+  exit 0
+fi
+
+log "Recalculating thresholds..."
+
+swift package --package-path "$benchmark_package_path" "${swift_package_arguments[@]}" benchmark thresholds update --format metricP90AbsoluteThresholds --path "${benchmark_package_path}/Thresholds/${swift_version}/"
+echo "=== BEGIN DIFF ==="  # use echo, not log for clean output to be scraped
+git diff --exit-code HEAD