-
Notifications
You must be signed in to change notification settings - Fork 359
[Script] Provide regression test script to help benchmark regression in local env #1551
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
Conversation
…ories by backing them up before installation and restoring them afterward.
…cts, including .perf_regression and build.bak.*
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
📝 WalkthroughWalkthroughThis PR adds a new performance regression testing workflow by introducing a Bash script that orchestrates building isolated Python environments for old and new versions, running regression tests, and generating results. A .gitignore entry is added to exclude regression test artifacts. Changes
Sequence DiagramsequenceDiagram
participant User
participant Script as run_perf_regression.sh
participant Git
participant FileSystem as File System
participant OldEnv as OLD Env<br/>(main)
participant NewEnv as NEW Env<br/>(current)
participant TestRunner as test_perf_regression.py
User->>Script: Execute run_perf_regression.sh
Script->>FileSystem: Backup build directory
Script->>Git: Stash uncommitted changes
Script->>Git: Validate/upgrade remote to upstream
rect rgb(200, 220, 255)
Note over Script,NewEnv: Build NEW Environment (current branch)
Script->>FileSystem: Create NEW venv
Script->>NewEnv: Install requirements-test.txt
Script->>NewEnv: Install project
end
rect rgb(200, 220, 255)
Note over Script,OldEnv: Build OLD Environment (upstream main)
Script->>Git: Fetch upstream main
Script->>Git: Checkout upstream main
Script->>FileSystem: Create OLD venv
Script->>OldEnv: Install requirements-test.txt
Script->>OldEnv: Install project from main
end
rect rgb(220, 240, 220)
Note over Script,TestRunner: Run Performance Regression Tests
Script->>TestRunner: Execute test with OLD python path
TestRunner->>OldEnv: Run tests (baseline)
TestRunner-->>Script: Return OLD results
Script->>TestRunner: Execute test with NEW python path
TestRunner->>NewEnv: Run tests (current)
TestRunner-->>Script: Return NEW results
end
Script->>FileSystem: Generate Markdown results
Script->>FileSystem: Generate PNG plot
Script->>User: Print results
rect rgb(240, 220, 220)
Note over Script,FileSystem: Cleanup & Restore
Script->>Git: Checkout original branch
Script->>Git: Restore stashed changes
Script->>Git: Reinitialize submodules
Script->>FileSystem: Restore backed-up build directory
Script->>FileSystem: Clean temporary working directory
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
maint/scripts/run_perf_regression.sh (3)
57-70: Consider non-interactive mode handling.The
readprompt will hang or fail when the script is run non-interactively (e.g., piped input, CI environment without TTY). Consider adding a TTY check or an environment variable to skip the prompt.🔎 Suggested enhancement
# Check for uncommitted changes if [[ -n "$(git status --porcelain)" ]]; then echo "WARNING: You have uncommitted changes. They will be stashed." - read -p "Continue? [y/N] " -n 1 -r - echo - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - echo "Aborted." - exit 1 + if [[ -t 0 ]]; then + read -p "Continue? [y/N] " -n 1 -r + echo + if [[ ! $REPLY =~ ^[Yy]$ ]]; then + echo "Aborted." + exit 1 + fi + else + echo "Non-interactive mode: proceeding with stash." fi STASHED=1 git stash push -m "perf_regression_temp_stash"
127-131: Documentuvas a required dependency.The script relies on
uv(an alternative Python package manager) but doesn't check for its presence or document it as a prerequisite. Users withoutuvinstalled will see a confusing error.🔎 Suggested enhancement: add prerequisite check
Add this check near the beginning of the script (after line 16):
# Check prerequisites if ! command -v uv &> /dev/null; then echo "ERROR: 'uv' is required but not installed." echo "Install with: curl -LsSf https://astral.sh/uv/install.sh | sh" exit 1 fi
1-13: Consider addingset -o pipefailfor robustness.With only
set -e, failures in the middle of a pipeline may go unnoticed (e.g.,git remote get-url origin 2>/dev/null || echo ""). Addingpipefailensures the script exits if any command in a pipeline fails.🔎 Suggested enhancement
-set -e +set -euo pipefailNote: Adding
-u(nounset) would require ensuring all variables are initialized before use, which may need additional adjustments.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.gitignoremaint/scripts/run_perf_regression.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test for Python 3.12 with Nightly-ROCm-7.1 (on self-hosted-amd)
- GitHub Check: Test for Python 3.12 with Metal (on macos-latest)
- GitHub Check: Test for Python 3.12 with CUDA-12.8 (on self-hosted-nvidia)
🔇 Additional comments (5)
.gitignore (1)
123-125: LGTM!The
.perf_regression/ignore entry correctly pairs with the new regression script's working directory, following the existing pattern of section comments.maint/scripts/run_perf_regression.sh (4)
80-98: LGTM!The cleanup function is well-structured with proper error suppression for robustness. The
trap EXITensures cleanup runs on both success and failure, and the build directory restore logic correctly handles the backup scenario.
147-149: LGTM!The
git cleanexclusions correctly preserve the work directory (.perf_regression/), cache, and build backup. This ensures aggressive cleanup doesn't accidentally destroy important artifacts.
188-195: LGTM!Clean output formatting with paths and inline display of the markdown results. Good UX for developers running the script.
180-184: No issue here. The test scripttest_perf_regression.pyexists inmaint/scripts/at the location referenced by${SCRIPT_DIR}.Likely an incorrect or invalid review comment.
This pull request introduces a new shell script,
run_perf_regression.sh, designed to automate performance regression testing between the current branch and the latest upstreammain. The script manages environment setup, virtual environments, and result reporting, making it easier to compare performance metrics across code changes.Key features of the new performance regression script:
Performance Regression Automation
maint/scripts/run_perf_regression.sh, a comprehensive shell script that automates the process of building and testing both the current branch and upstreammainin isolated Python virtual environments, handling environment variables, and generating markdown and plot results for performance comparisons.Environment and Build Management
Result Reporting
Summary by CodeRabbit
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.