[clang-tools-extra] r365531 - [clangd] Rewrite of logic to rebuild the background index serving structures.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 9 11:30:49 PDT 2019


Author: sammccall
Date: Tue Jul  9 11:30:49 2019
New Revision: 365531

URL: http://llvm.org/viewvc/llvm-project?rev=365531&view=rev
Log:
[clangd] Rewrite of logic to rebuild the background index serving structures.

Summary:
Previously it was rebuilding every 5s by default, which was much too frequent
in the long run - the goal was to provide an early build. There were also some
bugs. There were also some bugs, and a dedicated thread was used in production
but not tested.

 - rebuilds are triggered by #TUs built, rather than time. This should scale
   more sensibly to fast vs slow machines.
 - there are two separate indexed-TU thresholds to trigger index build: 5 TUs
   for the first build, 100 for subsequent rebuilds.
 - rebuild is always done on the regular indexing threads, and is affected by
   blockUntilIdle. This means unit/lit tests run the production configuration.
 - fixed a bug where we'd rebuild after attempting to load shards, even if there
   were no shards.
 - the BackgroundIndexTests don't really test the subtleties of the rebuild
   policy (for determinism, we call blockUntilIdle, so rebuild-on-idle is enough
   to pass the tests). Instead, we expose the rebuilder as a separate class and
   have fine-grained tests for it.

Reviewers: kadircet

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

Tags: #llvm

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

Added:
    clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp
    clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h
Modified:
    clang-tools-extra/trunk/clangd/CMakeLists.txt
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/index/Background.cpp
    clang-tools-extra/trunk/clangd/index/Background.h
    clang-tools-extra/trunk/clangd/test/background-index.test
    clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
    clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Jul  9 11:30:49 2019
