njames93 updated this revision to Diff 401197. njames93 added a comment. Address comments
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117529/new/ https://reviews.llvm.org/D117529 Files: clang-tools-extra/clang-tidy/ClangTidyModule.h clang-tools-extra/clangd/ParsedAST.cpp Index: clang-tools-extra/clangd/ParsedAST.cpp =================================================================== --- clang-tools-extra/clangd/ParsedAST.cpp +++ clang-tools-extra/clangd/ParsedAST.cpp @@ -10,6 +10,7 @@ #include "../clang-tidy/ClangTidyCheck.h" #include "../clang-tidy/ClangTidyDiagnosticConsumer.h" #include "../clang-tidy/ClangTidyModuleRegistry.h" +#include "../clang-tidy/GlobList.h" #include "AST.h" #include "Compiler.h" #include "Config.h" @@ -282,6 +283,42 @@ } } +std::vector<std::unique_ptr<tidy::ClangTidyCheck>> +buildChecks(tidy::ClangTidyContext *Context) { + using FactoryFunc = std::function<std::unique_ptr<tidy::ClangTidyCheck>( + llvm::StringRef, tidy::ClangTidyContext *)>; + const llvm::Optional<std::string> &CheckGlob = Context->getOptions().Checks; + if (!CheckGlob || CheckGlob->empty()) + return {}; + thread_local std::pair<std::string, + std::vector<std::pair<std::string, FactoryFunc>>> + Cached; + static std::atomic<size_t> CacheHit, CacheMiss; + if (*CheckGlob == Cached.first) { + ++CacheHit; + } else { + ++CacheMiss; + vlog("ClangTidyCheck factory cache miss '{0:P2}%'\n" + "Rebuilding ClangTidyChecks factory with new glob '{1}'", + static_cast<double>(CacheHit) / CacheMiss, *CheckGlob); + Cached.first.assign(CheckGlob->data(), CheckGlob->size()); + Cached.second.clear(); + tidy::ClangTidyCheckFactories CTFactories; + for (const auto &E : tidy::ClangTidyModuleRegistry::entries()) + E.instantiate()->addCheckFactories(CTFactories); + tidy::GlobList Glob(*CheckGlob); + for (const auto &Check : CTFactories) { + if (Glob.contains(Check.getKey())) + Cached.second.emplace_back(Check.getKey(), Check.getValue()); + } + } + std::vector<std::unique_ptr<tidy::ClangTidyCheck>> Result; + Result.reserve(Cached.second.size()); + for (auto &Factory : Cached.second) { + Result.push_back(Factory.second(Factory.first, Context)); + } + return Result; +} } // namespace llvm::Optional<ParsedAST> @@ -410,15 +447,12 @@ // diagnostics. if (PreserveDiags) { trace::Span Tracer("ClangTidyInit"); - tidy::ClangTidyCheckFactories CTFactories; - for (const auto &E : tidy::ClangTidyModuleRegistry::entries()) - E.instantiate()->addCheckFactories(CTFactories); CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>( tidy::ClangTidyGlobalOptions(), ClangTidyOpts)); CTContext->setDiagnosticsEngine(&Clang->getDiagnostics()); CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(Filename); - CTChecks = CTFactories.createChecks(CTContext.getPointer()); + CTChecks = buildChecks(CTContext.getPointer()); llvm::erase_if(CTChecks, [&](const auto &Check) { return !Check->isLanguageVersionSupported(CTContext->getLangOpts()); }); Index: clang-tools-extra/clang-tidy/ClangTidyModule.h =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyModule.h +++ clang-tools-extra/clang-tidy/ClangTidyModule.h @@ -71,6 +71,7 @@ FactoryMap::const_iterator begin() const { return Factories.begin(); } FactoryMap::const_iterator end() const { return Factories.end(); } bool empty() const { return Factories.empty(); } + size_t size() const { return Factories.size(); } private: FactoryMap Factories;
Index: clang-tools-extra/clangd/ParsedAST.cpp =================================================================== --- clang-tools-extra/clangd/ParsedAST.cpp +++ clang-tools-extra/clangd/ParsedAST.cpp @@ -10,6 +10,7 @@ #include "../clang-tidy/ClangTidyCheck.h" #include "../clang-tidy/ClangTidyDiagnosticConsumer.h" #include "../clang-tidy/ClangTidyModuleRegistry.h" +#include "../clang-tidy/GlobList.h" #include "AST.h" #include "Compiler.h" #include "Config.h" @@ -282,6 +283,42 @@ } } +std::vector<std::unique_ptr<tidy::ClangTidyCheck>> +buildChecks(tidy::ClangTidyContext *Context) { + using FactoryFunc = std::function<std::unique_ptr<tidy::ClangTidyCheck>( + llvm::StringRef, tidy::ClangTidyContext *)>; + const llvm::Optional<std::string> &CheckGlob = Context->getOptions().Checks; + if (!CheckGlob || CheckGlob->empty()) + return {}; + thread_local std::pair<std::string, + std::vector<std::pair<std::string, FactoryFunc>>> + Cached; + static std::atomic<size_t> CacheHit, CacheMiss; + if (*CheckGlob == Cached.first) { + ++CacheHit; + } else { + ++CacheMiss; + vlog("ClangTidyCheck factory cache miss '{0:P2}%'\n" + "Rebuilding ClangTidyChecks factory with new glob '{1}'", + static_cast<double>(CacheHit) / CacheMiss, *CheckGlob); + Cached.first.assign(CheckGlob->data(), CheckGlob->size()); + Cached.second.clear(); + tidy::ClangTidyCheckFactories CTFactories; + for (const auto &E : tidy::ClangTidyModuleRegistry::entries()) + E.instantiate()->addCheckFactories(CTFactories); + tidy::GlobList Glob(*CheckGlob); + for (const auto &Check : CTFactories) { + if (Glob.contains(Check.getKey())) + Cached.second.emplace_back(Check.getKey(), Check.getValue()); + } + } + std::vector<std::unique_ptr<tidy::ClangTidyCheck>> Result; + Result.reserve(Cached.second.size()); + for (auto &Factory : Cached.second) { + Result.push_back(Factory.second(Factory.first, Context)); + } + return Result; +} } // namespace llvm::Optional<ParsedAST> @@ -410,15 +447,12 @@ // diagnostics. if (PreserveDiags) { trace::Span Tracer("ClangTidyInit"); - tidy::ClangTidyCheckFactories CTFactories; - for (const auto &E : tidy::ClangTidyModuleRegistry::entries()) - E.instantiate()->addCheckFactories(CTFactories); CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>( tidy::ClangTidyGlobalOptions(), ClangTidyOpts)); CTContext->setDiagnosticsEngine(&Clang->getDiagnostics()); CTContext->setASTContext(&Clang->getASTContext()); CTContext->setCurrentFile(Filename); - CTChecks = CTFactories.createChecks(CTContext.getPointer()); + CTChecks = buildChecks(CTContext.getPointer()); llvm::erase_if(CTChecks, [&](const auto &Check) { return !Check->isLanguageVersionSupported(CTContext->getLangOpts()); }); Index: clang-tools-extra/clang-tidy/ClangTidyModule.h =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyModule.h +++ clang-tools-extra/clang-tidy/ClangTidyModule.h @@ -71,6 +71,7 @@ FactoryMap::const_iterator begin() const { return Factories.begin(); } FactoryMap::const_iterator end() const { return Factories.end(); } bool empty() const { return Factories.empty(); } + size_t size() const { return Factories.size(); } private: FactoryMap Factories;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits