[clang-tools-extra] 3c5745c - [clangd] Make background index thread count calculation clearer

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-06-25T00:18:53+02:00
New Revision: 3c5745cb1f3c501e551a8fa63b8f1d564c35016d

URL: https://github.com/llvm/llvm-project/commit/3c5745cb1f3c501e551a8fa63b8f1d564c35016d
DIFF: https://github.com/llvm/llvm-project/commit/3c5745cb1f3c501e551a8fa63b8f1d564c35016d.diff

LOG: [clangd] Make background index thread count calculation clearer

Summary:
This confusion was inadvertently introduced in a change to the
heavyweight_hardware_concurrency API: 8404aeb56a73ab24f9b295111de3b37a37f0b841

- don't indirect through the rebuilder policy when building the thread pool
- document that rebuilder thresholds are exposed for testing only
- don't use 0 as a sentinel value for "all threads", as we use it as a
  sentinel value for "synchronous" (though unsupported for BackgroundIndex)
- rather than pick some new sentinel value, just always use 4 threads for tests

Reviewers: kadircet, aganea

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82352

Added: 
    

Modified: 
    clang-tools-extra/clangd/index/Background.cpp
    clang-tools-extra/clangd/index/Background.h
    clang-tools-extra/clangd/index/BackgroundRebuild.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index 3e68d9e9c62a..5aae49345b65 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -103,10 +103,9 @@ BackgroundIndex::BackgroundIndex(
           CDB.watch([&](const std::vector<std::string> &ChangedFiles) {
             enqueue(ChangedFiles);
           })) {
-  assert(Rebuilder.TUsBeforeFirstBuild > 0 &&
-         "Thread pool size can't be zero.");
+  assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  for (unsigned I = 0; I < Rebuilder.TUsBeforeFirstBuild; ++I) {
+  for (unsigned I = 0; I < ThreadPoolSize; ++I) {
     ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), [this] {
       WithContext Ctx(this->BackgroundContext.clone());
       Queue.work([&] { Rebuilder.idle(); });

diff  --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h
index 9ae63828cc6b..9e6b9724af24 100644
--- a/clang-tools-extra/clangd/index/Background.h
+++ b/clang-tools-extra/clangd/index/Background.h
@@ -135,7 +135,9 @@ class BackgroundIndex : public SwapIndex {
       Context BackgroundContext, const ThreadsafeFS &,
       const GlobalCompilationDatabase &CDB,
       BackgroundIndexStorage::Factory IndexStorageFactory,
-      size_t ThreadPoolSize = 0, // 0 = use all hardware threads
+      // Arbitrary value to ensure some concurrency in tests.
+      // In production an explicit value is passed.
+      size_t ThreadPoolSize = 4,
       std::function<void(BackgroundQueue::Stats)> OnProgress = nullptr);
   ~BackgroundIndex(); // Blocks while the current task finishes.
 

diff  --git a/clang-tools-extra/clangd/index/BackgroundRebuild.h b/clang-tools-extra/clangd/index/BackgroundRebuild.h
index 295f705c98e8..bc68536c847f 100644
--- a/clang-tools-extra/clangd/index/BackgroundRebuild.h
+++ b/clang-tools-extra/clangd/index/BackgroundRebuild.h
@@ -49,9 +49,7 @@ class BackgroundIndexRebuilder {
 public:
   BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source,
                            unsigned Threads)
-      : TUsBeforeFirstBuild(llvm::heavyweight_hardware_concurrency(Threads)
-                                .compute_thread_count()),
-        Target(Target), Source(Source) {}
+      : TUsBeforeFirstBuild(Threads), Target(Target), Source(Source) {}
 
   // Called to indicate a TU has been indexed.
   // May rebuild, if enough TUs have been indexed.
@@ -72,7 +70,7 @@ class BackgroundIndexRebuilder {
   // Ensures we won't start any more rebuilds.
   void shutdown();
 
-  // Thresholds for rebuilding as TUs get indexed.
+  // Thresholds for rebuilding as TUs get indexed. Exposed for testing.
   const unsigned TUsBeforeFirstBuild; // Typically one per worker thread.
   const unsigned TUsBeforeRebuild = 100;