[clang-query] add basic profiling on matching each ASTs #114806

danix800 · 2024-11-04T14:47:23Z

We've found that basic profiling could help improving/optimizing when developing clang-tidy checks.

This PR adds an extra command

set enable-profile (true|false)   Set whether to enable matcher profiling.

which enables profiling queries on each file.

Sample output:

$ cat test.cql
set enable-profile true
m binaryOperator(isExpansionInMainFile())

$ cat test.c
int test(int i, int j) {
  return i + j;
}

$ clang-query --track-memory -f test.cql test.c --

Match #1:

{{.*}}/test.c:2:10: note: "root" binds here
    2 |   return i + j;
      |          ^~~~~
1 match.
===-------------------------------------------------------------------------===
                         clang-query matcher profiling
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  ---Mem---  --- Name ---
   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)        224  {{.*}}/test.c
   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)        224  Total

Sample output: $ cat test.cql set enable-profile true m binaryOperator(isExpansionInMainFile()) $ cat test.c int test(int i, int j) { return i + j; } $ clang-query --track-memory -f test.cql test.c -- Match llvm#1: {{.*}}/test.c:2:10: note: "root" binds here 2 | return i + j; | ^~~~~ 1 match. ===-------------------------------------------------------------------------=== clang-query matcher profiling ===-------------------------------------------------------------------------=== Total Execution Time: 0.0000 seconds (0.0000 wall clock) ---User Time--- --System Time-- --User+System-- ---Wall Time--- ---Mem--- --- Name --- 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 224 {{.*}}/test.c 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 224 Total

llvmbot · 2024-11-04T14:48:01Z

@llvm/pr-subscribers-clang-tools-extra

Author: Ding Fei (danix800)

Changes

Sample output:

$ cat test.cql
set enable-profile true
m binaryOperator(isExpansionInMainFile())

$ cat test.c
int test(int i, int j) {
  return i + j;
}

$ clang-query --track-memory -f test.cql test.c --

Match #<!-- -->1:

{{.*}}/test.c:2:10: note: "root" binds here
    2 |   return i + j;
      |          ^~~~~
1 match.
===-------------------------------------------------------------------------===
                         clang-query matcher profiling
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0000 seconds (0.0000 wall clock)

   ---User Time---   --System Time--   --User+System--   ---Wall Time---  ---Mem---  --- Name ---
   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)        224  {{.*}}/test.c
   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)   0.0000 (100.0%)        224  Total

Full diff: https://github.com/llvm/llvm-project/pull/114806.diff

6 Files Affected:

(modified) clang-tools-extra/clang-query/CMakeLists.txt (+1)
(modified) clang-tools-extra/clang-query/Query.cpp (+23-3)
(modified) clang-tools-extra/clang-query/QueryParser.cpp (+5)
(added) clang-tools-extra/clang-query/QueryProfile.cpp (+24)
(added) clang-tools-extra/clang-query/QueryProfile.h (+35)
(modified) clang-tools-extra/clang-query/QuerySession.h (+2-1)

diff --git a/clang-tools-extra/clang-query/CMakeLists.txt b/clang-tools-extra/clang-query/CMakeLists.txt
index b168a3a8581567..84a1ad6fa33582 100644
--- a/clang-tools-extra/clang-query/CMakeLists.txt
+++ b/clang-tools-extra/clang-query/CMakeLists.txt
@@ -7,6 +7,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangQuery STATIC
   Query.cpp
   QueryParser.cpp
+  QueryProfile.cpp
 
   DEPENDS
   omp_gen
diff --git a/clang-tools-extra/clang-query/Query.cpp b/clang-tools-extra/clang-query/Query.cpp
index 282d136aff721a..5b628de3c4deeb 100644
--- a/clang-tools-extra/clang-query/Query.cpp
+++ b/clang-tools-extra/clang-query/Query.cpp
@@ -8,6 +8,7 @@
 
 #include "Query.h"
 #include "QueryParser.h"
