sammccall added inline comments.
Herald added subscribers: msifontes, jurahul, Kayjukh, stephenneuendorffer, 
aartbik.
Herald added a project: MLIR.


================
Comment at: clang-tools-extra/clangd/index/Background.cpp:154
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  for (unsigned I = 0; I < ThreadPoolSize; ++I) {
+  for (unsigned I = 0; I < Rebuilder.TUsBeforeFirstBuild; ++I) {
     ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), [this] {
----------------
Hmm, I finally stumbled across this today when editing the rebuild policy.
I do wish this hadn't been changed without understanding what this code is 
doing or getting review from an owner.

After this change, modifying the background-index rebuild frequency (which was 
initially tuned to be roughly "after each thread built one file") has the 
side-effect of changing the number of threads used for background indexing!

Less seriously, the use of zero to mean "all the threads" is problematic here 
because in the other thread roots in this project we use zero to mean "no 
threads, work synchronously".

I'll look into reverting the clangd parts of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71775



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to