@@ -73,6 +73,7 @@ add_clang_library(clangDaemon
   XRefs.cpp
 
   index/Background.cpp
+  index/BackgroundRebuild.cpp
   index/BackgroundIndexStorage.cpp
   index/CanonicalIncludes.cpp
   index/FileIndex.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jul  9 11:30:49 2019
@@ -128,8 +128,7 @@ ClangdServer::ClangdServer(const GlobalC
   if (Opts.BackgroundIndex) {
     BackgroundIdx = llvm::make_unique<BackgroundIndex>(
         Context::current().clone(), FSProvider, CDB,
-        BackgroundIndexStorage::createDiskBackedStorageFactory(),
-        Opts.BackgroundIndexRebuildPeriodMs);
+        BackgroundIndexStorage::createDiskBackedStorageFactory());
     AddIndex(BackgroundIdx.get());
   }
   if (DynamicIdx)

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Jul  9 11:30:49 2019
@@ -98,10 +98,6 @@ public:
     /// If true, ClangdServer automatically indexes files in the current project
     /// on background threads. The index is stored in the project root.
     bool BackgroundIndex = false;
-    /// If set to non-zero, the background index rebuilds the symbol index
-    /// periodically every BuildIndexPeriodMs milliseconds; otherwise, the
-    /// symbol index will be updated for each indexed file.
-    size_t BackgroundIndexRebuildPeriodMs = 0;
 
     /// If set, use this index to augment code completion results.
     SymbolIndex *StaticIndex = nullptr;

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=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Jul  9 11:30:49 2019
@@ -17,6 +17,7 @@
 #include "Threading.h"
 #include "Trace.h"
 #include "URI.h"
+#include "index/FileIndex.h"
 #include "index/IndexAction.h"
 #include "index/MemIndex.h"
 #include "index/Ref.h"
@@ -36,7 +37,9 @@
 
 #include <atomic>
 #include <chrono>
+#include <condition_variable>
 #include <memory>
+#include <mutex>
 #include <numeric>
 #include <queue>
 #include <random>
@@ -119,12 +122,10 @@ llvm::SmallString<128> getAbsolutePath(c
 BackgroundIndex::BackgroundIndex(
     Context BackgroundContext, const FileSystemProvider &FSProvider,
     const GlobalCompilationDatabase &CDB,
-    BackgroundIndexStorage::Factory IndexStorageFactory,
-    size_t BuildIndexPeriodMs, size_t ThreadPoolSize)
+    BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize)
     : SwapIndex(llvm::make_unique<MemIndex>()), FSProvider(FSProvider),
       CDB(CDB), BackgroundContext(std::move(BackgroundContext)),
-      BuildIndexPeriodMs(BuildIndexPeriodMs),
-      SymbolsUpdatedSinceLastIndex(false),
+      Rebuilder(this, &IndexedSymbols),
       IndexStorageFactory(std::move(IndexStorageFactory)),
       CommandsChanged(
           CDB.watch([&](const std::vector<std::string> &ChangedFiles) {
@@ -136,11 +137,6 @@ BackgroundIndex::BackgroundIndex(
     ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1),
                         [this] { run(); });
   }
-  if (BuildIndexPeriodMs > 0) {
-    log("BackgroundIndex: build symbol index periodically every {0} ms.",
-        BuildIndexPeriodMs);
-    ThreadPool.runAsync("background-index-builder", [this] { buildIndex(); });
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -149,6 +145,7 @@ BackgroundIndex::~BackgroundIndex() {
 }
 
 void BackgroundIndex::stop() {
+  Rebuilder.shutdown();
   {
     std::lock_guard<std::mutex> QueueLock(QueueMu);
     std::lock_guard<std::mutex> IndexLock(IndexMu);
@@ -184,6 +181,12 @@ void BackgroundIndex::run() {
 
     {
       std::unique_lock<std::mutex> Lock(QueueMu);
+      if (NumActiveTasks == 1 && Queue.empty()) {
+        // We just finished the last item, the queue is going idle.
+        Lock.unlock();
+        Rebuilder.idle();
+        Lock.lock();
+      }
       assert(NumActiveTasks > 0 && "before decrementing");
       --NumActiveTasks;
     }
@@ -378,32 +381,6 @@ void BackgroundIndex::update(
   }
 }
 
-void BackgroundIndex::buildIndex() {
-  assert(BuildIndexPeriodMs > 0);
-  while (true) {
-    {
-      std::unique_lock<std::mutex> Lock(IndexMu);
-      if (ShouldStop) // Avoid waiting if stopped.
-        break;
-      // Wait until this is notified to stop or `BuildIndexPeriodMs` has past.
-      IndexCV.wait_for(Lock, std::chrono::milliseconds(BuildIndexPeriodMs));
-      if (ShouldStop) // Avoid rebuilding index if stopped.
-        break;
-    }
-    if (!SymbolsUpdatedSinceLastIndex.exchange(false))
-      continue;
-    // There can be symbol update right after the flag is reset above and before
-    // index is rebuilt below. The new index would contain the updated symbols
-    // but the flag would still be true. This is fine as we would simply run an
-    // extra index build.
-    reset(
-        IndexedSymbols.buildIndex(IndexType::Heavy, DuplicateHandling::Merge));
-    log("BackgroundIndex: rebuilt symbol index with estimated memory {0} "
-        "bytes.",
-        estimateMemoryUsage());
-  }
-}
-
 llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd,
                                    BackgroundIndexStorage *IndexStorage) {
   trace::Span Tracer("BackgroundIndex");
@@ -502,12 +479,7 @@ llvm::Error BackgroundIndex::index(tooli
   update(AbsolutePath, std::move(Index), ShardVersionsSnapshot, IndexStorage,
          HadErrors);
 
-  if (BuildIndexPeriodMs > 0)
-    SymbolsUpdatedSinceLastIndex = true;
-  else
-    reset(
-        IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
-
+  Rebuilder.indexedTU();
   return llvm::Error::success();
 }
 
@@ -627,6 +599,8 @@ BackgroundIndex::loadShard(const tooling
                             std::move(RelS), SI.CountReferences);
     }
   }
+  if (!IntermediateSymbols.empty())
+    Rebuilder.loadedTU();
 
   return Dependencies;
 }
@@ -643,6 +617,7 @@ BackgroundIndex::loadShards(std::vector<
   // Keeps track of the loaded shards to make sure we don't perform redundant
   // disk IO. Keys are absolute paths.
   llvm::StringSet<> LoadedShards;
+  Rebuilder.startLoading();
   for (const auto &File : ChangedFiles) {
     ProjectInfo PI;
     auto Cmd = CDB.getCompileCommand(File, &PI);
@@ -666,11 +641,7 @@ BackgroundIndex::loadShards(std::vector<
       break;
     }
   }
-  vlog("Loaded all shards");
-  reset(IndexedSymbols.buildIndex(IndexType::Heavy, DuplicateHandling::Merge));
-  vlog("BackgroundIndex: built symbol index with estimated memory {0} "
-       "bytes.",
-       estimateMemoryUsage());
+  Rebuilder.doneLoading();
   return NeedsReIndexing;
 }
 

Modified: clang-tools-extra/trunk/clangd/index/Background.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Tue Jul  9 11:30:49 2019
@@ -14,6 +14,7 @@
 #include "GlobalCompilationDatabase.h"
 #include "SourceCode.h"
 #include "Threading.h"
+#include "index/BackgroundRebuild.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -71,7 +72,6 @@ public:
       Context BackgroundContext, const FileSystemProvider &,
       const GlobalCompilationDatabase &CDB,
       BackgroundIndexStorage::Factory IndexStorageFactory,
-      size_t BuildIndexPeriodMs = 0,
       size_t ThreadPoolSize = llvm::heavyweight_hardware_concurrency());
   ~BackgroundIndex(); // Blocks while the current task finishes.
 
@@ -114,13 +114,11 @@ private:
   // index state
   llvm::Error index(tooling::CompileCommand,
                     BackgroundIndexStorage *IndexStorage);
-  void buildIndex(); // Rebuild index periodically every BuildIndexPeriodMs.
-  const size_t BuildIndexPeriodMs;
-  std::atomic<bool> SymbolsUpdatedSinceLastIndex;
   std::mutex IndexMu;
   std::condition_variable IndexCV;
 
   FileSymbols IndexedSymbols;
+  BackgroundIndexRebuilder Rebuilder;
   llvm::StringMap<ShardVersion> ShardVersions; // Key is absolute file path.
   std::mutex ShardVersionsMu;
 

Added: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp?rev=365531&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp (added)
+++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.cpp Tue Jul  9 11:30:49 2019
@@ -0,0 +1,137 @@
+//===-- BackgroundRebuild.cpp - when to rebuild thei background index -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "index/BackgroundRebuild.h"
+#include "ClangdUnit.h"
+#include "Compiler.h"
+#include "Headers.h"
+#include "Logger.h"
+#include "Path.h"
+#include "SourceCode.h"
+#include "Symbol.h"
+#include "Threading.h"
+#include "Trace.h"
+#include "URI.h"
+#include "index/FileIndex.h"
+#include "index/IndexAction.h"
+#include "index/MemIndex.h"
+#include "index/Ref.h"
+#include "index/Relation.h"
+#include "index/Serialization.h"
+#include "index/SymbolCollector.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Threading.h"
+
+#include <atomic>
+#include <chrono>
+#include <condition_variable>
+#include <memory>
+#include <mutex>
+#include <numeric>
+#include <queue>
+#include <random>
+#include <string>
+#include <thread>
+
+namespace clang {
+namespace clangd {
+
+bool BackgroundIndexRebuilder::enoughTUsToRebuild() const {
+  if (!ActiveVersion)                         // never built
+    return IndexedTUs == TUsBeforeFirstBuild; // use low threshold
+  // rebuild if we've reached the (higher) threshold
+  return IndexedTUs >= IndexedTUsAtLastRebuild + TUsBeforeRebuild;
+}
+
+void BackgroundIndexRebuilder::indexedTU() {
+  maybeRebuild("after indexing enough files", [this] {
+    ++IndexedTUs;
+    if (Loading)
+      return false;                      // rebuild once loading finishes
+    if (ActiveVersion != StartedVersion) // currently building
+      return false;                      // no urgency, avoid overlapping builds
+    return enoughTUsToRebuild();
+  });
+}
+
+void BackgroundIndexRebuilder::idle() {
+  maybeRebuild("when background indexer is idle", [this] {
+    // rebuild if there's anything new in the index.
+    // (even if currently rebuilding! this ensures eventual completeness)
+    return IndexedTUs > IndexedTUsAtLastRebuild;
+  });
+}
+
+void BackgroundIndexRebuilder::startLoading() {
+  std::lock_guard<std::mutex> Lock(Mu);
+  if (!Loading)
+    LoadedTUs = 0;
+  ++Loading;
+}
+void BackgroundIndexRebuilder::loadedTU() {
+  std::lock_guard<std::mutex> Lock(Mu);
+  assert(Loading);
+  ++LoadedTUs;
+}
+void BackgroundIndexRebuilder::doneLoading() {
+  maybeRebuild("after loading index from disk", [this] {
+    assert(Loading);
+    --Loading;
+    if (Loading)    // was loading multiple batches concurrently
+      return false; // rebuild once the last batch is done.
+    // Rebuild if we loaded any shards, or if we stopped an indexedTU rebuild.
+    return LoadedTUs > 0 || enoughTUsToRebuild();
+  });
+}
+
+void BackgroundIndexRebuilder::shutdown() {
+  std::lock_guard<std::mutex> Lock(Mu);
+  ShouldStop = true;
+}
+
+void BackgroundIndexRebuilder::maybeRebuild(const char *Reason,
+                                            std::function<bool()> Check) {
+  unsigned BuildVersion = 0;
+  {
+    std::lock_guard<std::mutex> Lock(Mu);
+    if (!ShouldStop && Check()) {
+      BuildVersion = ++StartedVersion;
+      IndexedTUsAtLastRebuild = IndexedTUs;
+    }
+  }
+  if (BuildVersion) {
+    std::unique_ptr<SymbolIndex> NewIndex;
+    {
+      vlog("BackgroundIndex: building version {0} {1}", BuildVersion, Reason);
+      trace::Span Tracer("RebuildBackgroundIndex");
+      SPAN_ATTACH(Tracer, "reason", Reason);
+      NewIndex = Source->buildIndex(IndexType::Heavy, DuplicateHandling::Merge);
+    }
+    {
+      std::lock_guard<std::mutex> Lock(Mu);
+      // Guard against rebuild finishing in the wrong order.
+      if (BuildVersion > ActiveVersion) {
+        ActiveVersion = BuildVersion;
+        vlog("BackgroundIndex: serving version {0} ({1} bytes)", BuildVersion,
+             NewIndex->estimateMemoryUsage());
+        Target->reset(std::move(NewIndex));
+      }
+    }
+  }
+}
+
+} // namespace clangd
+} // namespace clang

Added: clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h?rev=365531&view=auto
==============================================================================
--- clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h (added)
+++ clang-tools-extra/trunk/clangd/index/BackgroundRebuild.h Tue Jul  9 11:30:49 2019
@@ -0,0 +1,99 @@
+//===--- BackgroundIndexRebuild.h - when to rebuild the bg index--*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains an implementation detail of the background indexer
+// (Background.h), which is exposed for testing.
+//
+//===----------------------------------------------------------------------===//
+
+#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"
+
+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.
+//
+// The index is always built after a set of shards are loaded from disk.
+// This happens when clangd discovers a compilation database that we've
+// previously built an index for. It's a fairly fast process that yields lots
+// of data, so we wait to get all of it.
+//
+// The index is built after indexing a few translation units, if it wasn't built
+// already. This ensures quick startup if there's no existing index.
+// Waiting for a few random TUs yields coverage of the most common headers.
+//
+// The index is rebuilt every N TUs, to keep if fresh as files are indexed.
+//
+// The index is rebuilt every time the queue goes idle, if it's stale.
+//
+// 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) {}
+
+  // 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();
+
+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;
+  // Index builds are versioned. ActiveVersion chases StartedVersion.
+  unsigned StartedVersion = 0;
+  unsigned ActiveVersion = 0;
+  // How many TUs have we indexed so far since startup?
+  unsigned IndexedTUs = 0;
+  unsigned IndexedTUsAtLastRebuild = 0;
+  // Are we loading shards? May be multiple concurrent sessions.
+  unsigned Loading = 0;
+  unsigned LoadedTUs; // In the current loading session.
+
+  SwapIndex *Target;
+  FileSymbols *Source;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif

Modified: clang-tools-extra/trunk/clangd/test/background-index.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/background-index.test?rev=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/test/background-index.test (original)
+++ clang-tools-extra/trunk/clangd/test/background-index.test Tue Jul  9 11:30:49 2019
@@ -10,7 +10,7 @@
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
-# RUN: clangd -background-index -background-index-rebuild-period=0 -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc
 
 # Test that the index is writing files in the expected location.
 # RUN: ls %t/.clangd/index/foo.cpp.*.idx

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Jul  9 11:30:49 2019
@@ -190,14 +190,6 @@ static llvm::cl::opt<bool> EnableBackgro
         "Experimental"),
     llvm::cl::init(true));
 