+#include "QueryProfile.h"
 #include "QuerySession.h"
 #include "clang/AST/ASTDumper.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -44,6 +45,8 @@ bool HelpQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
         "  set bind-root (true|false)        "
         "Set whether to bind the root matcher to \"root\".\n"
         "  set print-matcher (true|false)    "
+        "  set enable-profile (true|false)    "
+        "Set whether to enable matcher profiling,\n"
         "Set whether to print the current matcher,\n"
         "  set traversal <kind>              "
         "Set traversal kind of clang-query session. Available kinds are:\n"
@@ -82,10 +85,13 @@ namespace {
 
 struct CollectBoundNodes : MatchFinder::MatchCallback {
   std::vector<BoundNodes> &Bindings;
-  CollectBoundNodes(std::vector<BoundNodes> &Bindings) : Bindings(Bindings) {}
+  StringRef Unit;
+  CollectBoundNodes(std::vector<BoundNodes> &Bindings, StringRef Unit)
+      : Bindings(Bindings), Unit(Unit) {}
   void run(const MatchFinder::MatchResult &Result) override {
     Bindings.push_back(Result.Nodes);
   }
+  StringRef getID() const override { return Unit; }
 };
 
 } // namespace
@@ -93,8 +99,19 @@ struct CollectBoundNodes : MatchFinder::MatchCallback {
 bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
   unsigned MatchCount = 0;
 
+  std::optional<QueryProfile> Profiling;
+  if (QS.EnableProfile)
+    Profiling = QueryProfile();
+
   for (auto &AST : QS.ASTs) {
-    MatchFinder Finder;
+    ast_matchers::MatchFinder::MatchFinderOptions FinderOptions;
+    std::optional<llvm::StringMap<llvm::TimeRecord>> Records;
+    if (QS.EnableProfile) {
+      Records.emplace();
+      FinderOptions.CheckProfiling.emplace(*Records);
+    }
+
+    MatchFinder Finder(FinderOptions);
     std::vector<BoundNodes> Matches;
     DynTypedMatcher MaybeBoundMatcher = Matcher;
     if (QS.BindRoot) {
@@ -102,7 +119,8 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
       if (M)
         MaybeBoundMatcher = *M;
     }
-    CollectBoundNodes Collect(Matches);
+    StringRef OrigSrcName = AST->getOriginalSourceFileName();
+    CollectBoundNodes Collect(Matches, OrigSrcName);
     if (!Finder.addDynamicMatcher(MaybeBoundMatcher, &Collect)) {
       OS << "Not a valid top-level matcher.\n";
       return false;
@@ -111,6 +129,8 @@ bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
     ASTContext &Ctx = AST->getASTContext();
     Ctx.getParentMapContext().setTraversalKind(QS.TK);
     Finder.matchAST(Ctx);
+    if (QS.EnableProfile)
+      Profiling->Records[OrigSrcName] += (*Records)[OrigSrcName];
 
     if (QS.PrintMatcher) {
       SmallVector<StringRef, 4> Lines;
diff --git a/clang-tools-extra/clang-query/QueryParser.cpp b/clang-tools-extra/clang-query/QueryParser.cpp
index 97cb264a611af3..1d5ec281defd40 100644
--- a/clang-tools-extra/clang-query/QueryParser.cpp
+++ b/clang-tools-extra/clang-query/QueryParser.cpp
@@ -182,6 +182,7 @@ enum ParsedQueryVariable {
   PQV_Output,
   PQV_BindRoot,
   PQV_PrintMatcher,
+  PQV_EnableProfile,
   PQV_Traversal
 };
 
@@ -285,6 +286,7 @@ QueryRef QueryParser::doParse() {
             .Case("output", PQV_Output)
             .Case("bind-root", PQV_BindRoot)
             .Case("print-matcher", PQV_PrintMatcher)
+            .Case("enable-profile", PQV_EnableProfile)
             .Case("traversal", PQV_Traversal)
             .Default(PQV_Invalid);
     if (VarStr.empty())
@@ -303,6 +305,9 @@ QueryRef QueryParser::doParse() {
     case PQV_PrintMatcher:
       Q = parseSetBool(&QuerySession::PrintMatcher);
       break;
+    case PQV_EnableProfile:
+      Q = parseSetBool(&QuerySession::EnableProfile);
+      break;
     case PQV_Traversal:
       Q = parseSetTraversalKind(&QuerySession::TK);
       break;
diff --git a/clang-tools-extra/clang-query/QueryProfile.cpp b/clang-tools-extra/clang-query/QueryProfile.cpp
new file mode 100644
index 00000000000000..9b60f0bf1c7f4e
--- /dev/null
+++ b/clang-tools-extra/clang-query/QueryProfile.cpp
@@ -0,0 +1,24 @@
+//===-------- QueryProfile.cpp - clang-query --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "QueryProfile.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace clang::query {
+
+void QueryProfile::printUserFriendlyTable(llvm::raw_ostream &OS) {
+  TG->print(OS);
+  OS.flush();
+}
+
+QueryProfile::~QueryProfiling() {
+  TG.emplace("clang-query", "clang-query matcher profiling", Records);
+  printUserFriendlyTable(llvm::errs());
+}
+
+} // namespace clang::query
diff --git a/clang-tools-extra/clang-query/QueryProfile.h b/clang-tools-extra/clang-query/QueryProfile.h
new file mode 100644
index 00000000000000..6815dc0245bdc8
--- /dev/null
+++ b/clang-tools-extra/clang-query/QueryProfile.h
@@ -0,0 +1,35 @@
+//===--------- QueryProfile.h - clang-query ---------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_QUERY_QUERY_PROFILE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_QUERY_QUERY_PROFILE_H
+
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Timer.h"
+#include <optional>
+
+namespace llvm {
+class raw_ostream;
+} // namespace llvm
+
+namespace clang::query {
+
+class QueryProfile {
+public:
+  llvm::StringMap<llvm::TimeRecord> Records;
+  QueryProfile() = default;
+  ~QueryProfile();
+
+private:
+  std::optional<llvm::TimerGroup> TG;
+  void printUserFriendlyTable(llvm::raw_ostream &OS);
+};
+
+} // namespace clang::query
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_QUERY_QUERY_PROFILE_H
diff --git a/clang-tools-extra/clang-query/QuerySession.h b/clang-tools-extra/clang-query/QuerySession.h
index 31a4900e26190b..c7d5a64c332008 100644
--- a/clang-tools-extra/clang-query/QuerySession.h
+++ b/clang-tools-extra/clang-query/QuerySession.h
@@ -26,7 +26,7 @@ class QuerySession {
   QuerySession(llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs)
       : ASTs(ASTs), PrintOutput(false), DiagOutput(true),
         DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
-        Terminate(false), TK(TK_AsIs) {}
+        EnableProfile(false), Terminate(false), TK(TK_AsIs) {}
 
   llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs;
 
@@ -36,6 +36,7 @@ class QuerySession {
 
   bool BindRoot;
   bool PrintMatcher;
+  bool EnableProfile;
   bool Terminate;
 
   TraversalKind TK;

AaronBallman

This is really cool, thank you! Can you also add a release note to clang-tools-extra/docs/ReleaseNotes.rst so users know about the new functionality?

Changes generally look good, I just had a few questions.

AaronBallman · 2024-11-04T20:25:39Z

clang-tools-extra/clang-query/QueryProfile.h

+  ~QueryProfile();
+
+private:
+  std::optional<llvm::TimerGroup> TG;


Why is this optional?

No need to be optional, I'll remove it.

AaronBallman · 2024-11-04T20:27:25Z

clang-tools-extra/clang-query/QueryProfile.h

+
+class QueryProfile {
+public:
+  llvm::StringMap<llvm::TimeRecord> Records;


Rather than leave this public, should we add an interface to insert a new record?

QueryProfile is a very small utility like class, I'll move it into Query.cpp.

QueryProfiler could be simplified to no more then 10 lines.

1. QueryProfiler is very small, no need to expose it; 2. TimerGroup is created when dumping profiling info, no need to be an optional member of QueryProfiler.

danix800 · 2024-11-05T04:33:19Z

This is really cool, thank you! Can you also add a release note to clang-tools-extra/docs/ReleaseNotes.rst so users know about the new functionality?

Changes generally look good, I just had a few questions.

ReleaseNotes updated.

AaronBallman

LGTM, thank you!

We've found that basic profiling could help improving/optimizing when developing clang-tidy checks. This PR adds an extra command ``` set enable-profile (true|false) Set whether to enable matcher profiling. ``` which enables profiling queries on each file. Sample output: ``` $ cat test.cql set enable-profile true m binaryOperator(isExpansionInMainFile()) $ cat test.c int test(int i, int j) { return i + j; } $ clang-query --track-memory -f test.cql test.c -- Match llvm#1: {{.*}}/test.c:2:10: note: "root" binds here 2 | return i + j; | ^~~~~ 1 match. ===-------------------------------------------------------------------------=== clang-query matcher profiling ===-------------------------------------------------------------------------=== Total Execution Time: 0.0000 seconds (0.0000 wall clock) ---User Time--- --System Time-- --User+System-- ---Wall Time--- ---Mem--- --- Name --- 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 224 {{.*}}/test.c 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 0.0000 (100.0%) 224 Total ```

llvmbot added the clang-tools-extra label Nov 4, 2024

danix800 added 2 commits November 4, 2024 22:50

fix typos

ccd96f0

fix punctuation error in command help text

57151c6

danix800 requested a review from AaronBallman November 4, 2024 15:08

AaronBallman reviewed Nov 4, 2024

View reviewed changes

danix800 added the clang-query label Nov 5, 2024

danix800 added 3 commits November 5, 2024 12:14

move QueryProfiler into Query.cpp

6b931b8

1. QueryProfiler is very small, no need to expose it; 2. TimerGroup is created when dumping profiling info, no need to be an optional member of QueryProfiler.

QueryProfiler further simplifiled

104f7f4

update ReleaseNotes.rst for this new profiling feature

6b2621b

AaronBallman approved these changes Nov 5, 2024

View reviewed changes

danix800 merged commit 03f06b9 into llvm:main Nov 5, 2024
9 checks passed

danix800 deleted the feature/clang-query-profiling branch November 5, 2024 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[clang-query] add basic profiling on matching each ASTs #114806

[clang-query] add basic profiling on matching each ASTs #114806

danix800 commented Nov 4, 2024 •

edited

Loading

llvmbot commented Nov 4, 2024

AaronBallman left a comment

AaronBallman Nov 4, 2024

danix800 Nov 5, 2024

AaronBallman Nov 4, 2024

danix800 Nov 5, 2024

danix800 Nov 5, 2024

danix800 commented Nov 5, 2024

AaronBallman left a comment

[clang-query] add basic profiling on matching each ASTs #114806

[clang-query] add basic profiling on matching each ASTs #114806

Conversation

danix800 commented Nov 4, 2024 • edited Loading

llvmbot commented Nov 4, 2024

AaronBallman left a comment

Choose a reason for hiding this comment

AaronBallman Nov 4, 2024

Choose a reason for hiding this comment

danix800 Nov 5, 2024

Choose a reason for hiding this comment

AaronBallman Nov 4, 2024

Choose a reason for hiding this comment

danix800 Nov 5, 2024

Choose a reason for hiding this comment

danix800 Nov 5, 2024

Choose a reason for hiding this comment

danix800 commented Nov 5, 2024

AaronBallman left a comment

Choose a reason for hiding this comment

danix800 commented Nov 4, 2024 •

edited

Loading