kuganv created this revision.
Herald added subscribers: kadircet, arphaman, javed.absar.
Herald added a project: All.
kuganv updated this revision to Diff 517147.
kuganv added a comment.
kuganv edited the summary of this revision.
kuganv added reviewers: kadircet, sam, ilya-biryukov, nridge.
kuganv added subscribers: DmitryPolukhin, ivanmurashko.
kuganv published this revision for review.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang-tools-extra.
Removed wrong assert
We would like to move the preamble index out of the critical path.
This patch is an RFC to get feedback on the correct implementation and
potential pitfalls to keep into consideration.
I am not entirely sure if the lazy AST initialisation would create using
Preamble AST in parallel. I tried with tsan enabled clangd but it seems to work
OK (at least for the cases I tried)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148088
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -335,6 +335,10 @@
/// Mostly useful for synchronizing tests.
bool blockUntilIdle(Deadline D) const;
+ AsyncTaskRunner* getPreambleIndexTasks() {
+ return PreambleIndexTasks ? &PreambleIndexTasks.value() : nullptr;
+ }
+
private:
/// This class stores per-file data in the Files map.
struct FileData;
@@ -371,6 +375,7 @@
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
std::optional<AsyncTaskRunner> PreambleTasks;
+ std::optional<AsyncTaskRunner> PreambleIndexTasks;
std::optional<AsyncTaskRunner> WorkerThreads;
// Used to create contexts for operations that are not bound to a particular
// file (e.g. index queries).
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1646,6 +1646,7 @@
}
if (0 < Opts.AsyncThreadsCount) {
PreambleTasks.emplace();
+ PreambleIndexTasks.emplace();
WorkerThreads.emplace();
}
}
@@ -1657,6 +1658,8 @@
// Wait for all in-flight tasks to finish.
if (PreambleTasks)
PreambleTasks->wait();
+ if (PreambleIndexTasks)
+ PreambleIndexTasks->wait();
if (WorkerThreads)
WorkerThreads->wait();
}
@@ -1668,6 +1671,9 @@
if (PreambleTasks)
if (!PreambleTasks->wait(D))
return false;
+ if (PreambleIndexTasks)
+ if (!PreambleIndexTasks->wait(D))
+ return false;
return true;
}
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -61,21 +61,34 @@
struct UpdateIndexCallbacks : public ParsingCallbacks {
UpdateIndexCallbacks(FileIndex *FIndex,
ClangdServer::Callbacks *ServerCallbacks,
- const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks)
+ const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks,
+ AsyncTaskRunner* PreambleIndexTask)
: FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
- Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks) {}
+ Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks),
+ PreambleIndexTask(PreambleIndexTask) {}
void onPreambleAST(PathRef Path, llvm::StringRef Version,
const CompilerInvocation &CI, ASTContext &Ctx,
Preprocessor &PP,
const CanonicalIncludes &CanonIncludes) override {
+ if (!FIndex)
+ return;
+
// If this preamble uses a standard library we haven't seen yet, index it.
- if (FIndex)
- if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
- indexStdlib(CI, std::move(*Loc));
+ if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
+ indexStdlib(CI, std::move(*Loc));
- if (FIndex)
- FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
+ auto Task = [FIndex(FIndex), Path(Path),
+ Version(Version), Ctx(&Ctx), PP(&PP),
+ CanonIncludes(CanonIncludes)] {
+ FIndex->updatePreamble(Path, Version, *Ctx, *PP, CanonIncludes);
+ };
+
+ if (PreambleIndexTask)
+ PreambleIndexTask->runAsync("task:" + Path + Version,
+ std::move(Task));
+ else
+ Task();
}
void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) {
@@ -139,6 +152,7 @@
const ThreadsafeFS &TFS;
std::shared_ptr<StdLibSet> Stdlib;
AsyncTaskRunner *Tasks;
+ AsyncTaskRunner *PreambleIndexTask;
};
class DraftStoreFS : public ThreadsafeFS {
@@ -201,7 +215,8 @@
WorkScheduler.emplace(CDB, TUScheduler::Options(Opts),
std::make_unique<UpdateIndexCallbacks>(
DynamicIdx.get(), Callbacks, TFS,
- IndexTasks ? &*IndexTasks : nullptr));
+ IndexTasks ? &*IndexTasks : nullptr,
+ WorkScheduler->getPreambleIndexTasks()));
// Adds an index to the stack, at higher priority than existing indexes.
auto AddIndex = [&](SymbolIndex *Idx) {
if (this->Index != nullptr) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