-static llvm::cl::opt<int> BackgroundIndexRebuildPeriod(
-    "background-index-rebuild-period",
-    llvm::cl::desc(
-        "If set to non-zero, the background index rebuilds the symbol index "
-        "periodically every X milliseconds; otherwise, the "
-        "symbol index will be updated for each indexed file"),
-    llvm::cl::init(5000), llvm::cl::Hidden);
-
 enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
 static llvm::cl::opt<CompileArgsFrom> CompileArgsFrom(
     "compile_args_from", llvm::cl::desc("The source of compile commands"),
@@ -465,7 +457,6 @@ int main(int argc, char *argv[]) {
     Opts.ResourceDir = ResourceDir;
   Opts.BuildDynamicSymbolIndex = EnableIndex;
   Opts.BackgroundIndex = EnableBackgroundIndex;
-  Opts.BackgroundIndexRebuildPeriodMs = BackgroundIndexRebuildPeriod;
   std::unique_ptr<SymbolIndex> StaticIdx;
   std::future<void> AsyncIndexLoad; // Block exit while loading the index.
   if (EnableIndex && !IndexFile.empty()) {

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=365531&r1=365530&r2=365531&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Tue Jul  9 11:30:49 2019
@@ -1,8 +1,10 @@
 #include "Headers.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
+#include "TestIndex.h"
 #include "TestTU.h"
 #include "index/Background.h"
+#include "index/BackgroundRebuild.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
@@ -294,42 +296,6 @@ TEST_F(BackgroundIndexTest, DirectInclud
               EmptyIncludeNode());
 }
 
-// FIXME: figure out the right timeouts or rewrite to not use the timeouts and
-// re-enable.
-TEST_F(BackgroundIndexTest, DISABLED_PeriodicalIndex) {
-  MockFSProvider FS;
-  llvm::StringMap<std::string> Storage;
-  size_t CacheHits = 0;
-  MemoryShardStorage MSS(Storage, CacheHits);
-  OverlayCDB CDB(/*Base=*/nullptr);
-  BackgroundIndex Idx(
-      Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; },
-      /*BuildIndexPeriodMs=*/500);
-
-  FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
-
-  tooling::CompileCommand Cmd;
-  FS.Files[testPath("root/A.h")] = "class X {};";
-  Cmd.Filename = testPath("root/A.cc");
-  Cmd.CommandLine = {"clang++", Cmd.Filename};
-  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
-
-  ASSERT_TRUE(Idx.blockUntilIdleForTest());
-  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre());
-  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
-  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X")));
-
-  FS.Files[testPath("root/A.h")] = "class Y {};";
-  FS.Files[testPath("root/A.cc")] += " "; // Force reindex the file.
-  Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
-  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
-
-  ASSERT_TRUE(Idx.blockUntilIdleForTest());
-  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X")));
-  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
-  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("Y")));
-}
-
 TEST_F(BackgroundIndexTest, ShardStorageLoad) {
   MockFSProvider FS;
   FS.Files[testPath("root/A.h")] = R"cpp(
@@ -605,5 +571,75 @@ TEST_F(BackgroundIndexTest, CmdLineHash)
   }
 }
 
