[clangd] Record time spent in tidy checks

[sammccall]

My only concern with this is the overhead of the timing itself.
We look up the hash bucket, turn timers on/off...

If I'm understanding right we do this once per AST node per registered matcher. Are you able to see any performance difference with this tracing on vs off?

If there's a performance penalty but our goal is to gather enough statistics to be useful, we could consider only doing it every N reparses (or just with a certain probability).
But even then, if it's slow maybe we should only be running it in cases where we know we're sending the stats somewhere useful.

clang-tools-extra/clangd/ParsedAST.cpp

 #include "SourceCode.h"
 #include "TidyProvider.h"
 #include "index/CanonicalIncludes.h"
 #include "index/Index.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"

 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <memory>
 #include <vector>
 
 // Force the linker to link in Clang-tidy modules.
 // clangd doesn't support the static analyzer.
 #if CLANGD_TIDY_CHECKS
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
 
   // Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists.
   // Clang-tidy has some limitations to ensure reasonable performance:
   //  - checks don't see all preprocessor events in the preamble
   //  - matchers run only over the main-file top-level decls (and can't see
   //    ancestors outside this scope).
   // In practice almost all checks work well without modifications.
   std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
-  ast_matchers::MatchFinder CTFinder;
+  llvm::StringMap<llvm::TimeRecord> CTCheckTimings;
+  ast_matchers::MatchFinder::MatchFinderOptions CTFinderOpts;
+  CTFinderOpts.CheckProfiling.emplace(CTCheckTimings);
+  ast_matchers::MatchFinder CTFinder(CTFinderOpts);
   llvm::Optional<tidy::ClangTidyContext> CTContext;
   llvm::Optional<IncludeFixer> FixIncludes;
   // No need to run clang-tidy or IncludeFixerif we are not going to surface
   // diagnostics.
   if (PreserveDiags) {
     trace::Span Tracer("ClangTidyInit");
     tidy::ClangTidyOptions ClangTidyOpts =
         getTidyOptionsForFile(Inputs.ClangTidyProvider, Filename);
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
   std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls();
   // AST traversals should exclude the preamble, to avoid performance cliffs.
   Clang->getASTContext().setTraversalScope(ParsedDecls);
   if (!CTChecks.empty()) {
     // Run the AST-dependent part of the clang-tidy checks.
     // (The preprocessor part ran already, via PPCallbacks).
     trace::Span Tracer("ClangTidyMatch");
     CTFinder.matchAST(Clang->getASTContext());
+    static constexpr trace::Metric TidyCheckLatencies(
+        "tidy_check_latency", trace::Metric::Distribution, "check_name");
+    for (auto &Elem : CTCheckTimings)
+      TidyCheckLatencies.record(Elem.second.getWallTime(), Elem.first());
   }
 
   // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
   // However Action->EndSourceFile() would destroy the ASTContext!
   // So just inform the preprocessor of EOF, while keeping everything alive.
   Clang->getPreprocessor().EndSourceFile();
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "TidyProvider.h"
 #include "index/MemIndex.h"
 #include "support/Context.h"
 #include "support/Path.h"
+#include "support/TestTracer.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/TargetSelect.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <algorithm>
 #include <memory>
TEST(DiagnosticsTest, ClangTidy) {
   auto TU = TestTU::withCode(Test.code());
   TU.HeaderFilename = "assert.h"; // Suppress "not found" error.
   TU.ClangTidyProvider = addTidyChecks("bugprone-sizeof-expression,"
                                        "bugprone-macro-repeated-side-effects,"
                                        "modernize-deprecated-headers,"
                                        "modernize-use-trailing-return-type,"
                                        "misc-no-recursion");
   TU.ExtraArgs.push_back("-Wno-unsequenced");
+  trace::TestTracer Tracer;
   EXPECT_THAT(
       *TU.build().getDiagnostics(),
       ifTidyChecks(UnorderedElementsAre(
           AllOf(Diag(Test.range("deprecated"),
                      "inclusion of deprecated C++ header 'assert.h'; consider "
                      "using 'cassert' instead"),
                 DiagSource(Diag::ClangTidy),
                 DiagName("modernize-deprecated-headers"),
EXPECT_THAT(
                 DiagName("modernize-use-trailing-return-type"),
                 // Verify there's no "[check-name]" suffix in the message.
                 WithFix(FixMessage(
                     "use a trailing return type for this function"))),
           Diag(Test.range("foo"),
                "function 'foo' is within a recursive call chain"),
           Diag(Test.range("bar"),
                "function 'bar' is within a recursive call chain"))));
+  if (CLANGD_TIDY_CHECKS) {
+    EXPECT_THAT(Tracer.takeMetric("tidy_check_latency",
+                                  "modernize-use-trailing-return-type"),
+                Not(IsEmpty()));
+  }
 }
 
 TEST(DiagnosticsTest, ClangTidyEOF) {
   // clang-format off
   Annotations Test(R"cpp(
   [[#]]include <b.h>
   #include "a.h")cpp");
   // clang-format on