Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -24,6 +24,7 @@ #include "Protocol.h" #include +#include #include #include #include @@ -49,30 +50,25 @@ std::vector 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 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 Request); private: bool RunSynchronously; @@ -83,11 +79,10 @@ 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 RequestQueue; + std::deque> RequestQueue; /// Condition variable to wake up the worker thread. std::condition_variable RequestCV; }; @@ -126,12 +121,12 @@ /// 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 CDB; std::unique_ptr DiagConsumer; DraftStore DraftMgr; Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -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 using namespace clang; using namespace clang::clangd; @@ -56,11 +58,7 @@ 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 @@ // 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 Request; // Pick request from the queue { @@ -83,19 +81,15 @@ 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 @@ std::lock_guard 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 Request) { if (RunSynchronously) { - Server.handleRequest(Request); + Request(); return; } - std::lock_guard Lock(Mutex); - RequestQueue.push_back(Request); + { + std::lock_guard Lock(Mutex); + RequestQueue.push_front(Request); + } + RequestCV.notify_one(); +} + +void ClangdScheduler::addToEnd(std::function Request) { + if (RunSynchronously) { + Request(); + return; + } + + { + std::lock_guard Lock(Mutex); + RequestQueue.push_back(Request); + } RequestCV.notify_one(); } @@ -129,19 +138,34 @@ bool RunSynchronously) : CDB(std::move(CDB)), DiagConsumer(std::move(DiagConsumer)), PCHs(std::make_shared()), - 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 ClangdServer::codeComplete(PathRef File, @@ -156,6 +180,7 @@ }); return Result; } + std::vector ClangdServer::formatRange(PathRef File, Range Rng) { std::string Code = getDocument(File); @@ -191,27 +216,23 @@ 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 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(); } Index: clang-tools-extra/trunk/clangd/ClangdUnit.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h +++ clang-tools-extra/trunk/clangd/ClangdUnit.h @@ -16,6 +16,10 @@ #include "clang/Tooling/Core/Replacement.h" #include +namespace llvm { +class raw_ostream; +} + namespace clang { class ASTUnit; class PCHContainerOperations; @@ -52,6 +56,10 @@ /// located in the current file. std::vector 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 Unit; Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -222,3 +222,7 @@ } return Result; } + +void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const { + Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true); +} Index: clang-tools-extra/trunk/clangd/ClangdUnitStore.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnitStore.h +++ clang-tools-extra/trunk/clangd/ClangdUnitStore.h @@ -50,6 +50,18 @@ std::forward(Action)); } + /// Run the specified \p Action on the ClangdUnit for \p File. + /// Unit for \p File should exist in the store. + template + void runOnExistingUnit(PathRef File, Func Action) { + std::lock_guard 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);