Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: Benchmark Hooks

on:
workflow_dispatch:

jobs:
benchmark:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Update Actions to supported versions; bump Python.

actionlint flags v2 runners as too old. Upgrade to current majors and avoid EOL Python 3.8.

Apply this diff:

-      - name: Checkout code
-        uses: actions/checkout@v2
+      - name: Checkout code
+        uses: actions/checkout@v4
@@
-      - name: Set up Python
-        uses: actions/setup-python@v2
+      - name: Set up Python
+        uses: actions/setup-python@v5
         with:
-          python-version: '3.8'
+          python-version: '3.11'

Also applies to: 14-16

🧰 Tools
🪛 actionlint (1.7.7)

11-11: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🤖 Prompt for AI Agents
In .github/workflows/benchmark.yml around lines 11 and 14-16, update deprecated
workflow action versions and the Python runner: replace uses:
actions/checkout@v2 with the current major (actions/checkout@v4) and update any
other actions pinned to v2/v3 to their current majors; also change the Python
version from 3.8 to a supported release (e.g., 3.11 or 3.10) in the jobs/steps
that set up Python. Ensure the workflow uses the new action majors consistently
and verify no breaking input changes are required by consulting each action's
release notes before committing.


- name: Set up Python
uses: actions/setup-python@v2
with:
python-version: '3.8'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install pre-commit

