Index: clangd/ClangdLSPServer.h =================================================================== --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -26,7 +26,7 @@ /// dispatch and ClangdServer together. class ClangdLSPServer { public: - ClangdLSPServer(JSONOutput &Out, bool RunSynchronously, + ClangdLSPServer(JSONOutput &Out, SchedulingParams SchedParams, llvm::Optional ResourceDir); /// Run LSP server loop, receiving input for it from \p In. \p In must be Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -220,10 +220,10 @@ R"(,"result":[)" + Locations + R"(]})"); } -ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, bool RunSynchronously, +ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, SchedulingParams SchedParams, llvm::Optional ResourceDir) : Out(Out), DiagConsumer(*this), - Server(CDB, DiagConsumer, FSProvider, RunSynchronously, ResourceDir) {} + Server(CDB, DiagConsumer, FSProvider, SchedParams, ResourceDir) {} void ClangdLSPServer::run(std::istream &In) { assert(!IsDone && "Run was called before"); Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -101,11 +101,38 @@ class ClangdServer; +/// A helper class to pass concurrency parameters to ClangdScheduler. +class SchedulingParams { +public: + /// Indicates that requests must be executed immidieately on the calling + /// thread. + static SchedulingParams RunOnCallingThread(); + /// Indicates then requests should be executed on separate worker threads. In + /// total \p ThreadsCount working threads will be created. + /// Asserts that \p ThreadsCount is > 0. + static SchedulingParams RunOnWorkerThreads(unsigned ThreadsCount); + /// Uses this::thread::hardware_concurrency() for the count of threads. + static SchedulingParams RunOnDefaultNumberOfThreads(); + +private: + SchedulingParams(unsigned ThreadsCount, bool RunSynchronously); + +public: + /// Returns the number of threads to use when shouldRunsynchronously() is + /// false. Must not be called if shouldRunsynchronously() is true. + unsigned getThreadsCount(); + bool shouldRunSynchronously(); + +private: + unsigned ThreadsCount; + bool RunSynchronously; +}; + /// Handles running WorkerRequests of ClangdServer on a separate threads. /// Currently runs only one worker thread. class ClangdScheduler { public: - ClangdScheduler(bool RunSynchronously); + ClangdScheduler(SchedulingParams Params); ~ClangdScheduler(); /// Add a new request to run function \p F with args \p As to the start of the @@ -149,12 +176,11 @@ /// We run some tasks on a separate threads(parsing, CppFile cleanup). /// This thread looks into RequestQueue to find requests to handle and /// terminates when Done is set to true. - std::thread Worker; + std::vector Workers; /// Setting Done to true will make the worker thread terminate. bool Done = false; - /// A queue of requests. - /// FIXME(krasimir): code completion should always have priority over parsing - /// for diagnostics. + /// A queue of requests. Elements of this vector are async computations (i.e. + /// results of calling std::async(std::launch::deferred, ...)). std::deque> RequestQueue; /// Condition variable to wake up the worker thread. std::condition_variable RequestCV; @@ -165,22 +191,18 @@ /// diagnostics for tracked files). class ClangdServer { public: - /// Creates a new ClangdServer. If \p RunSynchronously is false, no worker - /// thread will be created and all requests will be completed synchronously on - /// the calling thread (this is mostly used for tests). If \p RunSynchronously - /// is true, a worker thread will be created to parse files in the background - /// and provide diagnostics results via DiagConsumer.onDiagnosticsReady - /// callback. File accesses for each instance of parsing will be conducted via - /// a vfs::FileSystem provided by \p FSProvider. Results of code - /// completion/diagnostics also include a tag, that \p FSProvider returns - /// along with the vfs::FileSystem. - /// When \p ResourceDir is set, it will be used to search for internal headers + /// Creates a new ClangdServer. To server parsing requests ClangdScheduler, + /// created using \p SchedParams, will be created. File accesses for each + /// instance of parsing will be conducted via a vfs::FileSystem provided by \p + /// FSProvider. Results of code completion/diagnostics also include a tag, + /// that \p FSProvider returns along with the vfs::FileSystem. When \p + /// ResourceDir is set, it will be used to search for internal headers /// (overriding defaults and -resource-dir compiler flag, if set). If \p /// ResourceDir is None, ClangdServer will attempt to set it to a standard /// location, obtained via CompilerInvocation::GetResourcePath. ClangdServer(GlobalCompilationDatabase &CDB, DiagnosticsConsumer &DiagConsumer, - FileSystemProvider &FSProvider, bool RunSynchronously, + FileSystemProvider &FSProvider, SchedulingParams SchedParams, llvm::Optional ResourceDir = llvm::None); /// Add a \p File to the list of tracked C++ files or update the contents if Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -78,40 +78,77 @@ return make_tagged(vfs::getRealFileSystem(), VFSTag()); } -ClangdScheduler::ClangdScheduler(bool RunSynchronously) - : RunSynchronously(RunSynchronously) { +SchedulingParams SchedulingParams::RunOnCallingThread() { + return SchedulingParams(1, true); +} + +SchedulingParams SchedulingParams::RunOnWorkerThreads(unsigned ThreadsCount) { + assert(ThreadsCount > 0); + return SchedulingParams(ThreadsCount, false); +} + +SchedulingParams SchedulingParams::RunOnDefaultNumberOfThreads() { + unsigned Threads = std::thread::hardware_concurrency(); + if (Threads == 0) + Threads = 1; // C++ standard says hardware_concurrency() can return 0 on + // some platforms. + return RunOnWorkerThreads(Threads); +} + +SchedulingParams::SchedulingParams(unsigned ThreadsCount, bool RunSynchronously) + : ThreadsCount(ThreadsCount), RunSynchronously(RunSynchronously) { + if (ThreadsCount == 0) { + assert(false && "ThreadsCount should never be 0 to tolerate misuses of an " + "API with disabled asserts."); + ThreadsCount = 1; + } +} + +unsigned SchedulingParams::getThreadsCount() { + assert(!shouldRunSynchronously()); + return ThreadsCount; +} + +bool SchedulingParams::shouldRunSynchronously() { return RunSynchronously; } + +ClangdScheduler::ClangdScheduler(SchedulingParams Params) + : RunSynchronously(Params.shouldRunSynchronously()) { if (RunSynchronously) { // Don't start the worker thread if we're running synchronously return; } - // Initialize Worker in ctor body, rather than init list to avoid potentially - // using not-yet-initialized members - Worker = std::thread([this]() { - while (true) { - std::future Request; - - // Pick request from the queue - { - std::unique_lock Lock(Mutex); - // Wait for more requests. - RequestCV.wait(Lock, [this] { return !RequestQueue.empty() || Done; }); - if (Done) - return; - - assert(!RequestQueue.empty() && "RequestQueue was empty"); - - // 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 - - Request.get(); - } - }); + unsigned ThreadsCount = Params.getThreadsCount(); + + Workers.reserve(ThreadsCount); + for (unsigned I = 0; I < ThreadsCount; ++I) { + Workers.push_back(std::thread([this]() { + while (true) { + std::future Request; + + // Pick request from the queue + { + std::unique_lock Lock(Mutex); + // Wait for more requests. + RequestCV.wait(Lock, + [this] { return !RequestQueue.empty() || Done; }); + if (Done) + return; + + assert(!RequestQueue.empty() && "RequestQueue was empty"); + + // 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 + + Request.get(); + } + })); + } } ClangdScheduler::~ClangdScheduler() { @@ -123,19 +160,21 @@ // Wake up the worker thread Done = true; } // unlock Mutex - RequestCV.notify_one(); - Worker.join(); + RequestCV.notify_all(); + + for (auto &Worker : Workers) + Worker.join(); } ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB, DiagnosticsConsumer &DiagConsumer, FileSystemProvider &FSProvider, - bool RunSynchronously, + SchedulingParams SchedParams, llvm::Optional ResourceDir) : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()), PCHs(std::make_shared()), - WorkScheduler(RunSynchronously) {} + WorkScheduler(SchedParams) {} std::future ClangdServer::addDocument(PathRef File, StringRef Contents) { DocVersion Version = DraftMgr.updateDraft(File, Contents); Index: clangd/tool/ClangdMain.cpp =================================================================== --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -16,14 +16,30 @@ #include #include #include +#include using namespace clang; using namespace clang::clangd; -static llvm::cl::opt - RunSynchronously("run-synchronously", - llvm::cl::desc("Parse on main thread"), - llvm::cl::init(false), llvm::cl::Hidden); +static unsigned getDefaultThreadsCount() { + unsigned HardwareConcurrency = std::thread::hardware_concurrency(); + // C++ standard says that hardware_concurrency() + // may return 0, fallback to 1 worker thread in + // that case. + if (HardwareConcurrency == 0) + return 1; + return HardwareConcurrency; +} + +static llvm::cl::opt + ThreadsCount("j", + llvm::cl::desc("Number of parallel workers used by clangd."), + llvm::cl::init(getDefaultThreadsCount())); + +static llvm::cl::opt RunSynchronously( + "run-synchronously", + llvm::cl::desc("Parse on main thread. If set, -j is ignored."), + llvm::cl::init(false), llvm::cl::Hidden); static llvm::cl::opt ResourceDir("resource-dir", @@ -33,6 +49,11 @@ int main(int argc, char *argv[]) { llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); + if (!RunSynchronously && ThreadsCount == 0) { + llvm::errs() << "Number of threads must be positive."; + return 1; + } + llvm::raw_ostream &Outs = llvm::outs(); llvm::raw_ostream &Logs = llvm::errs(); JSONOutput Out(Outs, Logs); @@ -43,6 +64,11 @@ llvm::Optional ResourceDirRef = None; if (!ResourceDir.empty()) ResourceDirRef = ResourceDir; - ClangdLSPServer LSPServer(Out, RunSynchronously, ResourceDirRef); + + SchedulingParams SchedParams = + !RunSynchronously ? SchedulingParams::RunOnWorkerThreads(ThreadsCount) + : SchedulingParams::RunOnCallingThread(); + + ClangdLSPServer LSPServer(Out, SchedParams, ResourceDirRef); LSPServer.run(std::cin); } Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -273,7 +273,7 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*RunSynchronously=*/false); + SchedulingParams::RunOnDefaultNumberOfThreads()); for (const auto &FileWithContents : ExtraFiles) FS.Files[getVirtualTestFilePath(FileWithContents.first)] = FileWithContents.second; @@ -282,7 +282,8 @@ FS.ExpectedFile = SourceFilename; - // Have to sync reparses because RunSynchronously is false. + // Have to sync reparses because requests are processed on the calling + // thread. auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents); auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); @@ -335,7 +336,7 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*RunSynchronously=*/false); + SchedulingParams::RunOnDefaultNumberOfThreads()); const auto SourceContents = R"cpp( #include "foo.h" @@ -380,7 +381,7 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*RunSynchronously=*/false); + SchedulingParams::RunOnDefaultNumberOfThreads()); const auto SourceContents = R"cpp( #include "foo.h" @@ -426,15 +427,15 @@ ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*RunSynchronously=*/true); + SchedulingParams::RunOnCallingThread()); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = "int a;"; FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; - // No need to sync reparses, because RunSynchronously is set - // to true. + // No need to sync reparses, because requests are processed on the calling + // thread. FS.Tag = "123"; Server.addDocument(FooCpp, SourceContents); EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); @@ -457,7 +458,7 @@ {"-xc++", "-target", "x86_64-linux-unknown", "-m64", "--gcc-toolchain=/randomusr"}); ClangdServer Server(CDB, DiagConsumer, FS, - /*RunSynchronously=*/true); + SchedulingParams::RunOnCallingThread()); // Just a random gcc version string SmallString<8> Version("4.9.3"); @@ -486,8 +487,8 @@ )cpp"; FS.Files[FooCpp] = SourceContents; - // No need to sync reparses, because RunSynchronously is set - // to true. + // No need to sync reparses, because requests are processed on the calling + // thread. Server.addDocument(FooCpp, SourceContents); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); @@ -517,7 +518,7 @@ MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); ClangdServer Server(CDB, DiagConsumer, FS, - /*RunSynchronously=*/false); + SchedulingParams::RunOnDefaultNumberOfThreads()); auto FooCpp = getVirtualTestFilePath("foo.cpp"); const auto SourceContents = R"cpp(