kadircet updated this revision to Diff 254442.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77063/new/
https://reviews.llvm.org/D77063
Files:
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -968,6 +968,7 @@
// Only preamble is built, and no AST is built in this request.
Server.addDocument(FooCpp, FooWithoutHeader.code(), "null",
WantDiagnostics::No);
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
// We build AST here, and it should use the latest preamble rather than the
// stale one.
EXPECT_THAT(
@@ -979,6 +980,7 @@
// Both preamble and AST are built in this request.
Server.addDocument(FooCpp, FooWithoutHeader.code(), "null",
WantDiagnostics::Yes);
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
// Use the AST being built in above request.
EXPECT_THAT(
cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())),
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -323,9 +323,11 @@
// Helper to schedule a named update and return a function to cancel it.
auto Update = [&](std::string ID) -> Canceler {
auto T = cancelableTask();
+ auto Inp = getInputs(Path, "//" + ID);
+ Inp.Version = ID;
WithContext C(std::move(T.first));
updateWithDiags(
- S, Path, "//" + ID, WantDiagnostics::Yes,
+ S, Path, std::move(Inp), WantDiagnostics::Yes,
[&, ID](std::vector<Diag> Diags) { DiagsSeen.push_back(ID); });
return std::move(T.second);
};
@@ -498,7 +500,7 @@
WithContextValue WithNonce(NonceKey, ++Nonce);
Inputs.Version = std::to_string(Nonce);
updateWithDiags(
- S, File, Inputs, WantDiagnostics::Auto,
+ S, File, Inputs, WantDiagnostics::Yes,
[File, Nonce, &Mut, &TotalUpdates](std::vector<Diag>) {
EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
@@ -735,7 +737,8 @@
)cpp";
ParseInputs Inputs = getInputs(Source, SourceContents);
- std::atomic<size_t> DiagCount(0);
+ Inputs.Version = "MissingHeader";
+ std::atomic<int> DiagCount(0);
// Update the source contents, which should trigger an initial build with
// the header file missing.
@@ -754,6 +757,7 @@
Files[Header] = "int a;";
Timestamps[Header] = time_t(1);
Inputs = getInputs(Source, SourceContents);
+ Inputs.Version = "WithHeader";
// The addition of the missing header file shouldn't trigger a rebuild since
// we don't track missing files.
@@ -767,6 +771,7 @@
// Forcing the reload should should cause a rebuild which no longer has any
// errors.
Inputs.ForceRebuild = true;
+ Inputs.Version = "ForceRebuild";
updateWithDiags(S, Source, Inputs, WantDiagnostics::Yes,
[&DiagCount](std::vector<Diag> Diags) {
++DiagCount;
@@ -858,27 +863,20 @@
ASSERT_TRUE(Server.blockUntilIdleForTest());
- EXPECT_THAT(CaptureTUStatus.allStatus(),
- ElementsAre(
- // Everything starts with ASTWorker starting to execute an
- // update
- TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // We build the preamble
- TUState(PreambleAction::Building, ASTAction::RunningAction),
- // We built the preamble, and issued ast built on ASTWorker
- // thread. Preambleworker goes idle afterwards.
- TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // 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),
- // AST built finished successfully
- TUState(PreambleAction::Idle, ASTAction::Building),
- // Running go to def
- TUState(PreambleAction::Idle, ASTAction::RunningAction),
- // ASTWorker goes idle.
- TUState(PreambleAction::Idle, ASTAction::Idle)));
+ auto Statuses = CaptureTUStatus.allStatus();
+ const std::vector<PreambleAction> PreambleStatuses = {
+ PreambleAction::Idle, PreambleAction::Building, PreambleAction::Idle};
+
+ llvm::ArrayRef<PreambleAction> RemainingStatuses =
+ llvm::makeArrayRef(PreambleStatuses);
+ ASSERT_FALSE(Statuses.empty());
+ for (size_t I = 0, E = Statuses.size(); I != E; ++I) {
+ if (RemainingStatuses.front() != Statuses[I].PreambleActivity)
+ RemainingStatuses = RemainingStatuses.drop_front();
+ ASSERT_FALSE(RemainingStatuses.empty());
+ EXPECT_EQ(Statuses[I].PreambleActivity, RemainingStatuses.front());
+ }
+ EXPECT_EQ(RemainingStatuses.size(), 1U);
}
TEST_F(TUSchedulerTests, CommandLineErrors) {
Index: clang-tools-extra/clangd/unittests/ClangdTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ClangdTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdTests.cpp
@@ -210,18 +210,18 @@
FS.Files[FooCpp] = SourceContents;
Server.addDocument(FooCpp, SourceContents);
- auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
Server.addDocument(FooCpp, "");
- auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
Server.addDocument(FooCpp, SourceContents);
- auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
EXPECT_EQ(DumpParse1, DumpParse2);
@@ -246,20 +246,20 @@
FS.Files[FooCpp] = SourceContents;
Server.addDocument(FooCpp, SourceContents);
- auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
FS.Files[FooH] = "";
Server.addDocument(FooCpp, SourceContents);
- auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp);
EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags());
FS.Files[FooH] = "int a;";
Server.addDocument(FooCpp, SourceContents);
- auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+ auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp);
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
EXPECT_EQ(DumpParse1, DumpParse2);
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -221,6 +221,7 @@
/// will be built.
void update(std::unique_ptr<CompilerInvocation> CI, ParseInputs PI,
std::vector<Diag> CIDiags, WantDiagnostics WantDiags) {
+ dlog("Preamble worker queueing {0}", PI.Version);
// Make possibly expensive copy while not holding the lock.
Request Req = {std::move(CI), std::move(PI), std::move(CIDiags), WantDiags,
Context::current().clone()};
@@ -232,10 +233,14 @@
return;
}
{
- std::lock_guard<std::mutex> Lock(Mutex);
+ std::unique_lock<std::mutex> Lock(Mutex);
// If shutdown is issued, don't bother building.
if (Done)
return;
+ ReqCV.wait(Lock, [this] {
+ return !NextReq || NextReq->WantDiags != WantDiagnostics::Yes;
+ });
+ dlog("preamble worker queued {0}", Req.Inputs.Version);
NextReq = std::move(Req);
}
// Let the worker thread know there's a request, notify_one is safe as there
@@ -291,6 +296,7 @@
}
bool blockUntilIdle(Deadline Timeout) const {
+ dlog("blocking for preamble worker");
std::unique_lock<std::mutex> Lock(Mutex);
return wait(Lock, ReqCV, Timeout, [&] { return !NextReq && !CurrentReq; });
}
@@ -874,6 +880,8 @@
void ASTWorker::getCurrentPreamble(
llvm::unique_function<void(std::shared_ptr<const PreambleData>)> Callback) {
+ if (RunSync)
+ return Callback(getPossiblyStalePreamble());
// We could just call startTask() to throw the read on the queue, knowing
// it will run after any updates. But we know this task is cheap, so to
// improve latency we cheat: insert it on the queue after the last update.
@@ -881,23 +889,23 @@
auto LastUpdate =
std::find_if(Requests.rbegin(), Requests.rend(),
[](const Request &R) { return R.UpdateType.hasValue(); });
- // If there were no writes in the queue, and CurrentRequest is not a write,
- // the preamble is ready now.
- if (LastUpdate == Requests.rend() &&
- (!CurrentRequest || CurrentRequest->UpdateType.hasValue())) {
- Lock.unlock();
- return Callback(getPossiblyStalePreamble());
- }
- assert(!RunSync && "Running synchronously, but queue is non-empty!");
- Requests.insert(LastUpdate.base(),
- Request{[Callback = std::move(Callback), this]() mutable {
- Callback(getPossiblyStalePreamble());
- },
- "GetPreamble", steady_clock::now(),
- Context::current().clone(),
- /*UpdateType=*/None,
- /*InvalidationPolicy=*/TUScheduler::NoInvalidation,
- /*Invalidate=*/nullptr});
+ // FIXME: We should only serve stale preambles or build it here instead.
+ Requests.insert(
+ LastUpdate.base(),
+ Request{
+ [Callback = std::move(Callback), this]() mutable {
+ PreamblePeer.blockUntilIdle(Deadline::infinity());
+ PreambleRequests.push(
+ {[Callback = std::move(Callback), this]() mutable {
+ Callback(getPossiblyStalePreamble());
+ },
+ "GetPreamble", steady_clock::now(), Context::current().clone(),
+ None, TUScheduler::NoInvalidation, nullptr});
+ },
+ "WaitForPreamble", steady_clock::now(), Context::current().clone(),
+ /*UpdateType=*/None,
+ /*InvalidationPolicy=*/TUScheduler::NoInvalidation,
+ /*Invalidate=*/nullptr});
Lock.unlock();
RequestsCV.notify_all();
}
@@ -947,6 +955,7 @@
llvm::unique_function<void()> Task,
llvm::Optional<WantDiagnostics> UpdateType,
TUScheduler::ASTActionInvalidation Invalidation) {
+ dlog("ASTWorker queueing {0}", Name);
if (RunSync) {
assert(!Done && "running a task after stop()");
trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName));
@@ -1139,9 +1148,18 @@
bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
std::unique_lock<std::mutex> Lock(Mutex);
- return wait(Lock, RequestsCV, Timeout, [&] {
- return PreambleRequests.empty() && Requests.empty() && !CurrentRequest;
- });
+ // Make sure ASTWorker has processed all requests.
+ if (!wait(Lock, RequestsCV, Timeout,
+ [&] { return Requests.empty() && !CurrentRequest; }))
+ return false;
+ Lock.unlock();
+ if (!PreamblePeer.blockUntilIdle(Timeout))
+ return false;
+ Lock.lock();
+ assert(Requests.empty() && "Received new requests during blockUntilIdle");
+ // Make sure ASTWorker has processed all preambles.
+ return wait(Lock, RequestsCV, Timeout,
+ [&] { return PreambleRequests.empty() && !CurrentRequest; });
}
// Render a TUAction to a user-facing string representation.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