- name: Run benchmarks
run: |
python testing/benchmark_hooks.py
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ venv
result.txt
testing/main.c
*/*compile_commands.json
testing/benchmark_results.txt
testing/test-examples/*

# Ignore Python wheel packages (clang-format, clang-tidy)
clang-tidy-1*
Expand Down
5 changes: 1 addition & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,9 @@ repos:
- id: check-yaml
- id: check-toml
- id: requirements-txt-fixer
- repo: https://github.com/asottile/pyupgrade
rev: v3.20.0
hooks:
- id: pyupgrade
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.12.11
hooks:
- id: ruff
args: [--fix]
- id: ruff-format
15 changes: 15 additions & 0 deletions docs/benchmark.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Benchmarking

[![CodSpeed Badge](https://img.shields.io/endpoint?url=https://codspeed.io/badge.json)](https://codspeed.io/cpp-linter/cpp-linter-hooks)

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`.
Comment on lines +13 to +15
Copy link

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.

 ## Results
 
-The results of the benchmarking process will be saved to `testing/benchmark_results.txt`.
+The results of the benchmarking process will be saved to `testing/benchmark_results.txt`.
+
+Note: For fair comparisons, warm pre-commit caches once per hook (do not clean between repeats). If you need cold-cache numbers, run a separate pass that cleans caches before the first run only.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## Results
The results of the benchmarking process will be saved to `testing/benchmark_results.txt`.
## Results
The results of the benchmarking process will be saved to `testing/benchmark_results.txt`.
Note: For fair comparisons, warm pre-commit caches once per hook (do not clean between repeats). If you need cold-cache numbers, run a separate pass that cleans caches before the first run only.
🤖 Prompt for AI Agents
In docs/benchmark.md around lines 11 to 13, the Results section does not mention
cache-warming and thus can mislead readers because cold-cache runs will dominate
timings; update the text to call out cache-warming explicitly and prescribe a
procedure: run a number of warm-up iterations before timed runs (or document how
to clear caches for cold runs), run each benchmark multiple times, report both
cold and warm timings or the averaged warm timings, and note any
cache/state-reset steps used so results are fair and reproducible.

6 changes: 6 additions & 0 deletions testing/benchmark_hook_1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v1.1.0
hooks:
- id: clang-format
args: [--style=file, --version=21]
5 changes: 5 additions & 0 deletions testing/benchmark_hook_2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
repos:
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v21.1.0
hooks:
- id: clang-format
154 changes: 154 additions & 0 deletions testing/benchmark_hooks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
#!/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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In testing/benchmark_hooks.py around lines 5 to 14, the module docstring
contains incorrect usage text and wrong config file paths/names; update the
docstring to show the actual invocation to run this script (reflect whether it
should be run as python testing/benchmark_hooks.py or python -m
testing.benchmark_hooks), replace the two listed config filenames and their
paths with the real config files present in the repo, and correct the example
target file path (or note it is adjustable) so the docstring accurately matches
the repository layout and how to run the script.


import os
import subprocess
import time
import statistics
import glob

HOOKS = [
{
"name": "cpp-linter-hooks",
"config": "testing/benchmark_hook_1.yaml",
},
{
"name": "mirrors-clang-format",
"config": "testing/benchmark_hook_2.yaml",
},
]

# Automatically find all C/C++ files in testing/ (and optionally src/, include/)
TARGET_FILES = glob.glob("testing/test-examples/*.c", recursive=True)

REPEATS = 5
RESULTS_FILE = "testing/benchmark_results.txt"


def git_clone():
try:
subprocess.run(
[
"git",
"clone",
"--depth",
"1",
"https://github.com/gouravthakur39/beginners-C-program-examples.git",
"testing/test-examples",
],
check=True,
)
except subprocess.CalledProcessError:
pass


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
Copy link

Choose a reason for hiding this comment

The 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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
def run_hook(config, files):
cmd = ["pre-commit", "run", "--config", config, "--files", *files]
start = time.perf_counter()
try:
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
🧰 Tools
🪛 Ruff (0.12.2)

45-45: Consider iterable unpacking instead of concatenation

Replace with iterable unpacking

(RUF005)


48-48: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents
In testing/benchmark_hooks.py around lines 44-54, the subprocess.run call should
be hardened: construct the command using iterable unpacking for files (e.g.,
["pre-commit","run","--config", config,"--files", *files]), call subprocess.run
with capture_output=True instead of stdout/stderr, and pass a sensible timeout
(e.g., timeout=300) to avoid hangs; catch both subprocess.CalledProcessError and
subprocess.TimeoutExpired (still recording elapsed time and returning it) so
failing or timed-out hooks don't block the benchmark.


def safe_git_restore(files):
# Only restore files tracked by git
tracked = []
for f in files:
result = subprocess.run(
["git", "ls-files", "--error-unmatch", f],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
if result.returncode == 0:
tracked.append(f)
if tracked:
subprocess.run(["git", "restore"] + tracked)

Comment on lines +69 to +82
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Restoring via top-level Git misses cloned examples (nested repo).

Files in testing/test-examples are not tracked by the top-level repo, so no restore happens and runs are not independent. Reset the nested repo to HEAD between runs.

-def safe_git_restore(files):
-    # Only restore files tracked by git
-    tracked = []
-    for f in files:
-        result = subprocess.run(
-            ["git", "ls-files", "--error-unmatch", f],
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-        )
-        if result.returncode == 0:
-            tracked.append(f)
-    if tracked:
-        subprocess.run(["git", "restore"] + tracked)
+def safe_git_restore(_files):
+    # Reset the cloned examples repository (nested Git repo) to a clean state.
+    examples_repo = Path("testing/test-examples")
+    if (examples_repo / ".git").exists():
+        subprocess.run(
+            ["git", "-C", str(examples_repo), "reset", "--hard", "HEAD"],
+            check=False,
+            capture_output=True,
+        )

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff (0.12.2)

72-72: subprocess call: check for execution of untrusted input

(S603)


73-73: Starting a process with a partial executable path

(S607)


80-80: subprocess call: check for execution of untrusted input

(S603)


80-80: Consider ["git", "restore", *tracked] instead of concatenation

Replace with ["git", "restore", *tracked]

(RUF005)

🤖 Prompt for AI Agents
testing/benchmark_hooks.py around lines 68-81: the current safe_git_restore only
checks top-level git and misses files inside nested/cloned repos
(testing/test-examples), so those examples aren't reset between runs; update the
function to detect files that are not tracked by the top-level repo, locate
their nearest enclosing git repository root (walk up parents for a .git
directory), and for each distinct nested repo run a git reset --hard HEAD (and
optionally git clean -fd) via subprocess (using git -C <repo_root> ...) to
guarantee the nested repo is reset between runs before restoring top-level
tracked files.


def benchmark():
results = {}
for hook in HOOKS:
times = []
print(f"\nBenchmarking {hook['name']}...")
for i in range(REPEATS):
safe_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


def report(results):
headers = ["Hook", "Avg (s)", "Std (s)", "Min (s)", "Max (s)", "Runs"]
col_widths = [max(len(h), 16) for h in headers]
# Calculate max width for each column
for name, times in results.items():
col_widths[0] = max(col_widths[0], len(name))
print("\nBenchmark Results:\n")
# Print header
header_row = " | ".join(h.ljust(w) for h, w in zip(headers, col_widths))
print(header_row)
print("-+-".join("-" * w for w in col_widths))
# Print rows
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)
row = [
name.ljust(col_widths[0]),
f"{avg:.3f}".ljust(col_widths[1]),
f"{std:.3f}".ljust(col_widths[2]),
f"{min_t:.3f}".ljust(col_widths[3]),
f"{max_t:.3f}".ljust(col_widths[4]),
str(len(times)).ljust(col_widths[5]),
]
print(" | ".join(row))
lines.append(" | ".join(row))
# Save to file
with open(RESULTS_FILE, "w") as f:
f.write(header_row + "\n")
f.write("-+-".join("-" * w for w in col_widths) + "\n")
for line in lines:
f.write(line + "\n")
print(f"\nResults saved to {RESULTS_FILE}")

# Write to GitHub Actions summary if available
summary_path = os.environ.get("GITHUB_STEP_SUMMARY")
if summary_path:
with open(summary_path, "a") as f:
f.write("## Benchmark Results\n\n")
f.write(header_row + "\n")
f.write("-+-".join("-" * w for w in col_widths) + "\n")
for line in lines:
f.write(line + "\n")
f.write("\n")


def main():
git_clone()
results = benchmark()
report(results)


if __name__ == "__main__":
main()
Loading