kadircet updated this revision to Diff 252633.
kadircet added a comment.
- Build golden asts in ASTWorker thread, rather than preamble thread.
- Explain new model in file comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76725/new/
https://reviews.llvm.org/D76725
Files:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/unittests/FileIndexTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -66,8 +66,7 @@
auto CI = buildCompilerInvocation(Inputs, Diags);
assert(CI && "Failed to build compilation invocation.");
auto Preamble =
- buildPreamble(FullFilename, *CI,
- /*OldPreamble=*/nullptr, Inputs,
+ buildPreamble(FullFilename, *CI, Inputs,
/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
auto AST =
buildAST(FullFilename, std::move(CI), Diags.take(), Inputs, Preamble);
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -283,6 +283,7 @@
S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
[&](Expected<InputsAndPreamble> Pre) {
ASSERT_TRUE(bool(Pre));
+ ASSERT_TRUE(Pre->Preamble);
EXPECT_EQ(Pre->Preamble->Version, "A");
EXPECT_THAT(includes(Pre->Preamble),
ElementsAre("<A>"));
@@ -292,11 +293,13 @@
S.runWithPreamble("ConsistentRead", Path, TUScheduler::Consistent,
[&](Expected<InputsAndPreamble> Pre) {
ASSERT_TRUE(bool(Pre));
+ ASSERT_TRUE(Pre->Preamble);
EXPECT_EQ(Pre->Preamble->Version, "B");
EXPECT_THAT(includes(Pre->Preamble),
ElementsAre("<B>"));
++CallbackCount;
});
+ S.blockUntilIdle(timeoutSeconds(10));
}
EXPECT_EQ(2, CallbackCount);
}
@@ -853,15 +856,19 @@
TUState(PreambleAction::Idle, ASTAction::RunningAction),
// We build the preamble
TUState(PreambleAction::Building, ASTAction::RunningAction),
- // Preamble worker goes idle
+ // We built the preamble, and issued ast built on ASTWorker
+ // thread. Preambleworker goes idle afterwards.
TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // We start building the ast
+ // Start task for building the ast, as a result of building
+ // preamble, on astworker thread.
+ TUState(PreambleAction::Idle, ASTAction::RunningAction),
+ // AST build starts.
TUState(PreambleAction::Idle, ASTAction::Building),
- // Built finished succesffully
+ // AST built finished successfully
TUState(PreambleAction::Idle, ASTAction::Building),
- // Rnning go to def
+ // Running go to def
TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // both workers go idle
+ // ASTWorker goes idle.
TUState(PreambleAction::Idle, ASTAction::Idle)));
}
Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -286,7 +286,7 @@
FileIndex Index;
bool IndexUpdated = false;
- buildPreamble(FooCpp, *CI, /*OldPreamble=*/nullptr, PI,
+ buildPreamble(FooCpp, *CI, PI,
/*StoreInMemory=*/true,
[&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP,
const CanonicalIncludes &CanonIncludes) {
@@ -424,7 +424,7 @@
}
TEST(FileIndexTest, MergeMainFileSymbols) {
- const char* CommonHeader = "void foo();";
+ const char *CommonHeader = "void foo();";
TestTU Header = TestTU::withCode(CommonHeader);
TestTU Cpp = TestTU::withCode("void foo() {}");
Cpp.Filename = "foo.cpp";
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -6,16 +6,46 @@
//
//===----------------------------------------------------------------------===//
// For each file, managed by TUScheduler, we create a single ASTWorker that
-// manages an AST for that file. All operations that modify or read the AST are
-// run on a separate dedicated thread asynchronously in FIFO order.
+// manages an AST and a preamble for that file. All operations that modify or
+// read the AST are run on a separate dedicated thread asynchronously in FIFO
+// order. There is a second thread for preamble builds, ASTWorker thread issues
+// updates on that preamble thread as preamble becomes stale.
+//
+// As a consequence of that, read requests to the AST always get processed in
+// order and get the exact version of the file when they were issued. Those ASTs
+// are always built with a compatible preamble, built with latest update that
+// modified preamble section of the file.
+// FIXME: Get rid of the synchronization between ASTWorker::update and
+// PreambleThread builds by using patched ASTs instead of compatible preambles
+// for reads. Only keep the blocking behaviour for picky read requests, e.g.
+// Tweaks.
+//
+// Updates are processed in two phases, by issuing a preamble and an ast build.
+// The preamble build gets issued into a different worker and might be
+// overwritten by subsequent updates. An AST will be built for an update if
+// latest built preamble is compatible for it or a new preamble gets built for
+// that update.
+// AST builds are always executed on ASTWorker thread, either immediately
+// following an update request with a compatible preamble, or through a new
+// request, inserted at front of the queue, from PreambleThread after building a
+// new preamble, we'll call the ASTs built in latter case "golden AST"s.
+//
+// Diagnostics and index is updated as a result of building AST for write
+// requests. Progress on those updates are ensured with golden ASTs, as they'll
+// be built even when ASTWorker queue is full. Since ASTs aren't built with
+// incompatible preambles, we can partition diagnostic updates into epochs of
+// preambles. Each epoch starts with a golden AST(unless that update had
+// WantDiagnostics::No), and then moves forward with updates to the AST. Since
+// preambles are always built with newer versions of the file and updates in the
+// ASTWorker are in order, we ensure diagnostics are always built with newer
+// versions of the file.
//
// We start processing each update immediately after we receive it. If two or
// more updates come subsequently without reads in-between, we attempt to drop
// an older one to not waste time building the ASTs we don't need.
//
-// The processing thread of the ASTWorker is also responsible for building the
-// preamble. However, unlike AST, the same preamble can be read concurrently, so
-// we run each of async preamble reads on its own thread.
+// ASTWorker is also responsible for serving preambles. Since preambles can be
+// read concurrently, we run each of async preamble reads on its own thread.
//
// To limit the concurrent load that clangd produces we maintain a semaphore
// that keeps more than a fixed number of threads from running concurrently.
@@ -56,6 +86,7 @@
#include "index/CanonicalIncludes.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
@@ -64,14 +95,19 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Threading.h"
#include <algorithm>
+#include <chrono>
+#include <functional>
#include <memory>
#include <mutex>
#include <queue>
#include <string>
#include <thread>
+#include <type_traits>
+#include <utility>
namespace clang {
namespace clangd {
@@ -190,12 +226,15 @@
ParsingCallbacks &Callbacks;
};
-/// Responsible for building and providing access to the preamble of a TU.
-/// Whenever the thread is idle and the preamble is outdated, it starts to build
-/// a fresh preamble from the latest inputs. If RunSync is true, preambles are
-/// built synchronously in update() instead.
+/// Responsible for building preambles. Whenever the thread is idle and the
+/// preamble is outdated, it starts to build a fresh preamble from the latest
+/// inputs. If RunSync is true, preambles are built synchronously in update()
+/// instead.
class PreambleThread {
public:
+ using PreambleCallback =
+ std::function<void(std::unique_ptr<CompilerInvocation>, ParseInputs,
+ std::shared_ptr<const PreambleData>)>;
PreambleThread(llvm::StringRef FileName, ParsingCallbacks &Callbacks,
bool StorePreambleInMemory, bool RunSync,
SynchronizedTUStatus &Status)
@@ -203,31 +242,25 @@
StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status) {
}
- size_t getUsedBytes() const {
- auto Preamble = latest();
- return Preamble ? Preamble->Preamble.getSize() : 0;
- }
-
/// It isn't guaranteed that each requested version will be built. If there
/// are multiple update requests while building a preamble, only the last one
/// will be built.
- void update(CompilerInvocation *CI, ParseInputs PI) {
- // If compiler invocation was broken, just fail out early.
- if (!CI) {
- // Make sure anyone waiting for the preamble gets notified it could not be
- // built.
- BuiltFirst.notify();
- return;
- }
+ void update(const CompilerInvocation &CI, ParseInputs PI,
+ PreambleCallback PC) {
// Make possibly expensive copy while not holding the lock.
- Request Req = {std::make_unique<CompilerInvocation>(*CI), std::move(PI)};
+ Request Req = {std::make_unique<CompilerInvocation>(CI), std::move(PI),
+ std::move(PC), Context::current().clone()};
if (RunSync) {
build(std::move(Req));
return;
}
{
std::lock_guard<std::mutex> Lock(Mutex);
- assert(!Done && "Build request to PreambleWorker after stop");
+ // ASTWorker might've updates left in it even after PreambleWorker is
+ // stopped. Ignore those as it should be able to serve remaining requests
+ // with stale preambles.
+ if (Done)
+ return;
NextReq = std::move(Req);
}
// Let the worker thread know there's a request, notify_one is safe as there
@@ -235,17 +268,6 @@
ReqCV.notify_all();
}
- /// Blocks until at least a single request has been processed. Note that it
- /// will unblock even after an unsuccessful build.
- void waitForFirst() const { BuiltFirst.wait(); }
-
- /// Returns the latest built preamble, might be null if no preamble has been
- /// built or latest attempt resulted in a failure.
- std::shared_ptr<const PreambleData> latest() const {
- std::lock_guard<std::mutex> Lock(Mutex);
- return LatestBuild;
- }
-
void run() {
dlog("Starting preamble worker for {0}", FileName);
while (true) {
@@ -259,6 +281,7 @@
CurrentReq = std::move(*NextReq);
NextReq.reset();
}
+ WithContext Guard(std::move(CurrentReq->Ctx));
// Build the preamble and let the waiters know about it.
build(std::move(*CurrentReq));
bool IsEmpty = false;
@@ -275,11 +298,8 @@
Status.PreambleActivity = PreambleAction::Idle;
});
}
- BuiltFirst.notify();
ReqCV.notify_all();
}
- // We are no longer going to build any preambles, let the waiters know that.
- BuiltFirst.notify();
dlog("Preamble worker for {0} finished", FileName);
}
@@ -289,6 +309,7 @@
{
std::lock_guard<std::mutex> Lock(Mutex);
Done = true;
+ NextReq.reset();
}
// Let the worker thread know that it should stop.
ReqCV.notify_all();
@@ -305,6 +326,8 @@
struct Request {
std::unique_ptr<CompilerInvocation> CI;
ParseInputs Inputs;
+ PreambleCallback PC;
+ Context Ctx;
};
bool isDone() {
@@ -319,27 +342,31 @@
void build(Request Req) {
assert(Req.CI && "Got preamble request with null compiler invocation");
const ParseInputs &Inputs = Req.Inputs;
- std::shared_ptr<const PreambleData> OldPreamble =
- Inputs.ForceRebuild ? nullptr : latest();
+
+ if (LastInputVersion) {
+ vlog("Rebuilding invalidated preamble for {0} version {1} (previous was "
+ "version {2})",
+ FileName, Inputs.Version, *LastInputVersion);
+ } else {
+ vlog("Building first preamble for {0} version {1}", FileName,
+ Inputs.Version);
+ }
+ LastInputVersion = Inputs.Version;
Status.update([&](TUStatus &Status) {
Status.PreambleActivity = PreambleAction::Building;
});
auto Preamble = clang::clangd::buildPreamble(
- FileName, std::move(*Req.CI), OldPreamble, Inputs, StoreInMemory,
+ FileName, *Req.CI, Inputs, StoreInMemory,
[this, Version(Inputs.Version)](
ASTContext &Ctx, std::shared_ptr<clang::Preprocessor> PP,
const CanonicalIncludes &CanonIncludes) {
Callbacks.onPreambleAST(FileName, Version, Ctx, std::move(PP),
CanonIncludes);
});
- {
- std::lock_guard<std::mutex> Lock(Mutex);
- // LatestBuild might be the last reference to old preamble, do not trigger
- // destructor while holding the lock.
- std::swap(LatestBuild, Preamble);
- }
+ // FIXME: Invalidate NextReq, if this Preamble is compatible with that.
+ Req.PC(std::move(Req.CI), std::move(Req.Inputs), std::move(Preamble));
}
mutable std::mutex Mutex;
@@ -348,14 +375,14 @@
llvm::Optional<Request> CurrentReq; /* GUARDED_BY(Mutex) */
// Signaled whenever a thread populates NextReq or worker thread builds a
// Preamble.
- mutable std::condition_variable ReqCV; /* GUARDED_BY(Mutex) */
- std::shared_ptr<const PreambleData> LatestBuild; /* GUARDED_BY(Mutex) */
+ mutable std::condition_variable ReqCV; /* GUARDED_BY(Mutex) */
- Notification BuiltFirst;
const Path FileName;
ParsingCallbacks &Callbacks;
const bool StoreInMemory;
const bool RunSync;
+ // Used only for logging.
+ llvm::Optional<std::string> LastInputVersion;
SynchronizedTUStatus &Status;
};
@@ -418,6 +445,13 @@
bool isASTCached() const;
private:
+ /// Returns the latest built preamble, might be null if no preamble has been
+ /// built or latest attempt resulted in a failure.
+ std::shared_ptr<const PreambleData> latestPreamble() const {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ return LatestPreamble;
+ }
+
// Must be called exactly once on processing thread. Will return after
// stop() is called on a separate thread and all pending requests are
// processed.
@@ -461,6 +495,9 @@
const GlobalCompilationDatabase &CDB;
/// Callback invoked when preamble or main file AST is built.
ParsingCallbacks &Callbacks;
+ /// Latest build preamble for current TU.
+ std::shared_ptr<const PreambleData> LatestPreamble;
+ Notification BuiltFirstPreamble;
Semaphore &Barrier;
/// Whether the 'onMainAST' callback ran for the current FileInputs.
@@ -602,10 +639,9 @@
else
// FIXME: consider using old command if it's not a fallback one.
Inputs.CompileCommand = CDB.getFallbackCommand(FileName);
- auto PrevInputs = getCurrentFileInputs();
// Will be used to check if we can avoid rebuilding the AST.
bool InputsAreTheSame =
- std::tie(PrevInputs->CompileCommand, PrevInputs->Contents) ==
+ std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
std::tie(Inputs.CompileCommand, Inputs.Contents);
bool RanCallbackForPrevInputs = RanASTCallback;
@@ -623,14 +659,6 @@
std::vector<std::string> CC1Args;
std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
Inputs, CompilerInvocationDiagConsumer, &CC1Args);
- // This is true for now, as we always block until new preamble is build.
- // Once we start to block preambles out-of-order we need to make sure
- // OldPreamble refers to the preamble that was used to build last AST.
- auto OldPreamble = PW.latest();
- PW.update(Invocation.get(), Inputs);
- // FIXME make use of the stale preambles instead.
- PW.blockUntilIdle(Deadline::infinity());
- auto NewPreamble = PW.latest();
// Log cc1 args even (especially!) if creating invocation failed.
if (!CC1Args.empty())
vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
@@ -643,31 +671,167 @@
// Report the diagnostics we collected when parsing the command line.
Callbacks.onFailedAST(FileName, Inputs.Version,
std::move(CompilerInvocationDiags), RunPublish);
+ // Make sure anyone waiting for the preamble gets notified it could not be
+ // built.
+ BuiltFirstPreamble.notify();
return;
}
+ // Task that builds an AST and runs callbacks. Must be run with a preamble
+ // that's compatible with Inputs. Might be run immediately or in future,
+ // after a compatible preamble becomes ready. The built AST will only be
+ // cached if it is compatible with latest inputs of the ASTWorker, which
+ // might not be the case in case of deferred builds.
+ auto ASTTask = [this, WantDiags,
+ CIDiags = std::move(CompilerInvocationDiags),
+ TaskName = std::move(TaskName),
+ RunPublish = std::move(RunPublish)](
+ std::shared_ptr<const PreambleData> Preamble,
+ std::unique_ptr<CompilerInvocation> Invocation,
+ ParseInputs Inputs) mutable {
+ if (Preamble != LatestPreamble) {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ // LatestPreamble might be the last reference to old preamble, do not
+ // trigger destructor while holding the lock.
+ std::swap(LatestPreamble, Preamble);
+ BuiltFirstPreamble.notify();
+ }
+ // We only need to build the AST if diagnostics were requested.
+ if (WantDiags == WantDiagnostics::No)
+ return;
+ {
+ std::lock_guard<std::mutex> Lock(PublishMu);
+ // No need to rebuild the AST if we won't send the diagnostics.
+ if (!CanPublishResults)
+ return;
+ }
+ // Give up our ownership to old preamble before starting expensive AST
+ // build.
+ Preamble.reset();
+ // Used to check whether we can update AST cache and callback status.
+ bool InputsAreTheSame =
+ std::tie(FileInputs->CompileCommand, FileInputs->Contents) ==
+ std::tie(Inputs.CompileCommand, Inputs.Contents);
- bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble);
- // Before doing the expensive AST reparse, we want to release our reference
- // to the old preamble, so it can be freed if there are no other references
- // to it.
- OldPreamble.reset();
- Status.update([&](TUStatus &Status) {
- Status.ASTActivity.K = ASTAction::Building;
- Status.ASTActivity.Name = std::move(TaskName);
- });
- if (!CanReuseAST) {
- IdleASTs.take(this); // Remove the old AST if it's still in cache.
+ Status.update([&](TUStatus &Status) {
+ Status.ASTActivity.K = ASTAction::Building;
+ Status.ASTActivity.Name = std::move(TaskName);
+ });
+ // Get the AST for diagnostics.
+ llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
+ llvm::Optional<DebouncePolicy::clock::duration> RebuildDuration;
+ if (!AST) {
+ auto RebuildStartTime = DebouncePolicy::clock::now();
+ llvm::Optional<ParsedAST> NewAST = buildAST(
+ FileName, std::move(Invocation), CIDiags, Inputs, LatestPreamble);
+ RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
+ // buildAST fails.
+ Status.update([&](TUStatus &Status) {
+ Status.Details.ReuseAST = false;
+ Status.Details.BuildFailed = !NewAST.hasValue();
+ });
+ AST =
+ NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
+ } else {
+ // We are reusing the AST.
+ Status.update([](TUStatus &Status) {
+ Status.Details.ReuseAST = true;
+ Status.Details.BuildFailed = false;
+ });
+ }
+ // We want to report the diagnostics even if this update was cancelled. It
+ // seems more useful than making the clients wait indefinitely if they
+ // spam us with updates. Note *AST can still be null if buildAST fails.
+ if (*AST) {
+ {
+ // Try to record the AST-build time, to inform future update
+ // debouncing. This is best-effort only: if the lock is held, don't
+ // bother.
+ std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
+ if (RebuildDuration && Lock.owns_lock()) {
+ // Do not let RebuildTimes grow beyond its small-size (i.e.
+ // capacity).
+ if (RebuildTimes.size() == RebuildTimes.capacity())
+ RebuildTimes.erase(RebuildTimes.begin());
+ RebuildTimes.push_back(*RebuildDuration);
+ }
+ }
+ trace::Span Span("Running main AST callback");
+ Callbacks.onMainAST(FileName, **AST, RunPublish);
+ // RanASTCallback might be referring to a different FileInputs in case
+ // of deferred builds, only update if that's not the case.
+ if (InputsAreTheSame)
+ RanASTCallback = true;
+ } else {
+ // Failed to build the AST, at least report diagnostics from the
+ // command line if there were any.
+ // FIXME: we might have got more errors while trying to build the
+ // AST, surface them too.
+ Callbacks.onFailedAST(FileName, Inputs.Version, CIDiags, RunPublish);
+ }
+ // Stash the AST in the cache for further use iff it is compatible with
+ // current ASTWorker inputs.
+ if (InputsAreTheSame)
+ IdleASTs.put(this, std::move(*AST));
+ };
+
+ bool PreambleFresh =
+ !Inputs.ForceRebuild && LatestPreamble &&
+ isPreambleUsable(*LatestPreamble, Inputs, FileName, *Invocation);
+ // Do not build AST with stale preambles, as diagnostics might contain false
+ // positives and symbol information might be misleading. Instead defer the
+ // build until a compatible preamble is ready. It might be overwritten by a
+ // subsequent update.
+ if (!PreambleFresh) {
+ // Preamble is not compatible with current inputs.
+ IdleASTs.take(this);
+ PW.update(
+ *Invocation, Inputs,
+ [this, ASTTask = std::move(ASTTask)](
+ std::unique_ptr<CompilerInvocation> CI, ParseInputs PI,
+ std::shared_ptr<const PreambleData> Preamble) {
+ std::string TaskName = llvm::formatv("Build AST ({0})", PI.Version);
+ // Build golden AST with corresponding inputs.
+ auto DeferredTask = [Preamble = std::move(Preamble),
+ CI = std::move(CI), PI = std::move(PI),
+ ASTTask = std::move(ASTTask)]() mutable {
+ ASTTask(std::move(Preamble), std::move(CI), std::move(PI));
+ };
+ if (RunSync) {
+ DeferredTask();
+ return;
+ }
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ // We issue the update with a WantDiagnostics::Yes to make sure it
+ // doesn't get debounced and put it in front, as this update was
+ // issued before any requests in the queue.
+ Requests.push_front(
+ {std::move(DeferredTask), std::move(TaskName),
+ steady_clock::now(), Context::current().clone(),
+ WantDiagnostics::Yes, TUScheduler::NoInvalidation, nullptr});
+ }
+ RequestsCV.notify_all();
+ });
+ // FIXME: We should build patched-up ASTs in case of stale preambles
+ // instead of blocking.
+ PW.blockUntilIdle(Deadline::infinity());
+ return;
+ }
+ vlog("Reusing preamble version {0} for version {1} of {2}",
+ LatestPreamble->Version, Inputs.Version, FileName);
+ if (!InputsAreTheSame) {
+ // Cannot use the cached AST if inputs have changed.
+ IdleASTs.take(this);
} else {
- // We don't need to rebuild the AST, check if we need to run the callback.
+ // 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 handle empty result from buildAST.
+ // FIXME: the AST could actually change if non-preamble includes changed,
+ // but we choose to ignore it.
+ // FIXME: should we refresh the cache in IdleASTs for the current file at
+ // this point?
if (RanCallbackForPrevInputs) {
RanASTCallback = true;
- // Take a shortcut and don't report the diagnostics, since they should
- // not changed. All the clients should handle the lack of OnUpdated()
- // call anyway to handle empty result from buildAST.
- // FIXME(ibiryukov): the AST could actually change if non-preamble
- // includes changed, but we choose to ignore it.
- // FIXME(ibiryukov): should we refresh the cache in IdleASTs for the
- // current file at this point?
log("Skipping rebuild of the AST for {0}, inputs are the same.",
FileName);
@@ -678,71 +842,7 @@
return;
}
}
-
- // We only need to build the AST if diagnostics were requested.
- if (WantDiags == WantDiagnostics::No)
- return;
-
- {
- std::lock_guard<std::mutex> Lock(PublishMu);
- // No need to rebuild the AST if we won't send the diagnostics. However,
- // note that we don't prevent preamble rebuilds.
- if (!CanPublishResults)
- return;
- }
-
- // Get the AST for diagnostics.
- llvm::Optional<std::unique_ptr<ParsedAST>> AST = IdleASTs.take(this);
- auto RebuildStartTime = DebouncePolicy::clock::now();
- if (!AST) {
- llvm::Optional<ParsedAST> NewAST =
- buildAST(FileName, std::move(Invocation), CompilerInvocationDiags,
- Inputs, NewPreamble);
- // buildAST fails.
- Status.update([&](TUStatus &Status) {
- Status.Details.ReuseAST = false;
- Status.Details.BuildFailed = !NewAST.hasValue();
- });
- AST = NewAST ? std::make_unique<ParsedAST>(std::move(*NewAST)) : nullptr;
- } else {
- // We are reusing the AST.
- Status.update([](TUStatus &Status) {
- Status.Details.ReuseAST = true;
- Status.Details.BuildFailed = false;
- });
- }
-
- // We want to report the diagnostics even if this update was cancelled.
- // It seems more useful than making the clients wait indefinitely if they
- // spam us with updates.
- // Note *AST can still be null if buildAST fails.
- if (*AST) {
- {
- // Try to record the AST-build time, to inform future update debouncing.
- // This is best-effort only: if the lock is held, don't bother.
- auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime;
- std::unique_lock<std::mutex> Lock(Mutex, std::try_to_lock);
- if (Lock.owns_lock()) {
- // Do not let RebuildTimes grow beyond its small-size (i.e. capacity).
- if (RebuildTimes.size() == RebuildTimes.capacity())
- RebuildTimes.erase(RebuildTimes.begin());
- RebuildTimes.push_back(RebuildDuration);
- }
- }
- trace::Span Span("Running main AST callback");
-
- Callbacks.onMainAST(FileName, **AST, RunPublish);
- RanASTCallback = true;
- } else {
- // Failed to build the AST, at least report diagnostics from the command
- // line if there were any.
- // FIXME: we might have got more errors while trying to build the AST,
- // surface them too.
- Callbacks.onFailedAST(FileName, Inputs.Version, CompilerInvocationDiags,
- RunPublish);
- }
- // Stash the AST in the cache for further use.
- IdleASTs.put(this, std::move(*AST));
+ ASTTask(LatestPreamble, std::move(Invocation), std::move(Inputs));
};
startTask(TaskName, std::move(Task), WantDiags, TUScheduler::NoInvalidation);
}
@@ -788,7 +888,8 @@
std::shared_ptr<const PreambleData>
ASTWorker::getPossiblyStalePreamble() const {
- return PW.latest();
+ std::lock_guard<std::mutex> Lock(Mutex);
+ return LatestPreamble;
}
void ASTWorker::getCurrentPreamble(
@@ -821,7 +922,7 @@
RequestsCV.notify_all();
}
-void ASTWorker::waitForFirstPreamble() const { PW.waitForFirst(); }
+void ASTWorker::waitForFirstPreamble() const { BuiltFirstPreamble.wait(); }
std::shared_ptr<const ParseInputs> ASTWorker::getCurrentFileInputs() const {
std::unique_lock<std::mutex> Lock(Mutex);
@@ -855,6 +956,9 @@
assert(!Done && "stop() called twice");
Done = true;
}
+ // We are no longer going to build any preambles, let the waiters know that.
+ BuiltFirstPreamble.notify();
+ PW.stop();
Status.stop();
RequestsCV.notify_all();
}
@@ -898,7 +1002,6 @@
}
void ASTWorker::run() {
- auto _ = llvm::make_scope_exit([this] { PW.stop(); });
while (true) {
{
std::unique_lock<std::mutex> Lock(Mutex);
Index: clang-tools-extra/clangd/Preamble.h
===================================================================
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -27,7 +27,9 @@
#include "Diagnostics.h"
#include "FS.h"
#include "Headers.h"
+#include "Path.h"
#include "index/CanonicalIncludes.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -72,17 +74,19 @@
const CanonicalIncludes &)>;
/// Build a preamble for the new inputs unless an old one can be reused.
-/// If \p OldPreamble can be reused, it is returned unchanged.
-/// If \p OldPreamble is null, always builds the preamble.
/// If \p PreambleCallback is set, it will be run on top of the AST while
-/// building the preamble. Note that if the old preamble was reused, no AST is
-/// built and, therefore, the callback will not be executed.
+/// building the preamble.
std::shared_ptr<const PreambleData>
buildPreamble(PathRef FileName, CompilerInvocation CI,
- std::shared_ptr<const PreambleData> OldPreamble,
const ParseInputs &Inputs, bool StoreInMemory,
PreambleParsedCallback PreambleCallback);
+/// Returns true if \p Preamble is reusable for \p Inputs. Note that it will
+/// return true when some missing headers are now available.
+/// FIXME: Should return more information about the delta between \p Preamble
+/// and \p Inputs, e.g. new headers.
+bool isPreambleUsable(const PreambleData &Preamble, const ParseInputs &Inputs,
+ PathRef FileName, const CompilerInvocation &CI);
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -90,7 +90,6 @@
std::shared_ptr<const PreambleData>
buildPreamble(PathRef FileName, CompilerInvocation CI,
- std::shared_ptr<const PreambleData> OldPreamble,
const ParseInputs &Inputs, bool StoreInMemory,
PreambleParsedCallback PreambleCallback) {
// Note that we don't need to copy the input contents, preamble can live
@@ -100,23 +99,6 @@
auto Bounds =
ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0);
- if (OldPreamble &&
- compileCommandsAreEqual(Inputs.CompileCommand,
- OldPreamble->CompileCommand) &&
- OldPreamble->Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
- Inputs.FS.get())) {
- vlog("Reusing preamble version {0} for version {1} of {2}",
- OldPreamble->Version, Inputs.Version, FileName);
- return OldPreamble;
- }
- if (OldPreamble)
- vlog("Rebuilding invalidated preamble for {0} version {1} "
- "(previous was version {2})",
- FileName, Inputs.Version, OldPreamble->Version);
- else
- vlog("Building first preamble for {0} verson {1}", FileName,
- Inputs.Version);
-
trace::Span Tracer("BuildPreamble");
SPAN_ATTACH(Tracer, "File", FileName);
StoreDiags PreambleDiagnostics;
@@ -168,5 +150,16 @@
}
}
+bool isPreambleUsable(const PreambleData &Preamble, const ParseInputs &Inputs,
+ PathRef FileName, const CompilerInvocation &CI) {
+ auto ContentsBuffer =
+ llvm::MemoryBuffer::getMemBuffer(Inputs.Contents, FileName);
+ auto Bounds =
+ ComputePreambleBounds(*CI.getLangOpts(), ContentsBuffer.get(), 0);
+ return compileCommandsAreEqual(Inputs.CompileCommand,
+ Preamble.CompileCommand) &&
+ Preamble.Preamble.CanReuse(CI, ContentsBuffer.get(), Bounds,
+ Inputs.FS.get());
+}
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