+class BackgroundIndexRebuilderTest : public testing::Test {
+protected:
+  BackgroundIndexRebuilderTest()
+      : Target(llvm::make_unique<MemIndex>()), Rebuilder(&Target, &Source) {
+    // 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 reference count so we can tell if the index updates.
+    ++TestSymbol.References;
+    SymbolSlab::Builder SB;
+    SB.insert(TestSymbol);
+    Source.update("", llvm::make_unique<SymbolSlab>(std::move(SB).build()),
+                  nullptr, nullptr, false);
+    // Now maybe update the index.
+    Action();
+    // Now query the index to get the reference count.
+    unsigned ReadReferences = 0;
+    LookupRequest Req;
+    Req.IDs.insert(TestSymbol.ID);
+    Target.lookup(Req, [&](const Symbol &S) { ReadReferences = S.References; });
+    // The index was rebuild if the reference count is up to date.
+    return ReadReferences == TestSymbol.References;
+  }
+
+  Symbol TestSymbol;
+  FileSymbols Source;
+  SwapIndex Target;
+  BackgroundIndexRebuilder Rebuilder;
+};
+
+TEST_F(BackgroundIndexRebuilderTest, IndexingTUs) {
+  for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeFirstBuild - 1;
+       ++I)
+    EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
+  EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); }));
+  for (unsigned I = 0; I < BackgroundIndexRebuilder::TUsBeforeRebuild - 1; ++I)
+    EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
+  EXPECT_TRUE(checkRebuild([&] { Rebuilder.indexedTU(); }));
+}
+
+TEST_F(BackgroundIndexRebuilderTest, LoadingShards) {
+  Rebuilder.startLoading();
+  Rebuilder.loadedTU();
+  Rebuilder.loadedTU();
+  EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); }));
+
+  // No rebuild for no shards.
+  Rebuilder.startLoading();
+  EXPECT_FALSE(checkRebuild([&] { Rebuilder.doneLoading(); }));
+
+  // Loads can overlap.
+  Rebuilder.startLoading();
+  Rebuilder.loadedTU();
+  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)
+    EXPECT_FALSE(checkRebuild([&] { Rebuilder.indexedTU(); }));
+  // But they get indexed when we're done, even if no shards were loaded.
+  EXPECT_TRUE(checkRebuild([&] { Rebuilder.doneLoading(); }));
+}
+
 } // namespace clangd
 } // namespace clang




More information about the cfe-commits mailing list