[PATCH] D61724: [clangd] Use AsyncTaskRunner in BackgroundIndex instead of std::thread

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:152
+  for (unsigned I = 1; I <= ThreadPoolSize; ++I) {
+    ThreadPool.runAsync("background-worker-" + llvm::Twine(I),
+                        [this] { run(); });
----------------
kadircet wrote:
> NIT: why not count from zero :P
Done. The worker numbers still start with 1, sorry about that :-)


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:164
   stop();
-  for (auto &Thread : ThreadPool)
-    Thread.join();
+  ThreadPool.wait();
 }
----------------
kadircet wrote:
> Destructor of `AsyncTaskRunner` already does that
Yeah, but then we'll have to ensure the `AsyncTaskRunner` is the last member (I've been bitten by this before).
Having an explicit call to `wait()` in the destructor is bulletproof.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61724/new/

https://reviews.llvm.org/D61724