[PATCH] D42174: [clangd] Refactored threading in ClangdServer

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 09:25:07 PST 2018


ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdServer.h:107
+/// A simple fixed-size thread pool implementation.
+class SimpleThreadPool {
 public:
----------------
bkramer wrote:
> What's so simple about it? Why not `clangd::ThreadPool`?
> 
> Also there's `llvm::ThreadPool`, what's the difference between them?
Will rename it to `ThreadPool`.
Differences are:
- `llvm::ThreadPool` always process requests in FIFO order, we allow LIFO here (for code completion).
- `llvm::ThreadPool` will run tasks synchronously when `LLVM_ENABLE_THREADS` is set to `0`. I'm not sure that makes sense for clangd, which has a runtime switch for that (`-run-synchronously` flag)
- `llvm::ThreadPool` will not process any tasks when `ThreadsCount` is set to `0`, our implementation processes the tasks synchronously instead.

I'll also be adding per-unit queues in the latter commit (aka thread affinity) to our thead pool, so it'll have more differences. I suggest waiting a day or two before I send the patch for review.

Another minor difference is:
- `llvm::ThreadPool` creates a `std::packaged_task` and `std::future` for each task, our implementation simply runs the provided actions. The latter means less book-keeping and is more efficient, but I don't think it matters.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42174





More information about the cfe-commits mailing list