Skip to content

Commit 62a090f

Browse files
committed
[clangd] Initialize clang-tidy modules only once
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
1 parent 25495c9 commit 62a090f

File tree

5 files changed

+188
-138
lines changed

5 files changed

+188
-138
lines changed

clang-tools-extra/clangd/ParsedAST.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "ParsedAST.h"
1010
#include "../clang-tidy/ClangTidyCheck.h"
1111
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
12+
#include "../clang-tidy/ClangTidyModule.h"
1213
#include "../clang-tidy/ClangTidyModuleRegistry.h"
1314
#include "AST.h"
1415
#include "Compiler.h"
@@ -25,7 +26,6 @@
2526
#include "TidyProvider.h"
2627
#include "clang-include-cleaner/Record.h"
2728
#include "index/CanonicalIncludes.h"
28-
#include "index/Index.h"
2929
#include "index/Symbol.h"
3030
#include "support/Logger.h"
3131
#include "support/Trace.h"
@@ -50,7 +50,6 @@
5050
#include "llvm/ADT/STLExtras.h"
5151
#include "llvm/ADT/SmallVector.h"
5252
#include "llvm/ADT/StringRef.h"
53-
#include <algorithm>
5453
#include <memory>
5554
#include <optional>
5655
#include <vector>
@@ -476,16 +475,19 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
476475
// diagnostics.
477476
if (PreserveDiags) {
478477
trace::Span Tracer("ClangTidyInit");
479-
tidy::ClangTidyCheckFactories CTFactories;
480-
for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
481-
E.instantiate()->addCheckFactories(CTFactories);
478+
static const auto *CTFactories = [] {
479+
auto *CTFactories = new tidy::ClangTidyCheckFactories;
480+
for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
481+
E.instantiate()->addCheckFactories(*CTFactories);
482+
return CTFactories;
483+
}();
482484
CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
483485
tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
484486
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
485487
CTContext->setASTContext(&Clang->getASTContext());
486488
CTContext->setCurrentFile(Filename);
487489
CTContext->setSelfContainedDiags(true);
488-
CTChecks = CTFactories.createChecksForLanguage(&*CTContext);
490+
CTChecks = CTFactories->createChecksForLanguage(&*CTContext);
489491
Preprocessor *PP = &Clang->getPreprocessor();
490492
for (const auto &Check : CTChecks) {
491493
Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
@@ -610,10 +612,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
610612
Macros = Patch->mainFileMacros();
611613
Marks = Patch->marks();
612614
}
613-
auto& PP = Clang->getPreprocessor();
614-
PP.addPPCallbacks(
615-
std::make_unique<CollectMainFileMacros>(
616-
PP, Macros));
615+
auto &PP = Clang->getPreprocessor();
616+
PP.addPPCallbacks(std::make_unique<CollectMainFileMacros>(PP, Macros));
617617

618618
PP.addPPCallbacks(
619619
collectPragmaMarksCallback(Clang->getSourceManager(), Marks));

clang-tools-extra/clangd/TidyProvider.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "TidyProvider.h"
1010
#include "../clang-tidy/ClangTidyModuleRegistry.h"
11+
#include "../clang-tidy/ClangTidyOptions.h"
1112
#include "Config.h"
1213
#include "support/FileCache.h"
1314
#include "support/Logger.h"
@@ -283,8 +284,15 @@ TidyProvider combine(std::vector<TidyProvider> Providers) {
283284

284285
tidy::ClangTidyOptions getTidyOptionsForFile(TidyProviderRef Provider,
285286
llvm::StringRef Filename) {
286-
tidy::ClangTidyOptions Opts = tidy::ClangTidyOptions::getDefaults();
287-
Opts.Checks->clear();
287+
// getDefaults instantiates all check factories, which are registered at link
288+
// time. So cache the results once.
289+
static const auto *DefaultOpts = [] {
290+
auto *Opts = new tidy::ClangTidyOptions;
291+
*Opts = tidy::ClangTidyOptions::getDefaults();
292+
Opts->Checks->clear();
293+
return Opts;
294+
}();
295+
auto Opts = *DefaultOpts;
288296
if (Provider)
289297
Provider(Opts, Filename);
290298
return Opts;

clang-tools-extra/clangd/unittests/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,9 @@ add_unittest(ClangdUnitTests ClangdTests
7979
PrintASTTests.cpp
8080
ProjectAwareIndexTests.cpp
8181
QualityTests.cpp
82-
RenameTests.cpp
8382
RIFFTests.cpp
83+
RenameTests.cpp
84+
ReplayPeambleTests.cpp
8485
SelectionTests.cpp
8586
SemanticHighlightingTests.cpp
8687
SemanticSelectionTests.cpp

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

Lines changed: 0 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "../../clang-tidy/ClangTidyCheck.h"
15-
#include "../../clang-tidy/ClangTidyModule.h"
16-
#include "../../clang-tidy/ClangTidyModuleRegistry.h"
1715
#include "AST.h"
1816
#include "Annotations.h"
1917
#include "Compiler.h"
@@ -29,8 +27,6 @@
2927
#include "clang/Basic/SourceLocation.h"
3028
#include "clang/Basic/SourceManager.h"
3129
#include "clang/Basic/TokenKinds.h"
32-
#include "clang/Lex/PPCallbacks.h"
33-
#include "clang/Lex/Token.h"
3430
#include "clang/Tooling/Syntax/Tokens.h"
3531
#include "llvm/ADT/StringRef.h"
3632
#include "llvm/Testing/Support/Error.h"
@@ -96,10 +92,6 @@ MATCHER_P(withTemplateArgs, ArgName, "") {
9692
return false;
9793
}
9894

99-
MATCHER_P(rangeIs, R, "") {
100-
return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
101-
}
102-
10395
MATCHER_P(pragmaTrivia, P, "") { return arg.Trivia == P; }
10496

10597
MATCHER(eqInc, "") {
@@ -352,123 +344,6 @@ TEST(ParsedASTTest, CollectsMainFileMacroExpansions) {
352344

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

355-
TEST(ParsedASTTest, ReplayPreambleForTidyCheckers) {
356-
struct Inclusion {
357-
Inclusion(const SourceManager &SM, SourceLocation HashLoc,
358-
const Token &IncludeTok, llvm::StringRef FileName, bool IsAngled,
359-
CharSourceRange FilenameRange)
360-
: HashOffset(SM.getDecomposedLoc(HashLoc).second), IncTok(IncludeTok),
361-
IncDirective(IncludeTok.getIdentifierInfo()->getName()),
362-
FileNameOffset(SM.getDecomposedLoc(FilenameRange.getBegin()).second),
363-
FileName(FileName), IsAngled(IsAngled) {
364-
EXPECT_EQ(
365-
toSourceCode(SM, FilenameRange.getAsRange()).drop_back().drop_front(),
366-
FileName);
367-
}
368-
size_t HashOffset;
369-
syntax::Token IncTok;
370-
llvm::StringRef IncDirective;
371-
size_t FileNameOffset;
372-
llvm::StringRef FileName;
373-
bool IsAngled;
374-
};
375-
static std::vector<Inclusion> Includes;
376-
static std::vector<syntax::Token> SkippedFiles;
377-
struct ReplayPreamblePPCallback : public PPCallbacks {
378-
const SourceManager &SM;
379-
explicit ReplayPreamblePPCallback(const SourceManager &SM) : SM(SM) {}
380-
381-
void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
382-
StringRef FileName, bool IsAngled,
383-
CharSourceRange FilenameRange, OptionalFileEntryRef,
384-
StringRef, StringRef, const clang::Module *,
385-
SrcMgr::CharacteristicKind) override {
386-
Includes.emplace_back(SM, HashLoc, IncludeTok, FileName, IsAngled,
387-
FilenameRange);
388-
}
389-
390-
void FileSkipped(const FileEntryRef &, const Token &FilenameTok,
391-
SrcMgr::CharacteristicKind) override {
392-
SkippedFiles.emplace_back(FilenameTok);
393-
}
394-
};
395-
struct ReplayPreambleCheck : public tidy::ClangTidyCheck {
396-
ReplayPreambleCheck(StringRef Name, tidy::ClangTidyContext *Context)
397-
: ClangTidyCheck(Name, Context) {}
398-
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
399-
Preprocessor *ModuleExpanderPP) override {
400-
PP->addPPCallbacks(::std::make_unique<ReplayPreamblePPCallback>(SM));
401-
}
402-
};
403-
struct ReplayPreambleModule : public tidy::ClangTidyModule {
404-
void
405-
addCheckFactories(tidy::ClangTidyCheckFactories &CheckFactories) override {
406-
CheckFactories.registerCheck<ReplayPreambleCheck>(
407-
"replay-preamble-check");
408-
}
409-
};
410-
411-
static tidy::ClangTidyModuleRegistry::Add<ReplayPreambleModule> X(
412-
"replay-preamble-module", "");
413-
TestTU TU;
414-
// This check records inclusion directives replayed by clangd.
415-
TU.ClangTidyProvider = addTidyChecks("replay-preamble-check");
416-
llvm::Annotations Test(R"cpp(
417-
$hash^#$include[[import]] $filebegin^"$filerange[[bar.h]]"
418-
$hash^#$include[[include_next]] $filebegin^"$filerange[[baz.h]]"
419-
$hash^#$include[[include]] $filebegin^<$filerange[[a.h]]>)cpp");
420-
llvm::StringRef Code = Test.code();
421-
TU.Code = Code.str();
422-
TU.AdditionalFiles["bar.h"] = "";
423-
TU.AdditionalFiles["baz.h"] = "";
424-
TU.AdditionalFiles["a.h"] = "";
425-
// Since we are also testing #import directives, and they don't make much
426-
// sense in c++ (also they actually break on windows), just set language to
427-
// obj-c.
428-
TU.ExtraArgs = {"-isystem.", "-xobjective-c"};
429-
430-
const auto &AST = TU.build();
431-
const auto &SM = AST.getSourceManager();
432-
433-
auto HashLocs = Test.points("hash");
434-
ASSERT_EQ(HashLocs.size(), Includes.size());
435-
auto IncludeRanges = Test.ranges("include");
436-
ASSERT_EQ(IncludeRanges.size(), Includes.size());
437-
auto FileBeginLocs = Test.points("filebegin");
438-
ASSERT_EQ(FileBeginLocs.size(), Includes.size());
439-
auto FileRanges = Test.ranges("filerange");
440-
ASSERT_EQ(FileRanges.size(), Includes.size());
441-
442-
ASSERT_EQ(SkippedFiles.size(), Includes.size());
443-
for (size_t I = 0; I < Includes.size(); ++I) {
444-
const auto &Inc = Includes[I];
445-
446-
EXPECT_EQ(Inc.HashOffset, HashLocs[I]);
447-
448-
auto IncRange = IncludeRanges[I];
449-
EXPECT_THAT(Inc.IncTok.range(SM), rangeIs(IncRange));
450-
EXPECT_EQ(Inc.IncTok.kind(), tok::identifier);
451-
EXPECT_EQ(Inc.IncDirective,
452-
Code.substr(IncRange.Begin, IncRange.End - IncRange.Begin));
453-
454-
EXPECT_EQ(Inc.FileNameOffset, FileBeginLocs[I]);
455-
EXPECT_EQ(Inc.IsAngled, Code[FileBeginLocs[I]] == '<');
456-
457-
auto FileRange = FileRanges[I];
458-
EXPECT_EQ(Inc.FileName,
459-
Code.substr(FileRange.Begin, FileRange.End - FileRange.Begin));
460-
461-
EXPECT_EQ(SM.getDecomposedLoc(SkippedFiles[I].location()).second,
462-
Inc.FileNameOffset);
463-
// This also contains quotes/angles so increment the range by one from both
464-
// sides.
465-
EXPECT_EQ(
466-
SkippedFiles[I].text(SM),
467-
Code.substr(FileRange.Begin - 1, FileRange.End - FileRange.Begin + 2));
468-
EXPECT_EQ(SkippedFiles[I].kind(), tok::header_name);
469-
}
470-
}
471-
472347
TEST(ParsedASTTest, PatchesAdditionalIncludes) {
473348
llvm::StringLiteral ModifiedContents = R"cpp(
474349
#include "baz.h"

0 commit comments

Comments
 (0)