[clangd] Don't rebuild background index until we indexed one TU per thread.

[sammccall]

Summary

This increases the odds that the boosted file (cpp file matching header)
will be ready. (It always enqueues first, so it'll be present unless
another thread indexes *two* files before the first thread indexes one.)

[kadircet]

clang-tools-extra/clangd/index/BackgroundRebuild.h

51 ↗

(On Diff #209651)

let's not have a default here since it will get out-dated if we decide to change default for BackgroundIndex's constructor.

clang-tools-extra/trunk/clangd/index/Background.cpp

 } // namespace
 
 BackgroundIndex::BackgroundIndex(
     Context BackgroundContext, const FileSystemProvider &FSProvider,
     const GlobalCompilationDatabase &CDB,
     BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize)
     : SwapIndex(llvm::make_unique<MemIndex>()), FSProvider(FSProvider),
       CDB(CDB), BackgroundContext(std::move(BackgroundContext)),
-      Rebuilder(this, &IndexedSymbols),
+      Rebuilder(this, &IndexedSymbols, ThreadPoolSize),
       IndexStorageFactory(std::move(IndexStorageFactory)),
       CommandsChanged(
           CDB.watch([&](const std::vector<std::string> &ChangedFiles) {
             enqueue(ChangedFiles);
           })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
   for (unsigned I = 0; I < ThreadPoolSize; ++I) {

clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_BACKGROUND_INDEX_REBUILD_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_BACKGROUND_INDEX_REBUILD_H
 
 #include "index/FileIndex.h"
 #include "index/Index.h"
+#include "llvm/Support/Threading.h"
 
 namespace clang {
 namespace clangd {
 
 // The BackgroundIndexRebuilder builds the serving data structures periodically
 // in response to events in the background indexer. The goal is to ensure the
 // served data stays fairly fresh, without wasting lots of CPU rebuilding it
 // often.

 //
 // All methods are threadsafe. They're called after FileSymbols is updated
 // etc. Without external locking, the rebuilt index may include more updates
 // than intended, which is fine.
 //
 // This class is exposed in the header so it can be tested.
 class BackgroundIndexRebuilder {
 public:
-  // Thresholds for rebuilding as TUs get indexed.
-  static constexpr unsigned TUsBeforeFirstBuild = 5;
-  static constexpr unsigned TUsBeforeRebuild = 100;
-
-  BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source)
-      : Target(Target), Source(Source) {}
+  BackgroundIndexRebuilder(SwapIndex *Target, FileSymbols *Source,
+                           unsigned Threads)
+      : TUsBeforeFirstBuild(Threads), Target(Target), Source(Source) {}
 
   // Called to indicate a TU has been indexed.
   // May rebuild, if enough TUs have been indexed.
   void indexedTU();
   // Called to indicate that all worker threads are idle.
   // May reindex, if the index is not up to date.
   void idle();
   // Called to indicate we're going to load a batch of shards from disk.
   // startLoading() and doneLoading() must be paired, but multiple loading
   // sessions may happen concurrently.
   void startLoading();
   // Called to indicate some shards were actually loaded from disk.
   void loadedTU();
   // Called to indicate we're finished loading shards from disk.
   // May rebuild (if any were loaded).
   void doneLoading();
 
   // Ensures we won't start any more rebuilds.
   void shutdown();
 
+  // Thresholds for rebuilding as TUs get indexed.
+  const unsigned TUsBeforeFirstBuild; // Typically one per worker thread.
+  const unsigned TUsBeforeRebuild = 100;
+
 private:
   // Run Check under the lock, and rebuild if it returns true.
   void maybeRebuild(const char *Reason, std::function<bool()> Check);
   bool enoughTUsToRebuild() const;
 
   // All transient state is guarded by the mutex.
   std::mutex Mu;
   bool ShouldStop = false;

clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp

EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
     EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
     EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
   }
 }
 
 class BackgroundIndexRebuilderTest : public testing::Test {
 protected:
   BackgroundIndexRebuilderTest()
-      : Target(llvm::make_unique<MemIndex>()), Rebuilder(&Target, &Source) {
+      : Target(llvm::make_unique<MemIndex>()),
+        Rebuilder(&Target, &Source, /*Threads=*/10) {
     // Prepare FileSymbols with TestSymbol in it, for checkRebuild.
     TestSymbol.ID = SymbolID("foo");
   }
 
   // Perform Action and determine whether it rebuilt the index or not.
   bool checkRebuild(std::function<void()> Action) {
     // Update name so we can tell if the index updates.
     VersionStorage.push_back("Sym" + std::to_string(++VersionCounter));
protected:
   SwapIndex Target;
   BackgroundIndexRebuilder Rebuilder;
 
   unsigned VersionCounter = 0;
   std::deque<std::string> VersionStorage;
 };
 
 TEST_F(BackgroundIndexRebuilderTest, IndexingTUs) {
-  for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeFirstBuild - 1;
-       ++I)
+  for (unsigned I = 0; I < Rebuilder.TUsBeforeFirstBuild - 1; ++I)
     EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
   EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); }));
-  for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeRebuild - 1; ++I)
+  for (unsigned I = 0; I < Rebuilder.TUsBeforeRebuild - 1; ++I)
     EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
   EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); }));
 }
 
 TEST_F(BackgroundIndexRebuilderTest, LoadingShards) {
   Rebuilder.startLoading();
   Rebuilder.loadedTU();
   Rebuilder.loadedTU();
TEST_F(BackgroundIndexRebuilderTest, LoadingShards) {
   Rebuilder.startLoading();
   Rebuilder.loadedTU();
   EXPECT_FALSE(checkRebuild([&] { Rebuilder.doneLoading(); }));
   Rebuilder.loadedTU();
   EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); }));
 
   // No rebuilding for indexed files while loading.
   Rebuilder.startLoading();
-  for (unsigned I = 0; I < 3 * BackgroundIndexRebuilder::TUsBeforeRebuild; ++I)
+  for (unsigned I = 0; I < 3 * Rebuilder.TUsBeforeRebuild; ++I)
     EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
   // But they get indexed when we're done, even if no shards were loaded.
   EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); }));
 }
 
 TEST(BackgroundQueueTest, Priority) {
   // Create high and low priority tasks.
   // Once a bunch of high priority tasks have run, the queue is stopped.