[clang-tools-extra] r303634 - [clangd] Replaced WorkerRequest with std::function...
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Tue May 23 06:42:59 PDT 2017
Author: ibiryukov
Date: Tue May 23 08:42:59 2017
New Revision: 303634
URL: http://llvm.org/viewvc/llvm-project?rev=303634&view=rev
Log:
[clangd] Replaced WorkerRequest with std::function...
Summary:
And implemented a helper function to dump an AST of a file for
testing/debugging purposes.
Reviewers: bkramer, krasimir
Reviewed By: krasimir
Subscribers: klimek, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D33415
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/ClangdUnitStore.h
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=303634&r1=303633&r2=303634&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May 23 08:42:59 2017
@@ -15,6 +15,8 @@
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include <future>
using namespace clang;
using namespace clang::clangd;
@@ -56,11 +58,7 @@ Position clangd::offsetToPosition(String
return {Lines, Cols};
}
-WorkerRequest::WorkerRequest(WorkerRequestKind Kind, Path File,
- DocVersion Version)
- : Kind(Kind), File(File), Version(Version) {}
-
-ClangdScheduler::ClangdScheduler(ClangdServer &Server, bool RunSynchronously)
+ClangdScheduler::ClangdScheduler(bool RunSynchronously)
: RunSynchronously(RunSynchronously) {
if (RunSynchronously) {
// Don't start the worker thread if we're running synchronously
@@ -69,9 +67,9 @@ ClangdScheduler::ClangdScheduler(ClangdS
// Initialize Worker in ctor body, rather than init list to avoid potentially
// using not-yet-initialized members
- Worker = std::thread([&Server, this]() {
+ Worker = std::thread([this]() {
while (true) {
- WorkerRequest Request;
+ std::function<void()> Request;
// Pick request from the queue
{
@@ -83,19 +81,15 @@ ClangdScheduler::ClangdScheduler(ClangdS
assert(!RequestQueue.empty() && "RequestQueue was empty");
- Request = std::move(RequestQueue.back());
- RequestQueue.pop_back();
-
- // Skip outdated requests
- if (Request.Version != Server.DraftMgr.getVersion(Request.File)) {
- // FIXME(ibiryukov): Logging
- // Output.log("Version for " + Twine(Request.File) +
- // " in request is outdated, skipping request\n");
- continue;
- }
+ // We process requests starting from the front of the queue. Users of
+ // ClangdScheduler have a way to prioritise their requests by putting
+ // them to the either side of the queue (using either addToEnd or
+ // addToFront).
+ Request = std::move(RequestQueue.front());
+ RequestQueue.pop_front();
} // unlock Mutex
- Server.handleRequest(std::move(Request));
+ Request();
}
});
}
@@ -108,19 +102,34 @@ ClangdScheduler::~ClangdScheduler() {
std::lock_guard<std::mutex> Lock(Mutex);
// Wake up the worker thread
Done = true;
- RequestCV.notify_one();
} // unlock Mutex
+ RequestCV.notify_one();
Worker.join();
}
-void ClangdScheduler::enqueue(ClangdServer &Server, WorkerRequest Request) {
+void ClangdScheduler::addToFront(std::function<void()> Request) {
if (RunSynchronously) {
- Server.handleRequest(Request);
+ Request();
return;
}
- std::lock_guard<std::mutex> Lock(Mutex);
- RequestQueue.push_back(Request);
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ RequestQueue.push_front(Request);
+ }
+ RequestCV.notify_one();
+}
+
+void ClangdScheduler::addToEnd(std::function<void()> Request) {
+ if (RunSynchronously) {
+ Request();
+ return;
+ }
+
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ RequestQueue.push_back(Request);
+ }
RequestCV.notify_one();
}
@@ -129,19 +138,34 @@ ClangdServer::ClangdServer(std::unique_p
bool RunSynchronously)
: CDB(std::move(CDB)), DiagConsumer(std::move(DiagConsumer)),
PCHs(std::make_shared<PCHContainerOperations>()),
- WorkScheduler(*this, RunSynchronously) {}
+ WorkScheduler(RunSynchronously) {}
void ClangdServer::addDocument(PathRef File, StringRef Contents) {
- DocVersion NewVersion = DraftMgr.updateDraft(File, Contents);
- WorkScheduler.enqueue(
- *this, WorkerRequest(WorkerRequestKind::ParseAndPublishDiagnostics, File,
- NewVersion));
+ DocVersion Version = DraftMgr.updateDraft(File, Contents);
+ Path FileStr = File;
+ WorkScheduler.addToFront([this, FileStr, Version]() {
+ auto FileContents = DraftMgr.getDraft(FileStr);
+ if (FileContents.Version != Version)
+ return; // This request is outdated, do nothing
+
+ assert(FileContents.Draft &&
+ "No contents inside a file that was scheduled for reparse");
+ Units.runOnUnit(
+ FileStr, *FileContents.Draft, *CDB, PCHs, [&](ClangdUnit const &Unit) {
+ DiagConsumer->onDiagnosticsReady(FileStr, Unit.getLocalDiagnostics());
+ });
+ });
}
void ClangdServer::removeDocument(PathRef File) {
- auto NewVersion = DraftMgr.removeDraft(File);
- WorkScheduler.enqueue(
- *this, WorkerRequest(WorkerRequestKind::RemoveDocData, File, NewVersion));
+ auto Version = DraftMgr.removeDraft(File);
+ Path FileStr = File;
+ WorkScheduler.addToFront([this, FileStr, Version]() {
+ if (Version != DraftMgr.getVersion(FileStr))
+ return; // This request is outdated, do nothing
+
+ Units.removeUnitIfPresent(FileStr);
+ });
}
std::vector<CompletionItem> ClangdServer::codeComplete(PathRef File,
@@ -156,6 +180,7 @@ std::vector<CompletionItem> ClangdServer
});
return Result;
}
+
std::vector<tooling::Replacement> ClangdServer::formatRange(PathRef File,
Range Rng) {
std::string Code = getDocument(File);
@@ -191,27 +216,23 @@ std::string ClangdServer::getDocument(Pa
return *draft.Draft;
}
-void ClangdServer::handleRequest(WorkerRequest Request) {
- switch (Request.Kind) {
- case WorkerRequestKind::ParseAndPublishDiagnostics: {
- auto FileContents = DraftMgr.getDraft(Request.File);
- if (FileContents.Version != Request.Version)
- return; // This request is outdated, do nothing
-
- assert(FileContents.Draft &&
- "No contents inside a file that was scheduled for reparse");
- Units.runOnUnit(Request.File, *FileContents.Draft, *CDB, PCHs,
- [&](ClangdUnit const &Unit) {
- DiagConsumer->onDiagnosticsReady(
- Request.File, Unit.getLocalDiagnostics());
- });
- break;
- }
- case WorkerRequestKind::RemoveDocData:
- if (Request.Version != DraftMgr.getVersion(Request.File))
- return; // This request is outdated, do nothing
+std::string ClangdServer::dumpAST(PathRef File) {
+ std::promise<std::string> DumpPromise;
+ auto DumpFuture = DumpPromise.get_future();
+ auto Version = DraftMgr.getVersion(File);
+
+ WorkScheduler.addToEnd([this, &DumpPromise, File, Version]() {
+ assert(DraftMgr.getVersion(File) == Version && "Version has changed");
+
+ Units.runOnExistingUnit(File, [&DumpPromise](ClangdUnit &Unit) {
+ std::string Result;
+
+ llvm::raw_string_ostream ResultOS(Result);
+ Unit.dumpAST(ResultOS);
+ ResultOS.flush();
- Units.removeUnitIfPresent(Request.File);
- break;
- }
+ DumpPromise.set_value(std::move(Result));
+ });
+ });
+ return DumpFuture.get();
}
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=303634&r1=303633&r2=303634&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue May 23 08:42:59 2017
@@ -24,6 +24,7 @@
#include "Protocol.h"
#include <condition_variable>
+#include <functional>
#include <mutex>
#include <string>
#include <thread>
@@ -49,30 +50,25 @@ public:
std::vector<DiagWithFixIts> Diagnostics) = 0;
};
-enum class WorkerRequestKind { ParseAndPublishDiagnostics, RemoveDocData };
-
-/// A request to the worker thread
-class WorkerRequest {
-public:
- WorkerRequest() = default;
- WorkerRequest(WorkerRequestKind Kind, Path File, DocVersion Version);
-
- WorkerRequestKind Kind;
- Path File;
- DocVersion Version;
-};
-
class ClangdServer;
/// Handles running WorkerRequests of ClangdServer on a separate threads.
/// Currently runs only one worker thread.
class ClangdScheduler {
public:
- ClangdScheduler(ClangdServer &Server, bool RunSynchronously);
+ ClangdScheduler(bool RunSynchronously);
~ClangdScheduler();
- /// Enqueue WorkerRequest to be run on a worker thread
- void enqueue(ClangdServer &Server, WorkerRequest Request);
+ /// Add \p Request to the start of the queue. \p Request will be run on a
+ /// separate worker thread.
+ /// \p Request is scheduled to be executed before all currently added
+ /// requests.
+ void addToFront(std::function<void()> Request);
+ /// Add \p Request to the end of the queue. \p Request will be run on a
+ /// separate worker thread.
+ /// \p Request is scheduled to be executed after all currently added
+ /// requests.
+ void addToEnd(std::function<void()> Request);
private:
bool RunSynchronously;
@@ -83,11 +79,10 @@ private:
std::thread Worker;
/// Setting Done to true will make the worker thread terminate.
bool Done = false;
- /// A LIFO queue of requests. Note that requests are discarded if the
- /// `version` field is not equal to the one stored inside DraftStore.
+ /// A queue of requests.
/// FIXME(krasimir): code completion should always have priority over parsing
/// for diagnostics.
- std::deque<WorkerRequest> RequestQueue;
+ std::deque<std::function<void()>> RequestQueue;
/// Condition variable to wake up the worker thread.
std::condition_variable RequestCV;
};
@@ -126,12 +121,12 @@ public:
/// conversions in outside code, maybe there's a way to get rid of it.
std::string getDocument(PathRef File);
-private:
- friend class ClangdScheduler;
-
- /// This function is called on a worker thread.
- void handleRequest(WorkerRequest Request);
+ /// Only for testing purposes.
+ /// Waits until all requests to worker thread are finished and dumps AST for
+ /// \p File. \p File must be in the list of added documents.
+ std::string dumpAST(PathRef File);
+private:
std::unique_ptr<GlobalCompilationDatabase> CDB;
std::unique_ptr<DiagnosticsConsumer> DiagConsumer;
DraftStore DraftMgr;
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=303634&r1=303633&r2=303634&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue May 23 08:42:59 2017
@@ -222,3 +222,7 @@ std::vector<DiagWithFixIts> ClangdUnit::
}
return Result;
}
+
+void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const {
+ Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true);
+}
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=303634&r1=303633&r2=303634&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Tue May 23 08:42:59 2017
@@ -16,6 +16,10 @@
#include "clang/Tooling/Core/Replacement.h"
#include <memory>
+namespace llvm {
+class raw_ostream;
+}
+
namespace clang {
class ASTUnit;
class PCHContainerOperations;
@@ -52,6 +56,10 @@ public:
/// located in the current file.
std::vector<DiagWithFixIts> getLocalDiagnostics() const;
+ /// For testing/debugging purposes. Note that this method deserializes all
+ /// unserialized Decls, so use with care.
+ void dumpAST(llvm::raw_ostream &OS) const;
+
private:
Path FileName;
std::unique_ptr<ASTUnit> Unit;
Modified: clang-tools-extra/trunk/clangd/ClangdUnitStore.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnitStore.h?rev=303634&r1=303633&r2=303634&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnitStore.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnitStore.h Tue May 23 08:42:59 2017
@@ -50,6 +50,18 @@ public:
std::forward<Func>(Action));
}
+ /// Run the specified \p Action on the ClangdUnit for \p File.
+ /// Unit for \p File should exist in the store.
+ template <class Func>
+ void runOnExistingUnit(PathRef File, Func Action) {
+ std::lock_guard<std::mutex> Lock(Mutex);
+
+ auto It = OpenedFiles.find(File);
+ assert(It != OpenedFiles.end() && "File is not in OpenedFiles");
+
+ Action(It->second);
+ }
+
/// Remove ClangdUnit for \p File, if any
void removeUnitIfPresent(PathRef File);
More information about the cfe-commits
mailing list