Skip to content

Commit 4ac7b80

Browse files
committed
[clangd] Get rid of ASTWorker::getCurrentFileInputs
Summary: 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. Reviewers: sammccall Subscribers: ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77309
1 parent 7fc599c commit 4ac7b80

File tree

1 file changed

+14
-26
lines changed

1 file changed

+14
-26
lines changed

clang-tools-extra/clangd/TUScheduler.cpp

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -423,10 +423,6 @@ class ASTWorker {
423423
Deadline scheduleLocked();
424424
/// Should the first task in the queue be skipped instead of run?
425425
bool shouldSkipHeadLocked() const;
426-
/// This is private because `FileInputs.FS` is not thread-safe and thus not
427-
/// safe to share. Callers should make sure not to expose `FS` via a public
428-
/// interface.
429-
std::shared_ptr<const ParseInputs> getCurrentFileInputs() const;
430426

431427
struct Request {
432428
llvm::unique_function<void()> Action;
@@ -458,9 +454,9 @@ class ASTWorker {
458454
/// Guards members used by both TUScheduler and the worker thread.
459455
mutable std::mutex Mutex;
460456
/// File inputs, currently being used by the worker.
461-
/// Inputs are written and read by the worker thread, compile command can also
462-
/// be consumed by clients of ASTWorker.
463-
std::shared_ptr<const ParseInputs> FileInputs; /* GUARDED_BY(Mutex) */
457+
/// Writes and reads from unknown threads are locked. Reads from the worker
458+
/// thread are not locked, as it's the only writer.
459+
ParseInputs FileInputs; /* GUARDED_BY(Mutex) */
464460
/// Times of recent AST rebuilds, used for UpdateDebounce computation.
465461
llvm::SmallVector<DebouncePolicy::clock::duration, 8>
466462
RebuildTimes; /* GUARDED_BY(Mutex) */
@@ -556,12 +552,10 @@ ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB,
556552
// FIXME: Run PreamblePeer asynchronously once ast patching
557553
// is available.
558554
/*RunSync=*/true, Status, *this) {
559-
auto Inputs = std::make_shared<ParseInputs>();
560555
// Set a fallback command because compile command can be accessed before
561556
// `Inputs` is initialized. Other fields are only used after initialization
562557
// from client inputs.
563-
Inputs->CompileCommand = CDB.getFallbackCommand(FileName);
564-
FileInputs = std::move(Inputs);
558+
FileInputs.CompileCommand = CDB.getFallbackCommand(FileName);
565559
}
566560

567561
ASTWorker::~ASTWorker() {
@@ -590,7 +584,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
590584
Inputs.CompileCommand = CDB.getFallbackCommand(FileName);
591585

592586
bool InputsAreTheSame =
593-
std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
587+
std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
594588
std::tie(Inputs.CompileCommand, Inputs.Contents);
595589
// Cached AST is invalidated.
596590
if (!InputsAreTheSame) {
@@ -601,7 +595,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
601595
// Update current inputs so that subsequent reads can see them.
602596
{
603597
std::lock_guard<std::mutex> Lock(Mutex);
604-
FileInputs = std::make_shared<ParseInputs>(Inputs);
598+
FileInputs = Inputs;
605599
}
606600

607601
log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
@@ -655,21 +649,20 @@ void ASTWorker::runWithAST(
655649
if (isCancelled())
656650
return Action(llvm::make_error<CancelledError>());
657651
llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
658-
auto CurrentInputs = getCurrentFileInputs();
659652
if (!AST) {
660653
StoreDiags CompilerInvocationDiagConsumer;
661-
std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
662-
*CurrentInputs, CompilerInvocationDiagConsumer);
654+
std::unique_ptr<CompilerInvocation> Invocation =
655+
buildCompilerInvocation(FileInputs, CompilerInvocationDiagConsumer);
663656
// Try rebuilding the AST.
664657
vlog("ASTWorker rebuilding evicted AST to run {0}: {1} version {2}", Name,
665-
FileName, CurrentInputs->Version);
658+
FileName, FileInputs.Version);
666659
// FIXME: We might need to build a patched ast once preamble thread starts
667660
// running async. Currently getPossiblyStalePreamble below will always
668661
// return a compatible preamble as ASTWorker::update blocks.
669662
llvm::Optional<ParsedAST> NewAST =
670663
Invocation ? buildAST(FileName, std::move(Invocation),
671664
CompilerInvocationDiagConsumer.take(),
672-
*CurrentInputs, getPossiblyStalePreamble())
665+
FileInputs, getPossiblyStalePreamble())
673666
: None;
674667
AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
675668
}
@@ -681,8 +674,8 @@ void ASTWorker::runWithAST(
681674
return Action(llvm::make_error<llvm::StringError>(
682675
"invalid AST", llvm::errc::invalid_argument));
683676
vlog("ASTWorker running {0} on version {2} of {1}", Name, FileName,
684-
CurrentInputs->Version);
685-
Action(InputsAndAST{*CurrentInputs, **AST});
677+
FileInputs.Version);
678+
Action(InputsAndAST{FileInputs, **AST});
686679
};
687680
startTask(Name, std::move(Task), /*UpdateType=*/None, Invalidation);
688681
}
@@ -782,7 +775,7 @@ void ASTWorker::generateDiagnostics(
782775
}
783776
// Used to check whether we can update AST cache.
784777
bool InputsAreLatest =
785-
std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
778+
std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
786779
std::tie(Inputs.CompileCommand, Inputs.Contents);
787780
// Take a shortcut and don't report the diagnostics, since they should be the
788781
// same. All the clients should handle the lack of OnUpdated() call anyway to
@@ -899,14 +892,9 @@ void ASTWorker::getCurrentPreamble(
899892

900893
void ASTWorker::waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
901894

902-
std::shared_ptr<const ParseInputs> ASTWorker::getCurrentFileInputs() const {
903-
std::unique_lock<std::mutex> Lock(Mutex);
904-
return FileInputs;
905-
}
906-
907895
tooling::CompileCommand ASTWorker::getCurrentCompileCommand() const {
908896
std::unique_lock<std::mutex> Lock(Mutex);
909-
return FileInputs->CompileCommand;
897+
return FileInputs.CompileCommand;
910898
}
911899

912900
std::size_t ASTWorker::getUsedBytes() const {

0 commit comments

Comments
 (0)