Skip to content

Commit

Permalink
[clangd] Initialize clang-tidy modules only once
Browse files Browse the repository at this point in the history 
This is showing up on our profiles with ~100ms contribution @95th% for
buildAST latencies.
The patch is unlikely to address it all, but should help with some low-hanging
fruit.

Differential Revision: https://reviews.llvm.org/D150257
  • Loading branch information
@kadircet
kadircet committed May 10, 2023
1 parent 25495c9 commit 62a090f
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 138 deletions.
20 changes: 10 additions & 10 deletions clang-tools-extra/clangd/ParsedAST.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "ParsedAST.h"
#include "../clang-tidy/ClangTidyCheck.h"
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
#include "../clang-tidy/ClangTidyModule.h"
#include "../clang-tidy/ClangTidyModuleRegistry.h"
#include "AST.h"
#include "Compiler.h"
Expand All @@ -25,7 +26,6 @@
#include "TidyProvider.h"
#include "clang-include-cleaner/Record.h"
#include "index/CanonicalIncludes.h"
#include "index/Index.h"
#include "index/Symbol.h"
#include "support/Logger.h"
#include "support/Trace.h"
Expand All @@ -50,7 +50,6 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <algorithm>
#include <memory>
#include <optional>
#include <vector>
Expand Down Expand Up @@ -476,16 +475,19 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
// diagnostics.
if (PreserveDiags) {
trace::Span Tracer("ClangTidyInit");
tidy::ClangTidyCheckFactories CTFactories;
for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
E.instantiate()->addCheckFactories(CTFactories);
static const auto *CTFactories = [] {
auto *CTFactories = new tidy::ClangTidyCheckFactories;
for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
E.instantiate()->addCheckFactories(*CTFactories);
return CTFactories;
}();
CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
CTContext->setASTContext(&Clang->getASTContext());
CTContext->setCurrentFile(Filename);
CTContext->setSelfContainedDiags(true);
CTChecks = CTFactories.createChecksForLanguage(&*CTContext);
CTChecks = CTFactories->createChecksForLanguage(&*CTContext);
Preprocessor *PP = &Clang->getPreprocessor();
for (const auto &Check : CTChecks) {
Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
Expand Down Expand Up @@ -610,10 +612,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
Macros = Patch->mainFileMacros();
Marks = Patch->marks();
}
auto& PP = Clang->getPreprocessor();
PP.addPPCallbacks(
std::make_unique<CollectMainFileMacros>(
PP, Macros));
auto &PP = Clang->getPreprocessor();
PP.addPPCallbacks(std::make_unique<CollectMainFileMacros>(PP, Macros));

PP.addPPCallbacks(
collectPragmaMarksCallback(Clang->getSourceManager(), Marks));
Expand Down
12 changes: 10 additions & 2 deletions clang-tools-extra/clangd/TidyProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "TidyProvider.h"
#include "../clang-tidy/ClangTidyModuleRegistry.h"
#include "../clang-tidy/ClangTidyOptions.h"
#include "Config.h"
#include "support/FileCache.h"
#include "support/Logger.h"
Expand Down Expand Up @@ -283,8 +284,15 @@ TidyProvider combine(std::vector<TidyProvider> Providers) {

tidy::ClangTidyOptions getTidyOptionsForFile(TidyProviderRef Provider,
llvm::StringRef Filename) {
tidy::ClangTidyOptions Opts = tidy::ClangTidyOptions::getDefaults();
Opts.Checks->clear();
// getDefaults instantiates all check factories, which are registered at link
// time. So cache the results once.
static const auto *DefaultOpts = [] {
auto *Opts = new tidy::ClangTidyOptions;
*Opts = tidy::ClangTidyOptions::getDefaults();
Opts->Checks->clear();
return Opts;
}();
auto Opts = *DefaultOpts;
if (Provider)
Provider(Opts, Filename);
return Opts;
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/unittests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ add_unittest(ClangdUnitTests ClangdTests
PrintASTTests.cpp
ProjectAwareIndexTests.cpp
QualityTests.cpp
RenameTests.cpp
RIFFTests.cpp
RenameTests.cpp
ReplayPeambleTests.cpp
SelectionTests.cpp
SemanticHighlightingTests.cpp
SemanticSelectionTests.cpp
Expand Down
125 changes: 0 additions & 125 deletions clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
//===----------------------------------------------------------------------===//

#include "../../clang-tidy/ClangTidyCheck.h"
#include "../../clang-tidy/ClangTidyModule.h"
#include "../../clang-tidy/ClangTidyModuleRegistry.h"
#include "AST.h"
#include "Annotations.h"
#include "Compiler.h"
Expand All @@ -29,8 +27,6 @@
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TokenKinds.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Token.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Testing/Support/Error.h"
Expand Down Expand Up @@ -96,10 +92,6 @@ MATCHER_P(withTemplateArgs, ArgName, "") {
return false;
}

MATCHER_P(rangeIs, R, "") {
return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
}

MATCHER_P(pragmaTrivia, P, "") { return arg.Trivia == P; }

MATCHER(eqInc, "") {
Expand Down Expand Up @@ -352,123 +344,6 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {

MATCHER_P(withFileName, Inc, "") { return arg.FileName == Inc; }

TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
struct Inclusion {
Inclusion(const SourceManager &SM, SourceLocation HashLoc,
const Token &IncludeTok, llvm::StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange)
: HashOffset(SM.getDecomposedLoc(HashLoc).second), IncTok(IncludeTok),
IncDirective(IncludeTok.getIdentifierInfo()->getName()),
FileNameOffset(SM.getDecomposedLoc(FilenameRange.getBegin()).second),
FileName(FileName), IsAngled(IsAngled) {
EXPECT_EQ(
toSourceCode(SM, FilenameRange.getAsRange()).drop_back().drop_front(),
FileName);
}
size_t HashOffset;
syntax::Token IncTok;
llvm::StringRef IncDirective;
size_t FileNameOffset;
llvm::StringRef FileName;
bool IsAngled;
};
static std::vector<Inclusion> Includes;
static std::vector<syntax::Token> SkippedFiles;
struct ReplayPreamblePPCallback : public PPCallbacks {
const SourceManager &SM;
explicit ReplayPreamblePPCallback(const SourceManager &SM) : SM(SM) {}

void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
StringRef FileName, bool IsAngled,
CharSourceRange FilenameRange, OptionalFileEntryRef,
StringRef, StringRef, const clang::Module *,
SrcMgr::CharacteristicKind) override {
Includes.emplace_back(SM, HashLoc, IncludeTok, FileName, IsAngled,
FilenameRange);
}

void FileSkipped(const FileEntryRef &, const Token &FilenameTok,
SrcMgr::CharacteristicKind) override {
SkippedFiles.emplace_back(FilenameTok);
}
};
struct ReplayPreambleCheck : public tidy::ClangTidyCheck {
ReplayPreambleCheck(StringRef Name, tidy::ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override {
PP->addPPCallbacks(::std::make_unique<ReplayPreamblePPCallback>(SM));
}
};
struct ReplayPreambleModule : public tidy::ClangTidyModule {
void
addCheckFactories(tidy::ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<ReplayPreambleCheck>(
"replay-preamble-check");
}
};

static tidy::ClangTidyModuleRegistry::Add<ReplayPreambleModule> X(
"replay-preamble-module", "");
TestTU TU;
// This check records inclusion directives replayed by clangd.
TU.ClangTidyProvider = addTidyChecks("replay-preamble-check");
llvm::Annotations Test(R"cpp(
$hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
$hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
$hash^#$include[[include]] $filebegin^<$filerange[[a.h]]>)cpp");
llvm::StringRef Code = Test.code();
TU.Code = Code.str();
TU.AdditionalFiles["bar.h"] = "";
TU.AdditionalFiles["baz.h"] = "";
TU.AdditionalFiles["a.h"] = "";
// Since we are also testing #import directives, and they don't make much
// sense in c++ (also they actually break on windows), just set language to
// obj-c.
TU.ExtraArgs = {"-isystem.", "-xobjective-c"};

const auto &AST = TU.build();
const auto &SM = AST.getSourceManager();

auto HashLocs = Test.points("hash");
ASSERT_EQ(HashLocs.size(), Includes.size());
auto IncludeRanges = Test.ranges("include");
ASSERT_EQ(IncludeRanges.size(), Includes.size());
auto FileBeginLocs = Test.points("filebegin");
ASSERT_EQ(FileBeginLocs.size(), Includes.size());
auto FileRanges = Test.ranges("filerange");
ASSERT_EQ(FileRanges.size(), Includes.size());

ASSERT_EQ(SkippedFiles.size(), Includes.size());
for (size_t I = 0; I < Includes.size(); ++I) {
const auto &Inc = Includes[I];

EXPECT_EQ(Inc.HashOffset, HashLocs[I]);

auto IncRange = IncludeRanges[I];
EXPECT_THAT(Inc.IncTok.range(SM), rangeIs(IncRange));
EXPECT_EQ(Inc.IncTok.kind(), tok::identifier);
EXPECT_EQ(Inc.IncDirective,
Code.substr(IncRange.Begin, IncRange.End - IncRange.Begin));

EXPECT_EQ(Inc.FileNameOffset, FileBeginLocs[I]);
EXPECT_EQ(Inc.IsAngled, Code[FileBeginLocs[I]] == '<');

auto FileRange = FileRanges[I];
EXPECT_EQ(Inc.FileName,
Code.substr(FileRange.Begin, FileRange.End - FileRange.Begin));

EXPECT_EQ(SM.getDecomposedLoc(SkippedFiles[I].location()).second,
Inc.FileNameOffset);
// This also contains quotes/angles so increment the range by one from both
// sides.
EXPECT_EQ(
SkippedFiles[I].text(SM),
Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
}
}

TEST(ParsedASTTest, PatchesAdditionalIncludes) {
llvm::StringLiteral ModifiedContents = R"cpp(
#include "baz.h"
Expand Down

0 comments on commit 62a090f

Please sign in to comment.