- Notifications
You must be signed in to change notification settings - Fork 3
feat: add benchmark #101
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
feat: add benchmark #101
Changes from 1 commit
89d7faa c21a796 e2859e2 60eea57 dd0a13a 2e6797a 447bd7f File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Benchmarking | ||
| | ||
| This document outlines the benchmarking process for comparing the performance of cpp-linter-hooks and mirrors-clang-format. | ||
| | ||
| ## Running the Benchmark | ||
| | ||
| ```bash | ||
| python3 testing/benchmark_hooks.py | ||
| ``` | ||
| | ||
| ## Results | ||
| | ||
| The results of the benchmarking process will be saved to `testing/benchmark_results.txt`. | ||
| | ||
| ## To Do | ||
| | ||
| - Run benchmark against a larger codebase, such as [TheAlgorithms/C-Plus-Plus](https://github.com/TheAlgorithms/C-Plus-Plus). | ||
| - Run benchmark with GitHub Actions for continuous integration. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,95 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/env python3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Benchmark script to compare performance of cpp-linter-hooks vs mirrors-clang-format. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Usage: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| python benchmark_hooks.py | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Requirements: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - pre-commit must be installed and available in PATH | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Two config files: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - testing/pre-commit-config-cpp-linter-hooks.yaml | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - testing/pre-commit-config-mirrors-clang-format.yaml | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Target files: testing/main.c (or adjust as needed) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment on lines +5 to +14 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. Fix docstring: wrong paths and usage. The guidance doesn’t match the actual file path and config filenames. -Usage: - python benchmark_hooks.py +Usage: + python testing/benchmark_hooks.py @@ -- Two config files: - - testing/pre-commit-config-cpp-linter-hooks.yaml - - testing/pre-commit-config-mirrors-clang-format.yaml -- Target files: testing/main.c (or adjust as needed) +- Two config files: + - testing/benchmark_hook_1.yaml + - testing/benchmark_hook_2.yaml +- Target files are auto-discovered under testing/test-examples/ after cloning.
🤖 Prompt for AI Agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import statistics | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import glob | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HOOKS = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "name": "cpp-linter-hooks", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "config": "testing/pre-commit-config-cpp-linter-hooks.yaml", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "name": "mirrors-clang-format", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "config": "testing/pre-commit-config-mirrors-clang-format.yaml", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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. 💡 Verification agent 🧩 Analysis chainVerify configs exist before running. A missing testing/pre-commit-config-*.yaml will fail late. Add a preflight check and exit with a clear message. Proposed check (add near HOOKS or in main): for cfg in (h["config"] for h in HOOKS): if not Path(cfg).is_file(): raise SystemExit(f"Missing pre-commit config: {cfg}")To verify in your branch: 🏁 Script executed: #!/bin/bash set -e test -f testing/pre-commit-config-cpp-linter-hooks.yaml && echo "OK: cpp-linter-hooks config present" test -f testing/pre-commit-config-mirrors-clang-format.yaml && echo "OK: mirrors-clang-format config present"Length of output: 128 🏁 Script executed: #!/bin/bash ls -la testingLength of output: 910 Add preflight check and fix config paths
🤖 Prompt for AI Agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Automatically find all C/C++ files in testing/ (and optionally src/, include/) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TARGET_FILES = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| glob.glob("testing/**/*.c", recursive=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| + glob.glob("testing/**/*.cpp", recursive=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| + glob.glob("testing/**/*.h", recursive=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| + glob.glob("testing/**/*.hpp", recursive=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
shenxianpeng marked this conversation as resolved. Outdated Show resolved Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| REPEATS = 5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RESULTS_FILE = "testing/benchmark_results.txt" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def run_hook(config, files): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmd = ["pre-commit", "run", "--config", config, "--files"] + files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| start = time.perf_counter() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except subprocess.CalledProcessError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Still record time even if hook fails | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| end = time.perf_counter() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return end - start | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Comment on lines +57 to +67 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. 🛠️ Refactor suggestion Harden subprocess call: use iterable unpacking, capture_output, and a timeout. Prevents hangs from blocking hooks and addresses style nits. -def run_hook(config, files): - cmd = ["pre-commit", "run", "--config", config, "--files"] + files +def run_hook(config, files): + cmd = ["pre-commit", "run", "--config", config, "--files", *files] start = time.perf_counter() try: - subprocess.run(cmd, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + subprocess.run(cmd, check=True, capture_output=True, timeout=300) except subprocess.CalledProcessError: # Still record time even if hook fails pass + except subprocess.TimeoutExpired: + # Record as a timeout-run; caller still gets elapsed wall time + pass end = time.perf_counter() return end - start📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.12.2)45-45: Consider iterable unpacking instead of concatenation Replace with iterable unpacking (RUF005) 48-48: (S603) 🤖 Prompt for AI Agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def benchmark(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results = {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for hook in HOOKS: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| times = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Benchmarking {hook['name']}...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for i in range(REPEATS): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Clean up any changes before each run | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run(["git", "restore"] + TARGET_FILES) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subprocess.run(["pre-commit", "clean"]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t = run_hook(hook["config"], TARGET_FILES) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f" Run {i + 1}: {t:.3f} seconds") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| times.append(t) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results[hook["name"]] = times | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return results | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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. 🛠️ Refactor suggestion Don’t clean caches on every repeat; warm once per hook and restore files correctly. Cleaning inside the loop measures install time/network, not hook performance. Also restore both worktree and index to HEAD for deterministic state. def benchmark(): results = {} for hook in HOOKS: times = [] print(f"Benchmarking {hook['name']}...") - for i in range(REPEATS): - # Clean up any changes before each run - subprocess.run(["git", "restore"] + TARGET_FILES) - subprocess.run(["pre-commit", "clean"]) - t = run_hook(hook["config"], TARGET_FILES) + # Ensure no cross-hook cache effects, but keep caches across repeats + subprocess.run(["pre-commit", "clean"], check=False) + # Warm caches once (untimed) + print(" Warming caches (not timed)...") + _ = run_hook(hook["config"], TARGET_FILES) + for i in range(REPEATS): + # Reset files to a known clean state before each timed run + subprocess.run( + ["git", "restore", "--staged", "--worktree", "--source", "HEAD", "--", *TARGET_FILES], + check=False, + ) + t = run_hook(hook["config"], TARGET_FILES) print(f" Run {i + 1}: {t:.3f} seconds") times.append(t) results[hook["name"]] = times return results📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.12.2)63-63: (S603) 63-63: Consider Replace with (RUF005) 64-64: Starting a process with a partial executable path (S607) 🤖 Prompt for AI Agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def report(results): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lines = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for name, times in results.items(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| avg = statistics.mean(times) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| std = statistics.stdev(times) if len(times) > 1 else 0.0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| min_t = min(times) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| max_t = max(times) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lines.append( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"{name}: avg={avg:.3f}s, std={std:.3f}s, min={min_t:.3f}s, max={max_t:.3f}s, runs={len(times)}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print("\nBenchmark Results:") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print("\n".join(lines)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open(RESULTS_FILE, "w") as f: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write("\n".join(lines) + "\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Results saved to {RESULTS_FILE}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def main(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| results = benchmark() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| report(results) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| main() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Call out cache-warming to ensure fair, stable timings.
Make it explicit that cold-cache runs will dominate timings unless caches are warmed.
📝 Committable suggestion
🤖 Prompt for AI Agents