[clang-tools-extra] r366199 - [clangd] Don't rebuild background index until we indexed one TU per thread.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Jul 16 03:17:06 2019
New Revision: 366199

URL: http://llvm.org/viewvc/llvm-project?rev=366199&view=rev
Log:
[clangd] Don't rebuild background index until we indexed one TU per thread.

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.)

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, llvm-commits

Tags: #llvm

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

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

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=366199&r1=366198&r2=366199&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Jul 16 03:17:06 2019
@@ -127,7 +127,7 @@ BackgroundIndex::BackgroundIndex(
     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) {

Modified: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h?rev=366199&r1=366198&r2=366199&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h (original)
+++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h Tue Jul 16 03:17:06 2019
@@ -16,6 +16,7 @@
 
 #include "index/FileIndex.h"
 #include "index/Index.h"
+#include "llvm/Support/Threading.h"
 
 namespace clang {
 namespace clangd {
@@ -45,12 +46,9 @@ namespace clangd {
 // 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.
@@ -71,6 +69,10 @@ public:
   // 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);

Modified: clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp?rev=366199&r1=366198&r2=366199&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Tue Jul 16 03:17:06 2019
@@ -575,7 +575,8 @@ TEST_F(BackgroundIndexTest, CmdLineHash)
 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");
   }
@@ -610,11 +611,10 @@ protected:
 };
 
 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(); }));
 }
@@ -640,7 +640,7 @@ TEST_F(BackgroundIndexRebuilderTest, Loa
 
   // 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(); }));