sammccall added a comment.
Happy to take a look at this, but is there a particular motive for optimizing
this?
Looking at some profiles this appears to be something like 0.1-0.5ms in fairly
complex configurations.
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:289
+// necessary checks efficiently.
+class CachedFactories {
+ using FactoryFunc = std::function<std::unique_ptr<tidy::ClangTidyCheck>(
----------------
if it's possible, it seems like this class really wants to be a function
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:293
+ llvm::SmallVector<std::pair<std::string, FactoryFunc>, 0> AllChecks;
+ std::mutex Guard;
+ llvm::StringMap<llvm::BitVector> CachedGlobs;
----------------
I wonder if a global map is overkill (and the locking maybe undesirable).
In practice, this is being called in a loop on a single thread (the AST worker
thread), with no other calls on that thread.
The idea with caching is that the `checks` globs are usually the same, that is
especially true for parses of the main file.
So maybe we could just use a single-entry `thread_local` cache, with no map and
no locking?
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:296
+
+ llvm::BitVector &getCachedValue(StringRef CheckGlobString) {
+ std::lock_guard<std::mutex> LG(Guard);
----------------
why cache a bitmap instead of the vector<factory> directly?
================
Comment at: clang-tools-extra/clangd/ParsedAST.cpp:468
trace::Span Tracer("ClangTidyInit");
- tidy::ClangTidyCheckFactories CTFactories;
- for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
- E.instantiate()->addCheckFactories(CTFactories);
+ static CachedFactories Factory;
CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
----------------
we need to be more careful with static state than this. This adds global
destructors and we need to suppress them. (e.g. by using `new` without delete)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117529/new/
https://reviews.llvm.org/D117529
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
