-
Notifications
You must be signed in to change notification settings - Fork 60
Establish performance baselines and regression detection #3441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9cd5933
1ba4889
df4f09d
7594bbf
d39ae20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,6 @@ jobs: | |
| name: Stress Benchmark | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| continue-on-error: true | ||
| env: | ||
| # Tuned for 4 vCPU / 16 GB CI runners to complete within 5 minutes. | ||
| # Code defaults are 10 components / 35 workers. | ||
|
|
@@ -73,11 +72,23 @@ jobs: | |
|
|
||
| - name: Run stress benchmark | ||
| id: bench | ||
| continue-on-error: true | ||
| run: | | ||
| set -o pipefail | ||
| cd benchmark/stress | ||
| ./stress 2>benchmark-stderr.txt | tee benchmark-output.txt | ||
|
|
||
| - name: Compare against baseline | ||
| id: compare | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] error-handling The compare step is silently skipped when the benchmark fails (continue-on-error: true on the bench step), with no indication in the job summary. |
||
| if: steps.bench.outcome == 'success' | ||
| run: | | ||
| cd benchmark/stress | ||
| if [[ -f baseline.json ]]; then | ||
| ./compare.sh benchmark-output.txt | ||
| else | ||
| echo "No baseline found, skipping comparison." | ||
| fi | ||
|
|
||
| - name: Write job summary | ||
| if: always() | ||
| run: | | ||
|
|
@@ -103,25 +114,55 @@ jobs: | |
| exit 0 | ||
| fi | ||
|
|
||
| ns_op=$(echo "$line" | grep -oP '[\d.]+ ns/op' | awk '{print $1}') | ||
| peak_rss=$(echo "$line" | grep -oP '[\d.]+ peak-RSS-bytes' | awk '{print $1}') | ||
| alloc=$(echo "$line" | grep -oP '[\d.]+ allocated-bytes/op' | awk '{print $1}') | ||
| heap=$(echo "$line" | grep -oP '[\d.]+ heap-bytes-from-system' | awk '{print $1}') | ||
| read -r ns_op peak_rss alloc heap < <(BENCH_LINE="$line" python3 -c " | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] scope-creep The PR refactors existing grep/awk benchmark output parsing to python3 in the job summary step, which is tangential to the stated goal but establishes consistency with compare.sh. |
||
| import os, re | ||
| line = os.environ['BENCH_LINE'] | ||
| def val(p): | ||
| m = re.search(p, line) | ||
| return m.group(1) if m else '0' | ||
| print(val(r'([\d.]+)\s+ns/op'), val(r'([\d.]+)\s+peak-RSS-bytes'), val(r'([\d.]+)\s+allocated-bytes/op'), val(r'([\d.]+)\s+heap-bytes-from-system')) | ||
| ") | ||
|
|
||
| secs=$(awk -v val="${ns_op:-0}" 'BEGIN {printf "%.1f", val / 1000000000}') | ||
| rss_mb=$(awk -v val="${peak_rss:-0}" 'BEGIN {printf "%.0f", val / 1048576}') | ||
| alloc_mb=$(awk -v val="${alloc:-0}" 'BEGIN {printf "%.0f", val / 1048576}') | ||
| heap_mb=$(awk -v val="${heap:-0}" 'BEGIN {printf "%.0f", val / 1048576}') | ||
|
|
||
| has_baseline=false | ||
| if [[ -f benchmark/stress/baseline.json ]]; then | ||
| has_baseline=true | ||
| bl_rss=$(python3 -c "import json; print(json.load(open('benchmark/stress/baseline.json'))['peak_rss_bytes'])") | ||
| bl_ns=$(python3 -c "import json; print(json.load(open('benchmark/stress/baseline.json'))['ns_per_op'])") | ||
| bl_rss_mb=$(awk -v val="$bl_rss" 'BEGIN {printf "%.0f", val / 1048576}') | ||
| bl_secs=$(awk -v val="$bl_ns" 'BEGIN {printf "%.1f", val / 1000000000}') | ||
| rss_change=$(awk -v cur="$peak_rss" -v base="$bl_rss" 'BEGIN {printf "%+.1f", ((cur - base) / base) * 100}') | ||
| time_change=$(awk -v cur="$ns_op" -v base="$bl_ns" 'BEGIN {printf "%+.1f", ((cur - base) / base) * 100}') | ||
| fi | ||
|
|
||
| { | ||
| echo "## Stress Benchmark" | ||
| echo "" | ||
| echo "| Metric | Value | Description |" | ||
| echo "|--------|-------|-------------|" | ||
| echo "| Components | ${EC_STRESS_COMPONENTS} | Snapshot components validated |" | ||
| echo "| Workers | ${EC_STRESS_WORKERS} | Parallel validation workers |" | ||
| echo "| Execution time | ${secs}s | Wall-clock time per iteration |" | ||
| echo "| Peak RSS | ${rss_mb} MB | Max physical memory used |" | ||
| echo "| Allocated memory | ${alloc_mb} MB | Total Go heap allocations |" | ||
| echo "| Heap from system | ${heap_mb} MB | Heap memory requested from OS |" | ||
| if [[ "$has_baseline" == "true" ]]; then | ||
| echo "| Metric | Current | Baseline | Change | Description |" | ||
| echo "|--------|---------|----------|--------|-------------|" | ||
| echo "| Components | ${EC_STRESS_COMPONENTS} | | | Snapshot components validated |" | ||
| echo "| Workers | ${EC_STRESS_WORKERS} | | | Parallel validation workers |" | ||
| echo "| Execution time | ${secs}s | ${bl_secs}s | ${time_change}% | Wall-clock time per iteration |" | ||
| echo "| Peak RSS | ${rss_mb} MB | ${bl_rss_mb} MB | ${rss_change}% | Max physical memory used |" | ||
| echo "| Allocated memory | ${alloc_mb} MB | | | Total Go heap allocations |" | ||
| echo "| Heap from system | ${heap_mb} MB | | | Heap memory requested from OS |" | ||
| else | ||
| echo "| Metric | Value | Description |" | ||
| echo "|--------|-------|-------------|" | ||
| echo "| Components | ${EC_STRESS_COMPONENTS} | Snapshot components validated |" | ||
| echo "| Workers | ${EC_STRESS_WORKERS} | Parallel validation workers |" | ||
| echo "| Execution time | ${secs}s | Wall-clock time per iteration |" | ||
| echo "| Peak RSS | ${rss_mb} MB | Max physical memory used |" | ||
| echo "| Allocated memory | ${alloc_mb} MB | Total Go heap allocations |" | ||
| echo "| Heap from system | ${heap_mb} MB | Heap memory requested from OS |" | ||
| fi | ||
| if [[ "${{ steps.compare.outcome }}" == "failure" ]]; then | ||
| echo "" | ||
| echo "> **⚠️ Performance regression detected.** Update the baseline with \`make generate-baseline\` if this is expected." | ||
| fi | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -197,6 +197,28 @@ benchmark_data: benchmark/simple/data.tar.gz ## Prepare data for benchmark | |
| .PHONY: benchmark | ||
| benchmark: benchmark_simple ## Run benchmarks | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] naming-convention Target name generate-baseline uses hyphens; sibling benchmark targets use underscores (benchmark_simple, benchmark_data). |
||
| .PHONY: generate-baseline | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] logic-error The generate-baseline target runs go run . without setting EC_STRESS_WORKERS or EC_STRESS_COMPONENTS. The Go benchmark defaults to 35 workers, but the inline Python records workers=10 (bash fallback ${EC_STRESS_WORKERS:-10}). CI uses EC_STRESS_WORKERS=10 for comparison. This produces a baseline with incorrect metadata and metrics from a different workload than CI, making regression detection unreliable. Suggested fix: Set EC_STRESS_COMPONENTS=10 EC_STRESS_WORKERS=10 in the generate-baseline recipe to match CI values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] inline-scripting-convention The generate-baseline target embeds ~15 lines of inline Python. The codebase convention is to use standalone scripts for complex logic (e.g., prepare_data.sh, push_data.sh). |
||
| generate-baseline: benchmark/stress/data.tar.gz ## Generate stress benchmark baseline | ||
| @cd benchmark/stress && \ | ||
| go run . 2>benchmark-stderr.txt | tee benchmark-output.txt && \ | ||
| python3 -c "\ | ||
| import re, json, sys; \ | ||
| line = [l for l in open('benchmark-output.txt') if l.startswith('BenchmarkStress')]; \ | ||
| line or sys.exit('No BenchmarkStress results found'); \ | ||
| line = line[0]; \ | ||
| def val(p): \ | ||
| m = re.search(p, line); \ | ||
| return m.group(1) if m else ''; \ | ||
| ns = val(r'([\d.]+)\s+ns/op'); rss = val(r'([\d.]+)\s+peak-RSS-bytes'); \ | ||
| (ns and rss) or sys.exit('Failed to parse benchmark metrics'); \ | ||
| json.dump({'peak_rss_bytes': int(float(rss)), 'ns_per_op': int(float(ns)), \ | ||
| 'components': int('$${EC_STRESS_COMPONENTS:-10}'), 'workers': int('$${EC_STRESS_WORKERS:-10}'), \ | ||
| 'commit': '$(shell git rev-parse --short HEAD)', 'date': '$(shell date -u +%Y-%m-%d)', \ | ||
| 'go_version': '$(shell go env GOVERSION | sed "s/^go//")' \ | ||
| }, open('baseline.json','w'), indent=2); print()" && \ | ||
| rm -f benchmark-output.txt benchmark-stderr.txt && \ | ||
| echo "Baseline written to benchmark/stress/baseline.json" | ||
|
|
||
| .PHONY: tools-ci | ||
| tools-ci: ## Ensure all tools build cleanly | ||
| @echo "• tkn:" && \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "peak_rss_bytes": 2250485760, | ||
| "ns_per_op": 2567888013, | ||
| "components": 10, | ||
| "workers": 10, | ||
| "commit": "fc37eb13", | ||
| "date": "2026-08-11", | ||
| "go_version": "1.26.3" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| #!/bin/bash | ||
| # Copyright The Conforma Contributors | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| # Compares current benchmark results against a stored baseline and exits | ||
| # non-zero if any metric regresses beyond the configured threshold. | ||
| set -o errexit | ||
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| BASELINE="${SCRIPT_DIR}/baseline.json" | ||
| THRESHOLDS="${SCRIPT_DIR}/thresholds.json" | ||
| BENCHMARK_OUTPUT="${1:-${SCRIPT_DIR}/benchmark-output.txt}" | ||
|
|
||
| if [[ ! -f "$BASELINE" ]]; then | ||
| echo "No baseline found, skipping comparison." | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [[ ! -f "$THRESHOLDS" ]]; then | ||
| echo "No thresholds file found, skipping comparison." | ||
| exit 0 | ||
| fi | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case If benchmark output contains multiple BenchmarkStress lines, grep captures all; Python re.search matches only the first. Works correctly for expected single-result case but is fragile. |
||
|
|
||
| if [[ ! -f "$BENCHMARK_OUTPUT" ]]; then | ||
| echo "No benchmark output found at ${BENCHMARK_OUTPUT}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| line=$(grep '^BenchmarkStress' "$BENCHMARK_OUTPUT" || true) | ||
| if [[ -z "$line" ]]; then | ||
| echo "No BenchmarkStress results found in output." | ||
| exit 1 | ||
| fi | ||
|
dheerajodha marked this conversation as resolved.
|
||
|
|
||
| read -r current_ns current_rss baseline_ns baseline_rss threshold_rss threshold_time < <( | ||
| BENCH_LINE="${line}" BASELINE_PATH="${BASELINE}" THRESHOLDS_PATH="${THRESHOLDS}" python3 -c " | ||
| import json, os, re, sys | ||
| line = os.environ['BENCH_LINE'] | ||
| def extract(pattern): | ||
| m = re.search(pattern, line) | ||
| return m.group(1) if m else '' | ||
| ns = extract(r'([\d.]+)\s+ns/op') | ||
| rss = extract(r'([\d.]+)\s+peak-RSS-bytes') | ||
| if not ns or not rss: | ||
| print('Failed to parse benchmark metrics from output.', file=sys.stderr) | ||
| sys.exit(1) | ||
| b = json.load(open(os.environ['BASELINE_PATH'])) | ||
| t = json.load(open(os.environ['THRESHOLDS_PATH'])) | ||
| print(ns, rss, b['ns_per_op'], b['peak_rss_bytes'], t['peak_rss_percent'], t['ns_per_op_percent']) | ||
| " | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge-case Zero-value guard uses misleading awk variable names (b for $baseline_rss, t for $baseline_ns). |
||
| ) | ||
|
|
||
| if awk -v b="$baseline_rss" -v t="$baseline_ns" 'BEGIN {exit !(b==0 || t==0)}'; then | ||
| echo "Baseline contains zero values, cannot compute regression." | ||
| exit 1 | ||
| fi | ||
|
|
||
| rss_change=$(awk -v cur="$current_rss" -v base="$baseline_rss" 'BEGIN {printf "%.1f", ((cur - base) / base) * 100}') | ||
| time_change=$(awk -v cur="$current_ns" -v base="$baseline_ns" 'BEGIN {printf "%.1f", ((cur - base) / base) * 100}') | ||
|
|
||
| baseline_rss_mb=$(awk -v val="$baseline_rss" 'BEGIN {printf "%.0f", val / 1048576}') | ||
| current_rss_mb=$(awk -v val="$current_rss" 'BEGIN {printf "%.0f", val / 1048576}') | ||
| baseline_secs=$(awk -v val="$baseline_ns" 'BEGIN {printf "%.1f", val / 1000000000}') | ||
| current_secs=$(awk -v val="$current_ns" 'BEGIN {printf "%.1f", val / 1000000000}') | ||
|
|
||
| echo "" | ||
| echo "=== Benchmark Comparison ===" | ||
| echo "" | ||
| printf "%-20s %10s %10s %10s %10s\n" "Metric" "Baseline" "Current" "Change" "Threshold" | ||
| printf "%-20s %10s %10s %9s%% %9s%%\n" "Peak RSS" "${baseline_rss_mb} MB" "${current_rss_mb} MB" "$rss_change" "$threshold_rss" | ||
| printf "%-20s %10s %10s %9s%% %9s%%\n" "Execution time" "${baseline_secs}s" "${current_secs}s" "$time_change" "$threshold_time" | ||
| echo "" | ||
|
|
||
| failed=0 | ||
|
|
||
| rss_exceeded=$(awk -v change="$rss_change" -v thresh="$threshold_rss" 'BEGIN {print (change > thresh) ? 1 : 0}') | ||
| time_exceeded=$(awk -v change="$time_change" -v thresh="$threshold_time" 'BEGIN {print (change > thresh) ? 1 : 0}') | ||
|
|
||
| if [[ "$rss_exceeded" == "1" ]]; then | ||
| echo "FAIL: Peak RSS regressed by ${rss_change}% (threshold: ${threshold_rss}%)" | ||
| failed=1 | ||
| fi | ||
|
|
||
| if [[ "$time_exceeded" == "1" ]]; then | ||
| echo "FAIL: Execution time regressed by ${time_change}% (threshold: ${threshold_time}%)" | ||
| failed=1 | ||
| fi | ||
|
|
||
| if [[ "$failed" == "0" ]]; then | ||
| echo "PASS: No regressions detected." | ||
| fi | ||
|
|
||
| exit "$failed" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "peak_rss_percent": 15, | ||
| "ns_per_op_percent": 20 | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.