https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/151011
>From 65a04c0b903243bbc854477075b8e9e1a62fdfeb Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Mon, 28 Jul 2025 21:12:04 +0300 Subject: [PATCH 1/4] [clang-tidy] Add 'enable-check-profiling' with aggregated results to 'run-clang-tidy' --- .../clang-tidy/tool/run-clang-tidy.py | 150 +++++++++++++++++- clang-tools-extra/docs/ReleaseNotes.rst | 4 + .../run-clang-tidy-enable-check-profile.cpp | 54 +++++++ 3 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy-enable-check-profile.cpp 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() {} +}; >From 6688a60c6f1da816848f86a1064bb91d1c3c3d7e Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Tue, 29 Jul 2025 00:50:52 +0300 Subject: [PATCH 2/4] fix tests with DAG --- .../run-clang-tidy-enable-check-profile.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) 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 index 8489f29569822..9ead09a35da33 100644 --- 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 @@ -12,9 +12,10 @@ // 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 +// CHECK-SINGLE-EMPTY: +// CHECK-SINGLE-NEXT: ---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name --- +// CHECK-SINGLE: {{[[:space:]]*[0-9]+\.[0-9]+.*%.*readability-function-size}} +// CHECK-SINGLE: {{[[:space:]]*[0-9]+\.[0-9]+.*100\.0%.*Total}} // Test profiling functionality with multiple files and multiple checks // RUN: rm -rf %t-multi @@ -31,11 +32,12 @@ // 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 +// CHECK-MULTIPLE-EMPTY: +// CHECK-MULTIPLE-NEXT: ---User Time--- --System Time-- --User+System-- ---Wall Time--- --- Name --- +// CHECK-MULTIPLE-DAG: {{[[:space:]]*[0-9]+\.[0-9]+.*%.*readability-function-size}} +// CHECK-MULTIPLE-DAG: {{[[:space:]]*[0-9]+\.[0-9]+.*%.*misc-unused-using-decls}} +// CHECK-MULTIPLE-DAG: {{[[:space:]]*[0-9]+\.[0-9]+.*%.*llvm-qualified-auto}} +// CHECK-MULTIPLE: {{[[:space:]]*[0-9]+\.[0-9]+.*100\.0%.*Total}} // Test profiling functionality with no files (empty database) // RUN: rm -rf %t-empty >From f39f0b5a5916a8899386c9b30a36c58334766a6b Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Tue, 29 Jul 2025 07:58:36 +0300 Subject: [PATCH 3/4] address pr comments --- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 b6fb8f7b87d45..7a17e765c940d 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -198,8 +198,8 @@ def aggregate_profiles(profile_dir: str) -> Dict[str, float]: aggregated[key] += value else: aggregated[key] = value - except (json.JSONDecodeError, KeyError, IOError): - # Skip malformed or unreadable files + except (json.JSONDecodeError, KeyError, IOError) as e: + print(f"Warning: invalid json file {profile_file}: {e}", file=sys.stderr) continue return aggregated @@ -617,10 +617,8 @@ async def main() -> None: 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( @@ -728,8 +726,7 @@ async def main() -> None: if delete_fixes_dir: assert export_fixes_dir shutil.rmtree(export_fixes_dir) - if delete_profile_dir: - assert profile_dir + if profile_dir: shutil.rmtree(profile_dir) return @@ -763,8 +760,7 @@ async def main() -> None: if delete_fixes_dir: assert export_fixes_dir shutil.rmtree(export_fixes_dir) - if delete_profile_dir: - assert profile_dir + if profile_dir: shutil.rmtree(profile_dir) sys.exit(returncode) >From dee6d9f44371adcabd6c4a9800d0ea89f97dae28 Mon Sep 17 00:00:00 2001 From: Victor Baranov <bar.victor.2...@gmail.com> Date: Tue, 29 Jul 2025 08:13:31 +0300 Subject: [PATCH 4/4] change warning to error --- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7a17e765c940d..5310ae42d7dda 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -199,7 +199,7 @@ def aggregate_profiles(profile_dir: str) -> Dict[str, float]: else: aggregated[key] = value except (json.JSONDecodeError, KeyError, IOError) as e: - print(f"Warning: invalid json file {profile_file}: {e}", file=sys.stderr) + print(f"Error: invalid json file {profile_file}: {e}", file=sys.stderr) continue return aggregated _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits