This revision was automatically updated to reflect the committed changes.
Closed by commit rL310821: [clangd] Use multiple working threads in clangd.
(authored by ibiryukov).
Changed prior to commit:
https://reviews.llvm.org/D36261?vs=110381&id=110922#toc
Repository:
rL LLVM
https://reviews
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D36261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
ilya-biryukov added a comment.
Added a test that exercises some multi-threading behaviour.
It really finds bugs, at least it found a problem that was fixed by
https://reviews.llvm.org/D36397. (Will make sure to submit that other change
before this one).
As discussed with @klimek, will move to
ilya-biryukov updated this revision to Diff 110381.
ilya-biryukov added a comment.
- Added a stress test for multithreading.
https://reviews.llvm.org/D36261
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/tool/ClangdMain.
ilya-biryukov added a comment.
In https://reviews.llvm.org/D36261#834902, @klimek wrote:
> High-level question: Why can't we use llvm::ThreadPool?
It is not an in-place replacement as it does not allow to prioritize new tasks
over old ones (new tasks are usually more important for clangd as th
klimek added a comment.
High-level question: Why can't we use llvm::ThreadPool?
https://reviews.llvm.org/D36261
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.h:121-122
+public:
+ /// Returns the number of threads to use when shouldRunsynchronously() is
+ /// false. Must not be called if shouldRunsynchronously() is true.
+ unsigned getThreadsCount();
--
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/ClangdLSPServe
ilya-biryukov added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:69
+ SchedulingOptions SchedOpts =
+ !RunSynchronously ? SchedulingOptions::RunOnWorkerThreads(ThreadsCount)
+: SchedulingOptions::RunOnCallingThread();
kras
ilya-biryukov updated this revision to Diff 109532.
ilya-biryukov added a comment.
- Removed SchedulingOptions altogether, replaced with AsyncThreadsCount.
https://reviews.llvm.org/D36261
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdSer
krasimir added a comment.
Great!
Comment at: clangd/ClangdServer.h:131
+
/// Handles running WorkerRequests of ClangdServer on a separate threads.
/// Currently runs only one worker thread.
typo: "on separate threads"
Comment at: clangd/Cla
ilya-biryukov updated this revision to Diff 109527.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.
Addressed review comments.
- Fixed typos.
- Renamed SchedulingParams to SchedulingOptions.
https://reviews.llvm.org/D36261
Files:
clangd/ClangdLSPServer.cpp
cl
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.h:121-122
+public:
+ /// Returns the number of threads to use when shouldRunsynchronously() is
+ /// false. Must not be called if shouldRunsynchronously() is true.
+ unsigned getThreadsCount();
--
klimek added inline comments.
Comment at: clangd/ClangdServer.h:105
+/// A helper class to pass concurrency parameters to ClangdScheduler.
+class SchedulingParams {
+public:
I think calling it "Options" is more idiomatic.
Comment at: clangd/Cla
arphaman added inline comments.
Comment at: clangd/ClangdServer.h:107
+public:
+ /// Indicates that requests must be executed immidieately on the calling
+ /// thread.
Typo: immediately
https://reviews.llvm.org/D36261
__
ilya-biryukov created this revision.
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
==
16 matches
Mail list logo