sammccall updated this revision to Diff 135660.
sammccall marked 3 inline comments as done.
sammccall added a comment.
Make Deadline a real type with 0/finite/inf semantics.
Pull out another wait() overload.
Expose debounce delay option in clangdserver.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43648
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CodeComplete.cpp
clangd/DraftStore.cpp
clangd/DraftStore.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/Headers.cpp
clangd/Protocol.cpp
clangd/Protocol.h
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
clangd/Threading.cpp
clangd/Threading.h
unittests/clangd/ClangdTests.cpp
unittests/clangd/TUSchedulerTests.cpp
Index: unittests/clangd/TUSchedulerTests.cpp
===================================================================
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -50,7 +50,7 @@
auto Missing = testPath("missing.cpp");
Files[Missing] = "";
- S.update(Added, getInputs(Added, ""), ignoreUpdate);
+ S.update(Added, getInputs(Added, ""), WantDiagnostics::No, ignoreUpdate);
// Assert each operation for missing file is an error (even if it's available
// in VFS).
@@ -88,6 +88,58 @@
S.remove(Added);
}
+TEST_F(TUSchedulerTests, WantDiagnostics) {
+ std::atomic<int> CallbackCount(0);
+ {
+ // To avoid a racy test, don't allow tasks to actualy run on the worker
+ // thread until we've scheduled them all.
+ Notification Ready;
+ TUScheduler S(getDefaultAsyncThreadsCount(),
+ /*StorePreamblesInMemory=*/true,
+ /*ASTParsedCallback=*/nullptr);
+ auto Path = testPath("foo.cpp");
+ S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
+ [&](std::vector<DiagWithFixIts>) { Ready.wait(); });
+
+ S.update(Path, getInputs(Path, "request diags"), WantDiagnostics::Yes,
+ [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+ S.update(Path, getInputs(Path, "auto (clobbered)"), WantDiagnostics::Auto,
+ [&](std::vector<DiagWithFixIts> Diags) {
+ ADD_FAILURE() << "auto should have been cancelled by auto";
+ });
+ S.update(Path, getInputs(Path, "request no diags"), WantDiagnostics::No,
+ [&](std::vector<DiagWithFixIts> Diags) {
+ ADD_FAILURE() << "no diags should not be called back";
+ });
+ S.update(Path, getInputs(Path, "auto (produces)"), WantDiagnostics::Auto,
+ [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+ Ready.notify();
+ }
+ EXPECT_EQ(2, CallbackCount);
+}
+
+TEST_F(TUSchedulerTests, Debounce) {
+ std::atomic<int> CallbackCount(0);
+ {
+ TUScheduler S(getDefaultAsyncThreadsCount(),
+ /*StorePreamblesInMemory=*/true,
+ /*ASTParsedCallback=*/nullptr,
+ /*UpdateDebounce=*/std::chrono::milliseconds(50));
+ auto Path = testPath("foo.cpp");
+ S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
+ [&](std::vector<DiagWithFixIts> Diags) {
+ ADD_FAILURE() << "auto should have been debounced and canceled";
+ });
+ std::this_thread::sleep_for(std::chrono::milliseconds(10));
+ S.update(Path, getInputs(Path, "auto (timed out)"), WantDiagnostics::Auto,
+ [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+ std::this_thread::sleep_for(std::chrono::milliseconds(60));
+ S.update(Path, getInputs(Path, "auto (shut down)"), WantDiagnostics::Auto,
+ [&](std::vector<DiagWithFixIts> Diags) { ++CallbackCount; });
+ }
+ EXPECT_EQ(2, CallbackCount);
+}
+
TEST_F(TUSchedulerTests, ManyUpdates) {
const int FilesCount = 3;
const int UpdatesPerFile = 10;
@@ -132,7 +184,7 @@
{
WithContextValue WithNonce(NonceKey, ++Nonce);
- S.update(File, Inputs,
+ S.update(File, Inputs, WantDiagnostics::Auto,
[Nonce, &Mut, &TotalUpdates](
llvm::Optional<std::vector<DiagWithFixIts>> Diags) {
EXPECT_THAT(Context::current().get(NonceKey),
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -7,6 +7,7 @@
//
//===----------------------------------------------------------------------===//
+#include "Annotations.h"
#include "ClangdLSPServer.h"
#include "ClangdServer.h"
#include "Matchers.h"
@@ -77,6 +78,43 @@
VFSTag LastVFSTag = VFSTag();
};
+/// For each file, record whether the last published diagnostics contained at
+/// least one error.
+class MultipleErrorCheckingDiagConsumer : public DiagnosticsConsumer {
+public:
+ void
+ onDiagnosticsReady(PathRef File,
+ Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+ bool HadError = diagsContainErrors(Diagnostics.Value);
+
+ std::lock_guard<std::mutex> Lock(Mutex);
+ LastDiagsHadError[File] = HadError;
+ }
+
+ /// Exposes all files consumed by onDiagnosticsReady in an unspecified order.
+ /// For each file, a bool value indicates whether the last diagnostics
+ /// contained an error.
+ std::vector<std::pair<Path, bool>> filesWithDiags() const {
+ std::vector<std::pair<Path, bool>> Result;
+ std::lock_guard<std::mutex> Lock(Mutex);
+
+ for (const auto &it : LastDiagsHadError) {
+ Result.emplace_back(it.first(), it.second);
+ }
+
+ return Result;
+ }
+
+ void clear() {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ LastDiagsHadError.clear();
+ }
+
+private:
+ mutable std::mutex Mutex;
+ llvm::StringMap<bool> LastDiagsHadError;
+};
+
/// Replaces all patterns of the form 0x123abc with spaces
std::string replacePtrsInDump(std::string const &Dump) {
llvm::Regex RE("0x[0-9a-fA-F]+");
@@ -413,6 +451,72 @@
EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags());
}
+// Test ClangdServer.reparseOpenedFiles.
+TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
+ Annotations FooSource(R"cpp(
+#ifdef MACRO
+$one[[static void bob() {}]]
+#else
+$two[[static void bob() {}]]
+#endif
+
+int main () { bo^b (); return 0; }
+)cpp");
+
+ Annotations BarSource(R"cpp(
+#ifdef MACRO
+this is an error
+#endif
+)cpp");
+
+ Annotations BazSource(R"cpp(
+int hello;
+)cpp");
+
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ MultipleErrorCheckingDiagConsumer DiagConsumer;
+ ClangdServer Server(CDB, DiagConsumer, FS,
+ /*AsyncThreadsCount=*/0,
+ /*StorePreamblesInMemory=*/true);
+
+ auto FooCpp = testPath("foo.cpp");
+ auto BarCpp = testPath("bar.cpp");
+ auto BazCpp = testPath("baz.cpp");
+
+ FS.Files[FooCpp] = "";
+ FS.Files[BarCpp] = "";
+ FS.Files[BazCpp] = "";
+
+ CDB.ExtraClangFlags = {"-DMACRO=1"};
+ Server.addDocument(FooCpp, FooSource.code());
+ Server.addDocument(BarCpp, BarSource.code());
+ Server.addDocument(BazCpp, BazSource.code());
+
+ EXPECT_THAT(DiagConsumer.filesWithDiags(),
+ UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, true),
+ Pair(BazCpp, false)));
+
+ auto Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+ EXPECT_TRUE(bool(Locations));
+ EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
+ FooSource.range("one")}));
+
+ // Undefine MACRO, close baz.cpp.
+ CDB.ExtraClangFlags.clear();
+ DiagConsumer.clear();
+ Server.removeDocument(BazCpp);
+ Server.reparseOpenedFiles();
+
+ EXPECT_THAT(DiagConsumer.filesWithDiags(),
+ UnorderedElementsAre(Pair(FooCpp, false), Pair(BarCpp, false)));
+
+ Locations = runFindDefinitions(Server, FooCpp, FooSource.point());
+ EXPECT_TRUE(bool(Locations));
+ EXPECT_THAT(Locations->Value, ElementsAre(Location{URIForFile{FooCpp},
+ FooSource.range("two")}));
+}
+
TEST_F(ClangdVFSTest, MemoryUsage) {
MockFSProvider FS;
ErrorCheckingDiagConsumer DiagConsumer;
Index: clangd/Threading.h
===================================================================
--- clangd/Threading.h
+++ clangd/Threading.h
@@ -13,7 +13,6 @@
#include "Context.h"
#include "Function.h"
#include "llvm/ADT/Twine.h"
-#include <atomic>
#include <cassert>
#include <condition_variable>
#include <memory>
@@ -23,24 +22,18 @@
namespace clang {
namespace clangd {
-/// A shared boolean flag indicating if the computation was cancelled.
-/// Once cancelled, cannot be returned to the previous state.
-class CancellationFlag {
+/// A threadsafe flag that is initially clear.
+class Notification {
public:
- CancellationFlag();
-
- void cancel() {
- assert(WasCancelled && "the object was moved");
- WasCancelled->store(true);
- }
-
- bool isCancelled() const {
- assert(WasCancelled && "the object was moved");
- return WasCancelled->load();
- }
+ // Sets the flag. No-op if already set.
+ void notify();
+ // Blocks until flag is set.
+ void wait() const;
private:
- std::shared_ptr<std::atomic<bool>> WasCancelled;
+ bool Notified = false;
+ mutable std::condition_variable CV;
+ mutable std::mutex Mu;
};
/// Limits the number of threads that can acquire the lock at the same time.
@@ -57,18 +50,48 @@
std::size_t FreeSlots;
};
-/// A point in time we may wait for, or None to wait forever.
+/// A point in time we can wait for.
+/// Can be zero (don't wait) or infinity (wait forever).
/// (Not time_point::max(), because many std::chrono implementations overflow).
-using Deadline = llvm::Optional<std::chrono::steady_clock::time_point>;
-/// Makes a deadline from a timeout in seconds.
+class Deadline {
+public:
+ Deadline(std::chrono::steady_clock::time_point Time)
+ : Type(Finite), Time(Time) {}
+ static Deadline zero() { return Deadline(Zero); }
+ static Deadline infinity() { return Deadline(Infinite); }
+
+ std::chrono::steady_clock::time_point time() const {
+ assert(Type == Finite);
+ return Time;
+ }
+ bool expired() const {
+ return (Type == Zero) ||
+ (Type == Finite && Time < std::chrono::steady_clock::now());
+ }
+ bool operator==(const Deadline &Other) const {
+ return (Type == Other.Type) && (Type != Finite || Time == Other.Time);
+ }
+
+private:
+ enum Type { Zero, Infinite, Finite } Type;
+ Deadline(enum Type Type) : Type(Type) {}
+ std::chrono::steady_clock::time_point Time;
+};
+
+/// Makes a deadline from a timeout in seconds. None means wait forever.
Deadline timeoutSeconds(llvm::Optional<double> Seconds);
+/// Wait once for on CV for the specified duration.
+void wait(std::unique_lock<std::mutex> &Lock, std::condition_variable &CV,
+ Deadline D);
/// Waits on a condition variable until F() is true or D expires.
template <typename Func>
LLVM_NODISCARD bool wait(std::unique_lock<std::mutex> &Lock,
std::condition_variable &CV, Deadline D, Func F) {
- if (D)
- return CV.wait_until(Lock, *D, F);
- CV.wait(Lock, F);
+ while (!F()) {
+ if (D.expired())
+ return false;
+ wait(Lock, CV, D);
+ }
return true;
}
@@ -80,7 +103,7 @@
/// Destructor waits for all pending tasks to finish.
~AsyncTaskRunner();
- void wait() const { (void) wait(llvm::None); }
+ void wait() const { (void)wait(Deadline::infinity()); }
LLVM_NODISCARD bool wait(Deadline D) const;
// The name is used for tracing and debugging (e.g. to name a spawned thread).
void runAsync(llvm::Twine Name, UniqueFunction<void()> Action);
Index: clangd/Threading.cpp
===================================================================
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -7,8 +7,18 @@
namespace clang {
namespace clangd {
-CancellationFlag::CancellationFlag()
- : WasCancelled(std::make_shared<std::atomic<bool>>(false)) {}
+void Notification::notify() {
+ {
+ std::lock_guard<std::mutex> Lock(Mu);
+ Notified = true;
+ }
+ CV.notify_all();
+}
+
+void Notification::wait() const {
+ std::unique_lock<std::mutex> Lock(Mu);
+ CV.wait(Lock, [this] { return Notified; });
+}
Semaphore::Semaphore(std::size_t MaxLocks) : FreeSlots(MaxLocks) {}
@@ -66,10 +76,19 @@
Deadline timeoutSeconds(llvm::Optional<double> Seconds) {
using namespace std::chrono;
if (!Seconds)
- return llvm::None;
+ return Deadline::infinity();
return steady_clock::now() +
duration_cast<steady_clock::duration>(duration<double>(*Seconds));
}
+void wait(std::unique_lock<std::mutex> &Lock, std::condition_variable &CV,
+ Deadline D) {
+ if (D == Deadline::zero())
+ return;
+ if (D == Deadline::infinity())
+ return CV.wait(Lock);
+ CV.wait_until(Lock, D.time());
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -17,6 +17,7 @@
namespace clang {
namespace clangd {
+
/// Returns a number of a default async threads to use for TUScheduler.
/// Returned value is always >= 1 (i.e. will not cause requests to be processed
/// synchronously).
@@ -32,16 +33,27 @@
const PreambleData *Preamble;
};
+/// Determines whether diagnostics should be generated for a file snapshot.
+enum class WantDiagnostics {
+ Yes, /// Diagnostics must be generated for this snapshot.
+ No, /// Diagnostics must not be generated for this snapshot.
+ Auto, /// Diagnostics must be generated for this snapshot or a subsequent one,
+ /// within a bounded amount of time.
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
/// and scheduling tasks.
/// Callbacks are run on a threadpool and it's appropriate to do slow work in
/// them. Each task has a name, used for tracing (should be UpperCamelCase).
+/// FIXME(sammccall): pull out a scheduler options struct.
class TUScheduler {
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
- ASTParsedCallback ASTCallback);
+ ASTParsedCallback ASTCallback,
+ std::chrono::steady_clock::duration UpdateDebounce =
+ std::chrono::milliseconds(500));
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -51,9 +63,8 @@
/// Schedule an update for \p File. Adds \p File to a list of tracked files if
/// \p File was not part of it before.
/// FIXME(ibiryukov): remove the callback from this function.
- void update(PathRef File, ParseInputs Inputs,
- UniqueFunction<void(llvm::Optional<std::vector<DiagWithFixIts>>)>
- OnUpdated);
+ void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD,
+ UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated);
/// Remove \p File from the list of tracked files and schedule removal of its
/// resources.
@@ -94,6 +105,7 @@
// asynchronously.
llvm::Optional<AsyncTaskRunner> PreambleTasks;
llvm::Optional<AsyncTaskRunner> WorkerThreads;
+ std::chrono::steady_clock::duration UpdateDebounce;
};
} // namespace clangd
} // namespace clang
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -54,6 +54,7 @@
namespace clang {
namespace clangd {
+using std::chrono::steady_clock;
namespace {
class ASTWorkerHandle;
@@ -69,22 +70,22 @@
/// worker.
class ASTWorker {
friend class ASTWorkerHandle;
- ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST,
- bool RunSync);
+ ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST, bool RunSync,
+ steady_clock::duration UpdateDebounce);
public:
/// Create a new ASTWorker and return a handle to it.
/// The processing thread is spawned using \p Tasks. However, when \p Tasks
/// is null, all requests will be processed on the calling thread
/// synchronously instead. \p Barrier is acquired when processing each
/// request, it is be used to limit the number of actively running threads.
static ASTWorkerHandle Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
- Semaphore &Barrier, CppFile AST);
+ Semaphore &Barrier, CppFile AST,
+ steady_clock::duration UpdateDebounce);
~ASTWorker();
- void update(ParseInputs Inputs,
- UniqueFunction<void(llvm::Optional<std::vector<DiagWithFixIts>>)>
- OnUpdated);
+ void update(ParseInputs Inputs, WantDiagnostics,
+ UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated);
void runWithAST(llvm::StringRef Name,
UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action);
bool blockUntilIdle(Deadline Timeout) const;
@@ -101,16 +102,28 @@
void stop();
/// Adds a new task to the end of the request queue.
void startTask(llvm::StringRef Name, UniqueFunction<void()> Task,
- bool isUpdate, llvm::Optional<CancellationFlag> CF);
+ llvm::Optional<WantDiagnostics> UpdateType);
+ /// Determines the next action to perform.
+ /// All actions that should never run are disarded.
+ /// Returns a deadline for the next action. If it's expired, run now.
+ /// scheduleLocked() is called again at the deadline, or if requests arrive.
+ Deadline scheduleLocked();
+ /// Should the first task in the queue be skipped instead of run?
+ bool shouldSkipHeadLocked() const;
struct Request {
UniqueFunction<void()> Action;
std::string Name;
+ steady_clock::time_point AddTime;
Context Ctx;
+ llvm::Optional<WantDiagnostics> UpdateType;
};
- std::string File;
+ const std::string File;
const bool RunSync;
+ // Time to wait after an update to see whether another update obsoletes it.
+ const steady_clock::duration UpdateDebounce;
+
Semaphore &Barrier;
// AST and FileInputs are only accessed on the processing thread from run().
CppFile AST;
@@ -123,10 +136,7 @@
std::size_t LastASTSize; /* GUARDED_BY(Mutex) */
// Set to true to signal run() to finish processing.
bool Done; /* GUARDED_BY(Mutex) */
- std::queue<Request> Requests; /* GUARDED_BY(Mutex) */
- // Only set when last request is an update. This allows us to cancel an update
- // that was never read, if a subsequent update comes in.
- llvm::Optional<CancellationFlag> LastUpdateCF; /* GUARDED_BY(Mutex) */
+ std::deque<Request> Requests; /* GUARDED_BY(Mutex) */
mutable std::condition_variable RequestsCV;
};
@@ -173,20 +183,21 @@
};
ASTWorkerHandle ASTWorker::Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
- Semaphore &Barrier, CppFile AST) {
- std::shared_ptr<ASTWorker> Worker(
- new ASTWorker(File, Barrier, std::move(AST), /*RunSync=*/!Tasks));
+ Semaphore &Barrier, CppFile AST,
+ steady_clock::duration UpdateDebounce) {
+ std::shared_ptr<ASTWorker> Worker(new ASTWorker(
+ File, Barrier, std::move(AST), /*RunSync=*/!Tasks, UpdateDebounce));
if (Tasks)
Tasks->runAsync("worker:" + llvm::sys::path::filename(File),
[Worker]() { Worker->run(); });
return ASTWorkerHandle(std::move(Worker));
}
ASTWorker::ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST,
- bool RunSync)
- : File(File), RunSync(RunSync), Barrier(Barrier), AST(std::move(AST)),
- Done(false) {
+ bool RunSync, steady_clock::duration UpdateDebounce)
+ : File(File), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
+ Barrier(Barrier), AST(std::move(AST)), Done(false) {
if (RunSync)
return;
}
@@ -200,14 +211,9 @@
}
void ASTWorker::update(
- ParseInputs Inputs,
- UniqueFunction<void(llvm::Optional<std::vector<DiagWithFixIts>>)>
- OnUpdated) {
- auto Task = [=](CancellationFlag CF, decltype(OnUpdated) OnUpdated) mutable {
- if (CF.isCancelled()) {
- OnUpdated(llvm::None);
- return;
- }
+ ParseInputs Inputs, WantDiagnostics WantDiags,
+ UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated) {
+ auto Task = [=](decltype(OnUpdated) OnUpdated) mutable {
FileInputs = Inputs;
auto Diags = AST.rebuild(std::move(Inputs));
@@ -220,12 +226,11 @@
// 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.
- OnUpdated(std::move(Diags));
+ if (Diags && WantDiags != WantDiagnostics::No)
+ OnUpdated(std::move(*Diags));
};
- CancellationFlag UpdateCF;
- startTask("Update", BindWithForward(Task, UpdateCF, std::move(OnUpdated)),
- /*isUpdate=*/true, UpdateCF);
+ startTask("Update", BindWithForward(Task, std::move(OnUpdated)), WantDiags);
}
void ASTWorker::runWithAST(
@@ -247,7 +252,7 @@
};
startTask(Name, BindWithForward(Task, std::move(Action)),
- /*isUpdate=*/false, llvm::None);
+ /*UpdateType=*/llvm::None);
}
std::shared_ptr<const PreambleData>
@@ -271,10 +276,7 @@
}
void ASTWorker::startTask(llvm::StringRef Name, UniqueFunction<void()> Task,
- bool isUpdate, llvm::Optional<CancellationFlag> CF) {
- assert(isUpdate == CF.hasValue() &&
- "Only updates are expected to pass CancellationFlag");
-
+ llvm::Optional<WantDiagnostics> UpdateType) {
if (RunSync) {
assert(!Done && "running a task after stop()");
trace::Span Tracer(Name + ":" + llvm::sys::path::filename(File));
@@ -285,34 +287,42 @@
{
std::lock_guard<std::mutex> Lock(Mutex);
assert(!Done && "running a task after stop()");
- if (isUpdate) {
- if (!Requests.empty() && LastUpdateCF) {
- // There were no reads for the last unprocessed update, let's cancel it
- // to not waste time on it.
- LastUpdateCF->cancel();
- }
- LastUpdateCF = std::move(*CF);
- } else {
- LastUpdateCF = llvm::None;
- }
- Requests.push({std::move(Task), Name, Context::current().clone()});
- } // unlock Mutex.
+ Requests.push_back({std::move(Task), Name, steady_clock::now(),
+ Context::current().clone(), UpdateType});
+ }
RequestsCV.notify_all();
}
void ASTWorker::run() {
while (true) {
Request Req;
{
std::unique_lock<std::mutex> Lock(Mutex);
- RequestsCV.wait(Lock, [&]() { return Done || !Requests.empty(); });
- if (Requests.empty()) {
- assert(Done);
- return;
+ for (auto Wait = scheduleLocked(); !Wait.expired();
+ Wait = scheduleLocked()) {
+ if (Done) {
+ if (Requests.empty())
+ return;
+ else // Even though Done is set, finish pending requests.
+ break; // However, skip delays to shutdown fast.
+ }
+
+ // Tracing: we have a next request, attribute this sleep to it.
+ Optional<WithContext> Ctx;
+ Optional<trace::Span> Tracer;
+ if (!Requests.empty()) {
+ Ctx.emplace(Requests.front().Ctx.clone());
+ Tracer.emplace("Debounce");
+ SPAN_ATTACH(*Tracer, "next_request", Requests.front().Name);
+ if (!(Wait == Deadline::infinity()))
+ SPAN_ATTACH(*Tracer, "sleep_ms",
+ std::chrono::duration_cast<std::chrono::milliseconds>(
+ Wait.time() - steady_clock::now())
+ .count());
+ }
+
+ wait(Lock, RequestsCV, Wait);
}
- // Even when Done is true, we finish processing all pending requests
- // before exiting the processing loop.
-
Req = std::move(Requests.front());
// Leave it on the queue for now, so waiters don't see an empty queue.
} // unlock Mutex
@@ -326,12 +336,58 @@
{
std::lock_guard<std::mutex> Lock(Mutex);
- Requests.pop();
+ Requests.pop_front();
}
RequestsCV.notify_all();
}
}
+Deadline ASTWorker::scheduleLocked() {
+ if (Requests.empty())
+ return Deadline::infinity(); // Wait for new requests.
+ while (shouldSkipHeadLocked())
+ Requests.pop_front();
+ assert(!Requests.empty() && "skipped the whole queue");
+ // Some updates aren't dead yet, but never end up being used.
+ // e.g. the first keystroke is live until obsoleted by the second.
+ // We debounce "maybe-unused" writes, sleeping 500ms in case they become dead.
+ // But don't delay reads (including updates where diagnostics are needed).
+ for (const auto &R : Requests)
+ if (R.UpdateType == None || R.UpdateType == WantDiagnostics::Yes)
+ return Deadline::zero();
+ // Front request needs to be debounced, so determine when we're ready.
+ Deadline D(Requests.front().AddTime + UpdateDebounce);
+ return D;
+}
+
+// Returns true if Requests.front() is a dead update that can be skipped.
+bool ASTWorker::shouldSkipHeadLocked() const {
+ assert(!Requests.empty());
+ auto Next = Requests.begin();
+ auto UpdateType = Next->UpdateType;
+ if (!UpdateType) // Only skip updates.
+ return false;
+ ++Next;
+ // An update is live if its AST might still be read.
+ // That is, if it's not immediately followed by another update.
+ if (Next == Requests.end() || !Next->UpdateType)
+ return false;
+ // The other way an update can be live is if its diagnostics might be used.
+ switch (*UpdateType) {
+ case WantDiagnostics::Yes:
+ return false; // Always used.
+ case WantDiagnostics::No:
+ return true; // Always dead.
+ case WantDiagnostics::Auto:
+ // Used unless followed by an update that generates diagnostics.
+ for (; Next != Requests.end(); ++Next)
+ if (Next->UpdateType == WantDiagnostics::Yes ||
+ Next->UpdateType == WantDiagnostics::Auto)
+ return true; // Prefer later diagnostics.
+ return false;
+ }
+}
+
bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
std::unique_lock<std::mutex> Lock(Mutex);
return wait(Lock, RequestsCV, Timeout, [&] { return Requests.empty(); });
@@ -357,10 +413,12 @@
TUScheduler::TUScheduler(unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
- ASTParsedCallback ASTCallback)
+ ASTParsedCallback ASTCallback,
+ steady_clock::duration UpdateDebounce)
: StorePreamblesInMemory(StorePreamblesInMemory),
PCHOps(std::make_shared<PCHContainerOperations>()),
- ASTCallback(std::move(ASTCallback)), Barrier(AsyncThreadsCount) {
+ ASTCallback(std::move(ASTCallback)), Barrier(AsyncThreadsCount),
+ UpdateDebounce(UpdateDebounce) {
if (0 < AsyncThreadsCount) {
PreambleTasks.emplace();
WorkerThreads.emplace();
@@ -389,20 +447,20 @@
}
void TUScheduler::update(
- PathRef File, ParseInputs Inputs,
- UniqueFunction<void(llvm::Optional<std::vector<DiagWithFixIts>>)>
- OnUpdated) {
+ PathRef File, ParseInputs Inputs, WantDiagnostics WantDiags,
+ UniqueFunction<void(std::vector<DiagWithFixIts>)> OnUpdated) {
std::unique_ptr<FileData> &FD = Files[File];
if (!FD) {
// Create a new worker to process the AST-related tasks.
ASTWorkerHandle Worker = ASTWorker::Create(
File, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
- CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback));
+ CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback),
+ UpdateDebounce);
FD = std::unique_ptr<FileData>(new FileData{Inputs, std::move(Worker)});
} else {
FD->Inputs = Inputs;
}
- FD->Worker->update(std::move(Inputs), std::move(OnUpdated));
+ FD->Worker->update(std::move(Inputs), WantDiags, std::move(OnUpdated));
}
void TUScheduler::remove(PathRef File) {
Index: clangd/ProtocolHandlers.h
===================================================================
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -52,6 +52,7 @@
virtual void onRename(RenameParams &Parames) = 0;
virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
virtual void onHover(TextDocumentPositionParams &Params) = 0;
+ virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
};
void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===================================================================
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -72,4 +72,6 @@
Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
Register("textDocument/documentHighlight",
&ProtocolCallbacks::onDocumentHighlight);
+ Register("workspace/didChangeConfiguration",
+ &ProtocolCallbacks::onChangeConfiguration);
}
Index: clangd/Protocol.h
===================================================================
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -324,6 +324,20 @@
};
bool fromJSON(const json::Expr &, DidChangeWatchedFilesParams &);
+/// Clangd extension to manage a workspace/didChangeConfiguration notification
+/// since the data received is described as 'any' type in LSP.
+struct ClangdConfigurationParamsChange {
+ llvm::Optional<std::string> compilationDatabasePath;
+};
+bool fromJSON(const json::Expr &, ClangdConfigurationParamsChange &);
+
+struct DidChangeConfigurationParams {
+ // We use this predefined struct because it is easier to use
+ // than the protocol specified type of 'any'.
+ ClangdConfigurationParamsChange settings;
+};
+bool fromJSON(const json::Expr &, DidChangeConfigurationParams &);
+
struct FormattingOptions {
/// Size of a tab in spaces.
int tabSize = 0;
Index: clangd/Protocol.cpp
===================================================================
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -497,5 +497,15 @@
};
}
+bool fromJSON(const json::Expr &Params, DidChangeConfigurationParams &CCP) {
+ json::ObjectMapper O(Params);
+ return O && O.map("settings", CCP.settings);
+}
+
+bool fromJSON(const json::Expr &Params, ClangdConfigurationParamsChange &CCPC) {
+ json::ObjectMapper O(Params);
+ return O && O.map("compilationDatabasePath", CCPC.compilationDatabasePath);
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/Headers.cpp
===================================================================
--- clangd/Headers.cpp
+++ clangd/Headers.cpp
@@ -79,12 +79,12 @@
// added more than once.
CI->getPreprocessorOpts().SingleFileParseMode = true;
+ // The diagnostic options must be set before creating a CompilerInstance.
+ CI->getDiagnosticOpts().IgnoreWarnings = true;
auto Clang = prepareCompilerInstance(
std::move(CI), /*Preamble=*/nullptr,
llvm::MemoryBuffer::getMemBuffer(Code, File),
std::make_shared<PCHContainerOperations>(), FS, IgnoreDiags);
- auto &DiagOpts = Clang->getDiagnosticOpts();
- DiagOpts.IgnoreWarnings = true;
if (Clang->getFrontendOpts().Inputs.empty())
return llvm::make_error<llvm::StringError>(
Index: clangd/GlobalCompilationDatabase.h
===================================================================
--- clangd/GlobalCompilationDatabase.h
+++ clangd/GlobalCompilationDatabase.h
@@ -61,6 +61,9 @@
/// Uses the default fallback command, adding any extra flags.
tooling::CompileCommand getFallbackCommand(PathRef File) const override;
+ /// Set the compile commands directory to \p P.
+ void setCompileCommandsDir(Path P);
+
/// Sets the extra flags that should be added to a file.
void setExtraFlagsForFile(PathRef File, std::vector<std::string> ExtraFlags);
Index: clangd/GlobalCompilationDatabase.cpp
===================================================================
--- clangd/GlobalCompilationDatabase.cpp
+++ clangd/GlobalCompilationDatabase.cpp
@@ -51,6 +51,12 @@
return C;
}
+void DirectoryBasedGlobalCompilationDatabase::setCompileCommandsDir(Path P) {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ CompileCommandsDir = P;
+ CompilationDatabases.clear();
+}
+
void DirectoryBasedGlobalCompilationDatabase::setExtraFlagsForFile(
PathRef File, std::vector<std::string> ExtraFlags) {
std::lock_guard<std::mutex> Lock(Mutex);
Index: clangd/DraftStore.h
===================================================================
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -40,6 +40,12 @@
/// \return version and contents of the stored document.
/// For untracked files, a (0, None) pair is returned.
VersionedDraft getDraft(PathRef File) const;
+
+ /// \return List of names of active drafts in this store. Drafts that were
+ /// removed (which still have an entry in the Drafts map) are not returned by
+ /// this function.
+ std::vector<Path> getActiveFiles() const;
+
/// \return version of the tracked document.
/// For untracked files, 0 is returned.
DocVersion getVersion(PathRef File) const;
Index: clangd/DraftStore.cpp
===================================================================
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -21,6 +21,17 @@
return It->second;
}
+std::vector<Path> DraftStore::getActiveFiles() const {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ std::vector<Path> ResultVector;
+
+ for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
+ if (DraftIt->second.Draft)
+ ResultVector.push_back(DraftIt->getKey());
+
+ return ResultVector;
+}
+
DocVersion DraftStore::getVersion(PathRef File) const {
std::lock_guard<std::mutex> Lock(Mutex);
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -696,11 +696,11 @@
Input.Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds,
Input.VFS.get());
}
+ // The diagnostic options must be set before creating a CompilerInstance.
+ CI->getDiagnosticOpts().IgnoreWarnings = true;
auto Clang = prepareCompilerInstance(
std::move(CI), Input.Preamble, std::move(ContentsBuffer),
std::move(Input.PCHs), std::move(Input.VFS), DummyDiagsConsumer);
- auto &DiagOpts = Clang->getDiagnosticOpts();
- DiagOpts.IgnoreWarnings = true;
// Disable typo correction in Sema.
Clang->getLangOpts().SpellChecking = false;
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -125,6 +125,8 @@
/// \p DiagConsumer. Note that a callback to \p DiagConsumer happens on a
/// worker thread. Therefore, instances of \p DiagConsumer must properly
/// synchronize access to shared state.
+ /// UpdateDebounce determines how long to wait after a new version of the file
+ /// before starting to compute diagnostics.
///
/// \p StorePreamblesInMemory defines whether the Preambles generated by
/// clangd are stored in-memory or on disk.
@@ -135,22 +137,26 @@
///
/// If \p StaticIdx is set, ClangdServer uses the index for global code
/// completion.
+ /// FIXME(sammccall): pull out an options struct.
ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
bool BuildDynamicSymbolIndex = false,
SymbolIndex *StaticIdx = nullptr,
- llvm::Optional<StringRef> ResourceDir = llvm::None);
+ llvm::Optional<StringRef> ResourceDir = llvm::None,
+ std::chrono::steady_clock::duration UpdateDebounce =
+ std::chrono::milliseconds(500));
/// Set the root path of the workspace.
void setRootPath(PathRef RootPath);
/// Add a \p File to the list of tracked C++ files or update the contents if
/// \p File is already tracked. Also schedules parsing of the AST for it on a
/// separate thread. When the parsing is complete, DiagConsumer passed in
/// constructor will receive onDiagnosticsReady callback.
- void addDocument(PathRef File, StringRef Contents);
+ void addDocument(PathRef File, StringRef Contents,
+ WantDiagnostics WD = WantDiagnostics::Auto);
/// Remove \p File from list of tracked files, schedule a request to free
/// resources associated with it.
@@ -162,6 +168,11 @@
/// and AST and rebuild them from scratch.
void forceReparse(PathRef File);
+ /// Calls forceReparse() on all currently opened files.
+ /// As a result, this method may be very expensive.
+ /// This method is normally called when the compilation database is changed.
+ void reparseOpenedFiles();
+
/// Run code completion for \p File at \p Pos.
/// Request is processed asynchronously.
///
@@ -278,6 +289,7 @@
void
scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
+ WantDiagnostics WD,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
CompileArgsCache CompileArgs;
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -76,7 +76,8 @@
unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
bool BuildDynamicSymbolIndex, SymbolIndex *StaticIdx,
- llvm::Optional<StringRef> ResourceDir)
+ llvm::Optional<StringRef> ResourceDir,
+ std::chrono::steady_clock::duration UpdateDebounce)
: CompileArgs(CDB,
ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
DiagConsumer(DiagConsumer), FSProvider(FSProvider),
@@ -91,7 +92,8 @@
FileIdx
? [this](PathRef Path,
ParsedAST *AST) { FileIdx->update(Path, AST); }
- : ASTParsedCallback()) {
+ : ASTParsedCallback(),
+ UpdateDebounce) {
if (FileIdx && StaticIdx) {
MergedIndex = mergeIndex(FileIdx.get(), StaticIdx);
Index = MergedIndex.get();
@@ -110,11 +112,12 @@
this->RootPath = NewRootPath;
}
-void ClangdServer::addDocument(PathRef File, StringRef Contents) {
+void ClangdServer::addDocument(PathRef File, StringRef Contents,
+ WantDiagnostics WantDiags) {
DocVersion Version = DraftMgr.updateDraft(File, Contents);
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
- std::move(TaggedFS));
+ WantDiags, std::move(TaggedFS));
}
void ClangdServer::removeDocument(PathRef File) {
@@ -133,7 +136,8 @@
CompileArgs.invalidate(File);
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
- scheduleReparseAndDiags(File, std::move(FileContents), std::move(TaggedFS));
+ scheduleReparseAndDiags(File, std::move(FileContents), WantDiagnostics::Yes,
+ std::move(TaggedFS));
}
void ClangdServer::codeComplete(
@@ -519,20 +523,16 @@
}
void ClangdServer::scheduleReparseAndDiags(
- PathRef File, VersionedDraft Contents,
+ PathRef File, VersionedDraft Contents, WantDiagnostics WantDiags,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
- using OptDiags = llvm::Optional<std::vector<DiagWithFixIts>>;
-
DocVersion Version = Contents.Version;
Path FileStr = File.str();
VFSTag Tag = std::move(TaggedFS.Tag);
- auto Callback = [this, Version, FileStr, Tag](OptDiags Diags) {
- if (!Diags)
- return; // A new reparse was requested before this one completed.
-
+ auto Callback = [this, Version, FileStr,
+ Tag](std::vector<DiagWithFixIts> Diags) {
// We need to serialize access to resulting diagnostics to avoid calling
// `onDiagnosticsReady` in the wrong order.
std::lock_guard<std::mutex> DiagsLock(DiagnosticsMutex);
@@ -546,14 +546,19 @@
LastReportedDiagsVersion = Version;
DiagConsumer.onDiagnosticsReady(
- FileStr, make_tagged(std::move(*Diags), std::move(Tag)));
+ FileStr, make_tagged(std::move(Diags), std::move(Tag)));
};
WorkScheduler.update(File,
ParseInputs{std::move(Command),
std::move(TaggedFS.Value),
std::move(*Contents.Draft)},
- std::move(Callback));
+ WantDiags, std::move(Callback));
+}
+
+void ClangdServer::reparseOpenedFiles() {
+ for (const Path &FilePath : DraftMgr.getActiveFiles())
+ forceReparse(FilePath);
}
void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
Index: clangd/ClangdLSPServer.h
===================================================================
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -76,6 +76,7 @@
void onCommand(ExecuteCommandParams &Params) override;
void onRename(RenameParams &Parames) override;
void onHover(TextDocumentPositionParams &Params) override;
+ void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
std::vector<TextEdit> getFixIts(StringRef File, const clangd::Diagnostic &D);
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -141,16 +141,17 @@
if (Params.metadata && !Params.metadata->extraFlags.empty())
CDB.setExtraFlagsForFile(Params.textDocument.uri.file(),
std::move(Params.metadata->extraFlags));
- Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text);
+ Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text,
+ WantDiagnostics::Yes);
}
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
if (Params.contentChanges.size() != 1)
return replyError(ErrorCode::InvalidParams,
"can only apply one change at a time");
// We only support full syncing right now.
Server.addDocument(Params.textDocument.uri.file(),
- Params.contentChanges[0].text);
+ Params.contentChanges[0].text, WantDiagnostics::Auto);
}
void ClangdLSPServer::onFileEvent(DidChangeWatchedFilesParams &Params) {
@@ -369,6 +370,18 @@
});
}
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(
+ DidChangeConfigurationParams &Params) {
+ ClangdConfigurationParamsChange &Settings = Params.settings;
+
+ // Compilation database change.
+ if (Settings.compilationDatabasePath.hasValue()) {
+ CDB.setCompileCommandsDir(Settings.compilationDatabasePath.getValue());
+ Server.reparseOpenedFiles();
+ }
+}
+
ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
const clangd::CodeCompleteOptions &CCOpts,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
