-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[clang-tidy] Add parallel execution by default in 'run-clang-tidy' and 'clang-tidy-diff' #149739
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
…d 'clang-tidy-diff'
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesChange the default value of Both scripts now also print the number of threads being used to Fixes #148624. Full diff: https://github.com/llvm/llvm-project/pull/149739.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py b/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
index 0f8ac7344aca3..7cd21afd70f7e 100755
--- a/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
+++ b/clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
@@ -177,7 +177,7 @@ def main():
parser.add_argument(
"-j",
type=int,
- default=1,
+ default=0,
help="number of tidy instances to be run in parallel.",
)
parser.add_argument(
@@ -318,6 +318,7 @@ def main():
if max_task_count == 0:
max_task_count = multiprocessing.cpu_count()
max_task_count = min(len(lines_by_file), max_task_count)
+ print(f"Running clang-tidy in {max_task_count} threads...")
combine_fixes = False
export_fixes_dir = None
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 8741147a4f8a3..a3dca6c57571c 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -548,7 +548,7 @@ async def main() -> None:
files = {f for f in files if file_name_re.search(f)}
print(
- "Running clang-tidy for",
+ f"Running clang-tidy in {max_task} threads for",
len(files),
"files out of",
number_files_in_database,
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 07ebf8008928d..8e0aafdd84572 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -93,6 +93,10 @@ Improvements to clang-query
Improvements to clang-tidy
--------------------------
+- The ``run-clang-tidy.py`` and ``clang-tidy-diff.py`` scripts now run checks in
+ parallel by default using all available hardware threads. Both scripts now
+ display the number of threads being used in their output.
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
index 2e18a43c8e94f..7aa6ce9ab7501 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/clang-tidy-diff.cpp
@@ -1,15 +1,19 @@
// REQUIRES: shell
// RUN: sed 's/placeholder_for_f/f/' %s > %t.cpp
// RUN: clang-tidy -checks=-*,modernize-use-override %t.cpp -- -std=c++11 | FileCheck -check-prefix=CHECK-SANITY %s
-// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -- -std=c++11 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-JMAX
// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -quiet -- -std=c++11 2>&1 | FileCheck -check-prefix=CHECK-QUIET %s
// RUN: mkdir -p %T/compilation-database-test/
// RUN: echo '[{"directory": "%T", "command": "clang++ -o test.o -std=c++11 %t.cpp", "file": "%t.cpp"}]' > %T/compilation-database-test/compile_commands.json
// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -path %T/compilation-database-test 2>&1 | FileCheck -check-prefix=CHECK %s
+
+// RUN: not diff -U0 %s %t.cpp | %clang_tidy_diff -checks=-*,modernize-use-override -j 1 -- -std=c++11 2>&1 | FileCheck %s --check-prefix=CHECK-J1
+// CHECK-J1: Running clang-tidy in 1 threads...
struct A {
virtual void f() {}
virtual void g() {}
};
+// CHECK-JMAX: Running clang-tidy in {{[1-9][0-9]*}} threads...
// CHECK-NOT: warning:
// CHECK-QUIET-NOT: warning:
struct B : public A {
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp
index 505ee28ed7120..6337686c58518 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp
@@ -8,7 +8,11 @@
// RUN: echo " modernize-use-auto.MinTypeNameLength: '0'" >> %t/.clang-tidy
// RUN: cp "%s" "%t/test.cpp"
// RUN: cd "%t"
-// RUN: not %run_clang_tidy "test.cpp"
+// RUN: not %run_clang_tidy "test.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-JMAX
+// CHECK-JMAX: Running clang-tidy in {{[1-9][0-9]*}} threads for
+
+// RUN: not %run_clang_tidy -j 1 "test.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-J1
+// CHECK-J1: Running clang-tidy in 1 threads for
int main()
{
|
Note about comment by Eugene:
|
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.
LGTM!
Not sure if possible but a less aggressive option would be to have a "-l" flag like "make" has, which uses as many threads as requested while keeping track on the load of the system and adjusting accordingly.
Can the load factor be platform independent? AFAIK windows don't have same load factor mechanic as UNIX. With this feature, we should carefully plan what platforms will support it. |
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.
Also I think clang-tidy-diff.py
should refer to Clang-Tidy. Several places are affected.
@@ -93,6 +93,10 @@ Improvements to clang-query | |||
Improvements to clang-tidy | |||
-------------------------- | |||
|
|||
- The ``run-clang-tidy.py`` and ``clang-tidy-diff.py`` scripts now run checks in |
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.
- The ``run-clang-tidy.py`` and ``clang-tidy-diff.py`` scripts now run checks in | |
- The :program:`run-clang-tidy.py` and :program:`clang-tidy-diff.py` scripts now run checks in |
Do you mean renaming |
I will make the renaming on the next PR. |
Change the default value of
-j
from1
to0
inclang-tidy-diff.py
script to autodetect number of CPU cores to run on.Script
run-clang-tidy.py
already had this behavior by default.Both scripts now also print the number of threads being used to
provide better visibility into their execution behavior.
Fixes #148624.