-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[clang-tidy] Add 'enable-check-profiling' with aggregated results to 'run-clang-tidy' #151011
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?
[clang-tidy] Add 'enable-check-profiling' with aggregated results to 'run-clang-tidy' #151011
Conversation
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesAdd new option This PR will help users to benchmark their Full diff: https://github.com/llvm/llvm-project/pull/151011.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index a3dca6c57571c..b6fb8f7b87d45 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -49,7 +49,7 @@
import time
import traceback
from types import ModuleType
-from typing import Any, Awaitable, Callable, List, Optional, TypeVar
+from typing import Any, Awaitable, Callable, Dict, List, Optional, Tuple, TypeVar
yaml: Optional[ModuleType] = None
@@ -105,6 +105,7 @@ def get_tidy_invocation(
warnings_as_errors: Optional[str],
exclude_header_filter: Optional[str],
allow_no_checks: bool,
+ store_check_profile: Optional[str],
) -> List[str]:
"""Gets a command line for clang-tidy."""
start = [clang_tidy_binary]
@@ -147,6 +148,9 @@ def get_tidy_invocation(
start.append(f"--warnings-as-errors={warnings_as_errors}")
if allow_no_checks:
start.append("--allow-no-checks")
+ if store_check_profile:
+ start.append("--enable-check-profile")
+ start.append(f"--store-check-profile={store_check_profile}")
if f:
start.append(f)
return start
@@ -178,6 +182,122 @@ def merge_replacement_files(tmpdir: str, mergefile: str) -> None:
open(mergefile, "w").close()
+def aggregate_profiles(profile_dir: str) -> Dict[str, float]:
+ """Aggregate timing data from multiple profile JSON files"""
+ aggregated: Dict[str, float] = {}
+
+ for profile_file in glob.iglob(os.path.join(profile_dir, "*.json")):
+ try:
+ with open(profile_file, "r", encoding="utf-8") as f:
+ data = json.load(f)
+ profile_data: Dict[str, float] = data.get("profile", {})
+
+ for key, value in profile_data.items():
+ if key.startswith("time.clang-tidy."):
+ if key in aggregated:
+ aggregated[key] += value
+ else:
+ aggregated[key] = value
+ except (json.JSONDecodeError, KeyError, IOError):
+ # Skip malformed or unreadable files
+ continue
+
+ return aggregated
+
+
+def display_profile_data(aggregated_data: Dict[str, float]) -> None:
+ """Display aggregated profile data in the same format as clang-tidy"""
+ if not aggregated_data:
+ return
+
+ # Extract checker names and their timing data
+ checkers: Dict[str, Dict[str, float]] = {}
+ for key, value in aggregated_data.items():
+ parts = key.split(".")
+ if len(parts) >= 4 and parts[0] == "time" and parts[1] == "clang-tidy":
+ checker_name = ".".join(
+ parts[2:-1]
+ ) # Everything between "clang-tidy" and the timing type
+ timing_type = parts[-1] # wall, user, or sys
+
+ if checker_name not in checkers:
+ checkers[checker_name] = {"wall": 0.0, "user": 0.0, "sys": 0.0}
+
+ checkers[checker_name][timing_type] = value
+
+ if not checkers:
+ return
+
+ total_user = sum(data["user"] for data in checkers.values())
+ total_sys = sum(data["sys"] for data in checkers.values())
+ total_wall = sum(data["wall"] for data in checkers.values())
+
+ sorted_checkers: List[Tuple[str, Dict[str, float]]] = sorted(
+ checkers.items(), key=lambda x: x[1]["user"] + x[1]["sys"], reverse=True
+ )
+
+ print(
+ "===-------------------------------------------------------------------------==="
+ )
+ print(" clang-tidy checks profiling")
+ print(
+ "===-------------------------------------------------------------------------==="
+ )
+ print(
+ f" Total Execution Time: {total_user + total_sys:.4f} seconds ({total_wall:.4f} wall clock)\n"
+ )
+
+ # Calculate field widths based on the Total line which has the largest values
+ total_combined = total_user + total_sys
+ user_width = len(f"{total_user:.4f}")
+ sys_width = len(f"{total_sys:.4f}")
+ combined_width = len(f"{total_combined:.4f}")
+ wall_width = len(f"{total_wall:.4f}")
+
+ # Header with proper alignment
+ additional_width = 9 # for " (100.0%)"
+ user_header = "---User Time---".center(user_width + additional_width)
+ sys_header = "--System Time--".center(sys_width + additional_width)
+ combined_header = "--User+System--".center(combined_width + additional_width)
+ wall_header = "---Wall Time---".center(wall_width + additional_width)
+
+ print(
+ f" {user_header} {sys_header} {combined_header} {wall_header} --- Name ---"
+ )
+
+ for checker_name, data in sorted_checkers:
+ user_time = data["user"]
+ sys_time = data["sys"]
+ wall_time = data["wall"]
+ combined_time = user_time + sys_time
+
+ user_pct = (user_time / total_user * 100) if total_user > 0 else 0
+ sys_pct = (sys_time / total_sys * 100) if total_sys > 0 else 0
+ combined_pct = (
+ (combined_time / total_combined * 100) if total_combined > 0 else 0
+ )
+ wall_pct = (wall_time / total_wall * 100) if total_wall > 0 else 0
+
+ user_str = f"{user_time:{user_width}.4f} ({user_pct:5.1f}%)"
+ sys_str = f"{sys_time:{sys_width}.4f} ({sys_pct:5.1f}%)"
+ combined_str = f"{combined_time:{combined_width}.4f} ({combined_pct:5.1f}%)"
+ wall_str = f"{wall_time:{wall_width}.4f} ({wall_pct:5.1f}%)"
+
+ print(
+ f" {user_str} {sys_str} {combined_str} {wall_str} {checker_name}"
+ )
+
+ # Total line
+ user_total_str = f"{total_user:{user_width}.4f} (100.0%)"
+ sys_total_str = f"{total_sys:{sys_width}.4f} (100.0%)"
+ combined_total_str = f"{total_combined:{combined_width}.4f} (100.0%)"
+ wall_total_str = f"{total_wall:{wall_width}.4f} (100.0%)"
+
+ print(
+ f" {user_total_str} {sys_total_str} {combined_total_str} {wall_total_str} Total"
+ )
+
+
def find_binary(arg: str, name: str, build_path: str) -> str:
"""Get the path for a binary or exit"""
if arg:
@@ -240,6 +360,7 @@ async def run_tidy(
clang_tidy_binary: str,
tmpdir: str,
build_path: str,
+ store_check_profile: Optional[str],
) -> ClangTidyResult:
"""
Runs clang-tidy on a single file and returns the result.
@@ -263,6 +384,7 @@ async def run_tidy(
args.warnings_as_errors,
args.exclude_header_filter,
args.allow_no_checks,
+ store_check_profile,
)
try:
@@ -447,6 +569,11 @@ async def main() -> None:
action="store_true",
help="Allow empty enabled checks.",
)
+ parser.add_argument(
+ "-enable-check-profile",
+ action="store_true",
+ help="Enable per-check timing profiles, and print a report",
+ )
args = parser.parse_args()
db_path = "compile_commands.json"
@@ -489,6 +616,12 @@ async def main() -> None:
export_fixes_dir = tempfile.mkdtemp()
delete_fixes_dir = True
+ profile_dir: Optional[str] = None
+ delete_profile_dir = False
+ if args.enable_check_profile:
+ profile_dir = tempfile.mkdtemp()
+ delete_profile_dir = True
+
try:
invocation = get_tidy_invocation(
None,
@@ -509,6 +642,7 @@ async def main() -> None:
args.warnings_as_errors,
args.exclude_header_filter,
args.allow_no_checks,
+ None, # No profiling for the list-checks invocation
)
invocation.append("-list-checks")
invocation.append("-")
@@ -567,6 +701,7 @@ async def main() -> None:
clang_tidy_binary,
export_fixes_dir,
build_path,
+ profile_dir,
)
)
for f in files
@@ -593,8 +728,18 @@ async def main() -> None:
if delete_fixes_dir:
assert export_fixes_dir
shutil.rmtree(export_fixes_dir)
+ if delete_profile_dir:
+ assert profile_dir
+ shutil.rmtree(profile_dir)
return
+ if args.enable_check_profile and profile_dir:
+ aggregated_data = aggregate_profiles(profile_dir)
+ if aggregated_data:
+ display_profile_data(aggregated_data)
+ else:
+ print("No profiling data found.")
+
if combine_fixes:
print(f"Writing fixes to {args.export_fixes} ...")
try:
@@ -618,6 +763,9 @@ async def main() -> None:
if delete_fixes_dir:
assert export_fixes_dir
shutil.rmtree(export_fixes_dir)
+ if delete_profile_dir:
+ assert profile_dir
+ shutil.rmtree(profile_dir)
sys.exit(returncode)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 61debd89becaa..cf66c9dd299a2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,10 @@ Improvements to clang-tidy
now run checks in parallel by default using all available hardware threads.
Both scripts display the number of threads being used in their output.
+- Improved :program:`run-clang-tidy.py` by adding a new option
+ `enable-check-profile` to enable per-check timing profiles and print a
+ report based on all analyzed files.
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy-enable-check-profile.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy-enable-check-profile.cpp
new file mode 100644
index 0000000000000..8489f29569822
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy-enable-check-profile.cpp
@@ -0,0 +1,54 @@
+// Test profiling functionality with single file
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "[{\"directory\":\".\",\"command\":\"clang++ -c %/t/test.cpp\",\"file\":\"%/t/test.cpp\"}]" | sed -e 's/\\/\\\\/g' > %t/compile_commands.json
+// RUN: echo "Checks: '-*,readability-function-size'" > %t/.clang-tidy
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: cd "%t"
+// RUN: %run_clang_tidy -enable-check-profile "test.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-SINGLE
+
+// CHECK-SINGLE: Running clang-tidy in {{[1-9][0-9]*}} threads for 1 files out of 1 in compilation database
+// CHECK-SINGLE: ===-------------------------------------------------------------------------===
+// CHECK-SINGLE-NEXT: clang-tidy checks profiling
+// CHECK-SINGLE-NEXT: ===-------------------------------------------------------------------------===
+// CHECK-SINGLE-NEXT: Total Execution Time: {{.*}} seconds ({{.*}} wall clock)
+// CHECK-SINGLE: {{.*}} --- Name ---
+// CHECK-SINGLE-NEXT: {{.*}} readability-function-size
+// CHECK-SINGLE-NEXT: {{.*}} Total
+
+// Test profiling functionality with multiple files and multiple checks
+// RUN: rm -rf %t-multi
+// RUN: mkdir %t-multi
+// RUN: echo "[{\"directory\":\".\",\"command\":\"clang++ -c %/t-multi/test1.cpp\",\"file\":\"%/t-multi/test1.cpp\"},{\"directory\":\".\",\"command\":\"clang++ -c %/t-multi/test2.cpp\",\"file\":\"%/t-multi/test2.cpp\"}]" | sed -e 's/\\/\\\\/g' > %t-multi/compile_commands.json
+// RUN: echo "Checks: '-*,readability-function-size,misc-unused-using-decls,llvm-qualified-auto'" > %t-multi/.clang-tidy
+// RUN: cp "%s" "%t-multi/test1.cpp"
+// RUN: cp "%s" "%t-multi/test2.cpp"
+// RUN: cd "%t-multi"
+// RUN: %run_clang_tidy -enable-check-profile -j 2 "test1.cpp" "test2.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE
+
+// CHECK-MULTIPLE: Running clang-tidy in 2 threads for 2 files out of 2 in compilation database
+// CHECK-MULTIPLE: ===-------------------------------------------------------------------------===
+// CHECK-MULTIPLE-NEXT: clang-tidy checks profiling
+// CHECK-MULTIPLE-NEXT: ===-------------------------------------------------------------------------===
+// CHECK-MULTIPLE-NEXT: Total Execution Time: {{.*}} seconds ({{.*}} wall clock)
+// CHECK-MULTIPLE: {{.*}} --- Name ---
+// CHECK-MULTIPLE: {{.*}} readability-function-size
+// CHECK-MULTIPLE: {{.*}} misc-unused-using-decls
+// CHECK-MULTIPLE: {{.*}} llvm-qualified-auto
+// CHECK-MULTIPLE: {{.*}} Total
+
+// Test profiling functionality with no files (empty database)
+// RUN: rm -rf %t-empty
+// RUN: mkdir %t-empty
+// RUN: echo "[]" > %t-empty/compile_commands.json
+// RUN: echo "Checks: '-*'" > %t-empty/.clang-tidy
+// RUN: cd "%t-empty"
+// RUN: %run_clang_tidy -enable-check-profile -allow-no-checks 2>&1 | FileCheck %s --check-prefix=CHECK-EMPTY
+
+// CHECK-EMPTY: Running clang-tidy in {{[1-9][0-9]*}} threads for 0 files out of 0 in compilation database
+// CHECK-EMPTY: No profiling data found.
+
+class A {
+ A() {}
+ ~A() {}
+};
|
Funny, I wrote this feature a while ago and never got around to making a PR out of it. FWIW here is my (old) diff: main...nicovank:llvm-project:temporary-tidy-profiles |
Add new option
enable-check-profiling
torun-clang-tidy
for seamless integration ofclang-tidy
'senable-check-profiling
option.run-clang-tidy
will post aggregated results report in the same style asclang-tidy
.This PR will help users to benchmark their
clang-tidy
runs easily.Also,
clang-tidy
developers could build benchmark infrastructure in the future.