ilya-biryukov updated this revision to Diff 109534.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.
- Fixed more typos/inconsistences in comments, pointed out by @krasimir.
https://reviews.llvm.org/D36261
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/tool/ClangdMain.cpp
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===================================================================
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -272,17 +272,17 @@
MockFSProvider FS;
ErrorCheckingDiagConsumer DiagConsumer;
MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
- ClangdServer Server(CDB, DiagConsumer, FS,
- /*RunSynchronously=*/false);
+ ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount());
for (const auto &FileWithContents : ExtraFiles)
FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
FileWithContents.second;
auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath);
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);
@@ -334,8 +334,7 @@
MockFSProvider FS;
ErrorCheckingDiagConsumer DiagConsumer;
MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
- ClangdServer Server(CDB, DiagConsumer, FS,
- /*RunSynchronously=*/false);
+ ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount());
const auto SourceContents = R"cpp(
#include "foo.h"
@@ -379,8 +378,7 @@
ErrorCheckingDiagConsumer DiagConsumer;
MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
- ClangdServer Server(CDB, DiagConsumer, FS,
- /*RunSynchronously=*/false);
+ ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount());
const auto SourceContents = R"cpp(
#include "foo.h"
@@ -425,16 +423,17 @@
MockFSProvider FS;
ErrorCheckingDiagConsumer DiagConsumer;
MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
+ // Run ClangdServer synchronously.
ClangdServer Server(CDB, DiagConsumer, FS,
- /*RunSynchronously=*/true);
+ /*AsyncThreadsCount=*/0);
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);
@@ -456,8 +455,9 @@
CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(),
{"-xc++", "-target", "x86_64-linux-unknown",
"-m64", "--gcc-toolchain=/randomusr"});
+ // Run ClangdServer synchronously.
ClangdServer Server(CDB, DiagConsumer, FS,
- /*RunSynchronously=*/true);
+ /*AsyncThreadsCount=*/0);
// Just a random gcc version string
SmallString<8> Version("4.9.3");
@@ -486,8 +486,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());
@@ -516,8 +516,7 @@
ErrorCheckingDiagConsumer DiagConsumer;
MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
- ClangdServer Server(CDB, DiagConsumer, FS,
- /*RunSynchronously=*/false);
+ ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount());
auto FooCpp = getVirtualTestFilePath("foo.cpp");
const auto SourceContents = R"cpp(
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -16,14 +16,20 @@
#include <iostream>
#include <memory>
#include <string>
+#include <thread>
using namespace clang;
using namespace clang::clangd;
-static llvm::cl::opt<bool>
- RunSynchronously("run-synchronously",
- llvm::cl::desc("Parse on main thread"),
- llvm::cl::init(false), llvm::cl::Hidden);
+static llvm::cl::opt<unsigned>
+ WorkerThreadsCount("j",
+ llvm::cl::desc("Number of async workers used by clangd"),
+ llvm::cl::init(getDefaultAsyncThreadsCount()));
+
+static llvm::cl::opt<bool> 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<std::string>
ResourceDir("resource-dir",
@@ -33,6 +39,17 @@
int main(int argc, char *argv[]) {
llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
+ if (!RunSynchronously && WorkerThreadsCount == 0) {
+ llvm::errs() << "A number of worker threads cannot be 0. Did you mean to "
+ "specify -run-synchronously?";
+ return 1;
+ }
+
+ // Ignore -j option if -run-synchonously is used.
+ // FIXME: a warning should be shown here.
+ if (RunSynchronously)
+ WorkerThreadsCount = 0;
+
llvm::raw_ostream &Outs = llvm::outs();
llvm::raw_ostream &Logs = llvm::errs();
JSONOutput Out(Outs, Logs);
@@ -43,6 +60,7 @@
llvm::Optional<StringRef> ResourceDirRef = None;
if (!ResourceDir.empty())
ResourceDirRef = ResourceDir;
- ClangdLSPServer LSPServer(Out, RunSynchronously, ResourceDirRef);
+
+ ClangdLSPServer LSPServer(Out, WorkerThreadsCount, ResourceDirRef);
LSPServer.run(std::cin);
}
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -101,11 +101,20 @@
class ClangdServer;
-/// Handles running WorkerRequests of ClangdServer on a separate threads.
-/// Currently runs only one worker thread.
+/// Returns a number of a default async threads to use for ClangdScheduler.
+/// Returned value is always >= 1 (i.e. will not cause requests to be processed
+/// synchronously).
+unsigned getDefaultAsyncThreadsCount();
+
+/// Handles running WorkerRequests of ClangdServer on a number of worker
+/// threads.
class ClangdScheduler {
public:
- ClangdScheduler(bool RunSynchronously);
+ /// If \p AsyncThreadsCount is 0, requests added using addToFront and addToEnd
+ /// will be processed synchronously on the calling thread.
+ // Otherwise, \p AsyncThreadsCount threads will be created to schedule the
+ // requests.
+ ClangdScheduler(unsigned AsyncThreadsCount);
~ClangdScheduler();
/// Add a new request to run function \p F with args \p As to the start of the
@@ -146,41 +155,37 @@
private:
bool RunSynchronously;
std::mutex Mutex;
- /// 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;
- /// Setting Done to true will make the worker thread terminate.
+ /// We run some tasks on separate threads(parsing, CppFile cleanup).
+ /// These threads looks into RequestQueue to find requests to handle and
+ /// terminate when Done is set to true.
+ std::vector<std::thread> Workers;
+ /// Setting Done to true will make the worker threads 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<std::future<void>> RequestQueue;
- /// Condition variable to wake up the worker thread.
+ /// Condition variable to wake up worker threads.
std::condition_variable RequestCV;
};
/// Provides API to manage ASTs for a collection of C++ files and request
/// various language features(currently, only codeCompletion and async
/// 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,
+ /// that spawns \p AsyncThreadsCount worker threads will be created (when \p
+ /// AsyncThreadsCount is 0, requests will be processed on the calling thread).
+ /// 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, unsigned AsyncThreadsCount,
llvm::Optional<StringRef> 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,52 @@
return make_tagged(vfs::getRealFileSystem(), VFSTag());
}
-ClangdScheduler::ClangdScheduler(bool RunSynchronously)
- : RunSynchronously(RunSynchronously) {
+unsigned clangd::getDefaultAsyncThreadsCount() {
+ 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;
+}
+
+ClangdScheduler::ClangdScheduler(unsigned AsyncThreadsCount)
+ : RunSynchronously(AsyncThreadsCount == 0) {
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<void> Request;
-
- // Pick request from the queue
- {
- std::unique_lock<std::mutex> 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();
- }
- });
+ Workers.reserve(AsyncThreadsCount);
+ for (unsigned I = 0; I < AsyncThreadsCount; ++I) {
+ Workers.push_back(std::thread([this]() {
+ while (true) {
+ std::future<void> Request;
+
+ // Pick request from the queue
+ {
+ std::unique_lock<std::mutex> 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 +135,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,
+ unsigned AsyncThreadsCount,
llvm::Optional<StringRef> ResourceDir)
: CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
PCHs(std::make_shared<PCHContainerOperations>()),
- WorkScheduler(RunSynchronously) {}
+ WorkScheduler(AsyncThreadsCount) {}
std::future<void> ClangdServer::addDocument(PathRef File, StringRef Contents) {
DocVersion Version = DraftMgr.updateDraft(File, Contents);
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, unsigned AsyncThreadsCount,
llvm::Optional<StringRef> 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, unsigned AsyncThreadsCount,
llvm::Optional<StringRef> ResourceDir)
: Out(Out), DiagConsumer(*this),
- Server(CDB, DiagConsumer, FSProvider, RunSynchronously, ResourceDir) {}
+ Server(CDB, DiagConsumer, FSProvider, AsyncThreadsCount, ResourceDir) {}
void ClangdLSPServer::run(std::istream &In) {
assert(!IsDone && "Run was called before");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
