kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
javed.absar, ilya-biryukov.
Herald added a project: clang.
kadircet added a parent revision: D76725: [clangd] Build ASTs only with fresh
preambles or after building a new preamble.
FileInputs are only written by ASTWorker thread, therefore it is safe
to read them without the lock inside that thread. It can still be read by other
threads through ASTWorker::getCurrentCompileCommand though.
This patch also gets rid of the smart pointer wrapping FileInputs as there is
never mutliple owners.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77309
Files:
clang-tools-extra/clangd/TUScheduler.cpp
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -430,10 +430,6 @@
Deadline scheduleLocked();
/// Should the first task in the queue be skipped instead of run?
bool shouldSkipHeadLocked() const;
- /// This is private because `FileInputs.FS` is not thread-safe and thus not
- /// safe to share. Callers should make sure not to expose `FS` via a public
- /// interface.
- std::shared_ptr<const ParseInputs> getCurrentFileInputs() const;
struct Request {
llvm::unique_function<void()> Action;
@@ -467,7 +463,7 @@
/// File inputs, currently being used by the worker.
/// Inputs are written and read by the worker thread, compile command can also
/// be consumed by clients of ASTWorker.
- std::shared_ptr<const ParseInputs> FileInputs; /* GUARDED_BY(Mutex) */
+ ParseInputs FileInputs; /* GUARDED_BY(Mutex) */
/// Times of recent AST rebuilds, used for UpdateDebounce computation.
llvm::SmallVector<DebouncePolicy::clock::duration, 8>
RebuildTimes; /* GUARDED_BY(Mutex) */
@@ -563,12 +559,10 @@
// FIXME: Run PreamblePeer asynchronously once ast patching
// is available.
/*RunSync=*/true, Status, *this) {
- auto Inputs = std::make_shared<ParseInputs>();
// Set a fallback command because compile command can be accessed before
// `Inputs` is initialized. Other fields are only used after initialization
// from client inputs.
- Inputs->CompileCommand = CDB.getFallbackCommand(FileName);
- FileInputs = std::move(Inputs);
+ FileInputs.CompileCommand = CDB.getFallbackCommand(FileName);
}
ASTWorker::~ASTWorker() {
@@ -597,7 +591,7 @@
Inputs.CompileCommand = CDB.getFallbackCommand(FileName);
bool InputsAreTheSame =
- std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
+ std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
std::tie(Inputs.CompileCommand, Inputs.Contents);
// Cached AST is invalidated.
if (!InputsAreTheSame) {
@@ -608,7 +602,7 @@
// Update current inputs so that subsequent reads can see them.
{
std::lock_guard<std::mutex> Lock(Mutex);
- FileInputs = std::make_shared<ParseInputs>(Inputs);
+ FileInputs = Inputs;
}
log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
@@ -662,14 +656,13 @@
if (isCancelled())
return Action(llvm::make_error<CancelledError>());
llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
- auto CurrentInputs = getCurrentFileInputs();
if (!AST) {
StoreDiags CompilerInvocationDiagConsumer;
- std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
- *CurrentInputs, CompilerInvocationDiagConsumer);
+ std::unique_ptr<CompilerInvocation> Invocation =
+ buildCompilerInvocation(FileInputs, CompilerInvocationDiagConsumer);
// Try rebuilding the AST.
vlog("ASTWorker rebuilding evicted AST to run {0}: {1} version {2}", Name,
- FileName, CurrentInputs->Version);
+ FileName, FileInputs.Version);
// FIXME: We might need to build a patched ast once preamble thread starts
// running async. Currently getPossiblyStalePreamble below will always
// return a compatible preamble as ASTWorker::update blocks.
@@ -677,7 +670,7 @@
Invocation
? buildAST(FileName,
std::make_unique<CompilerInvocation>(*Invocation),
- CompilerInvocationDiagConsumer.take(), *CurrentInputs,
+ CompilerInvocationDiagConsumer.take(), FileInputs,
getPossiblyStalePreamble())
: None;
AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
@@ -690,8 +683,8 @@
return Action(llvm::make_error<llvm::StringError>(
"invalid AST", llvm::errc::invalid_argument));
vlog("ASTWorker running {0} on version {2} of {1}", Name, FileName,
- CurrentInputs->Version);
- Action(InputsAndAST{*CurrentInputs, **AST});
+ FileInputs.Version);
+ Action(InputsAndAST{FileInputs, **AST});
};
startTask(Name, std::move(Task), /*UpdateType=*/None, Invalidation);
}
@@ -791,7 +784,7 @@
}
// Used to check whether we can update AST cache.
bool InputsAreTheSame =
- std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
+ std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
std::tie(Inputs.CompileCommand, Inputs.Contents);
// Take a shortcut and don't report the diagnostics, since they should be the
// same. All the clients should handle the lack of OnUpdated() call anyway to
@@ -904,14 +897,9 @@
void ASTWorker::waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
-std::shared_ptr<const ParseInputs> ASTWorker::getCurrentFileInputs() const {
- std::unique_lock<std::mutex> Lock(Mutex);
- return FileInputs;
-}
-
tooling::CompileCommand ASTWorker::getCurrentCompileCommand() const {
std::unique_lock<std::mutex> Lock(Mutex);
- return FileInputs->CompileCommand;
+ return FileInputs.CompileCommand;
}
std::size_t ASTWorker::getUsedBytes() const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
