kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric.

Whenever a change happens on a CDB, load shards associated with that
CDB before issuing re-index actions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55224

Files:
  clangd/index/Background.cpp
  clangd/index/Background.h
  test/clangd/background-index.test
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===================================================================
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -7,6 +7,7 @@
 
 using testing::_;
 using testing::AllOf;
+using testing::Contains;
 using testing::Not;
 using testing::UnorderedElementsAre;
 
@@ -120,7 +121,7 @@
                        FileURI("unittest:///root/B.cc")}));
 }
 
-TEST_F(BackgroundIndexTest, ShardStorageWriteTest) {
+TEST_F(BackgroundIndexTest, ShardStorageTest) {
   MockFSProvider FS;
   FS.Files[testPath("root/A.h")] = R"cpp(
       void common();
@@ -149,6 +150,16 @@
   EXPECT_EQ(CacheHits, 0U);
   EXPECT_EQ(Storage.size(), 2U);
 
+  {
+    OverlayCDB CDB(/*Base=*/nullptr);
+    BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+                        [&](llvm::StringRef) { return &MSS; });
+    CDB.setCompileCommand(testPath("root"), Cmd);
+    ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+  EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache.
+  EXPECT_EQ(Storage.size(), 2U);
+
   auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
   EXPECT_NE(ShardHeader, nullptr);
   EXPECT_THAT(
@@ -208,5 +219,73 @@
       UnorderedElementsAre("unittest:///root/B.h"));
 }
 
+TEST_F(BackgroundIndexTest, ShardStorageLoad) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.h")] = R"cpp(
+      void common();
+      void f_b();
+      class A_CC {};
+      )cpp";
+  FS.Files[testPath("root/A.cc")] =
+      "#include \"A.h\"\nvoid g() { (void)common; }";
+
+  llvm::StringMap<std::string> Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
+  // Check nothing is loaded from Storage, but A.cc and A.h has been stored.
+  {
+    OverlayCDB CDB(/*Base=*/nullptr);
+    BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+                        [&](llvm::StringRef) { return &MSS; });
+    CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+    ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+
+  // Change header.
+  FS.Files[testPath("root/A.h")] = R"cpp(
+      void common();
+      void f_b();
+      class A_CC {};
+      class A_CCnew {};
+      )cpp";
+  {
+    OverlayCDB CDB(/*Base=*/nullptr);
+    BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+                        [&](llvm::StringRef) { return &MSS; });
+    CDB.setCompileCommand(testPath("root"), Cmd);
+    ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+  EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache.
+
+  // Check if the new symbol has arrived.
+  auto ShardHeader = MSS.loadShard(testPath("root/A.h"));
+  EXPECT_NE(ShardHeader, nullptr);
+  EXPECT_THAT(*ShardHeader->Symbols, Contains(Named("A_CCnew")));
+
+  // Change source.
+  FS.Files[testPath("root/A.cc")] =
+      "#include \"A.h\"\nvoid g() { (void)common; }\nvoid f_b() {}";
+  {
+    CacheHits = 0;
+    OverlayCDB CDB(/*Base=*/nullptr);
+    BackgroundIndex Idx(Context::empty(), "", FS, CDB,
+                        [&](llvm::StringRef) { return &MSS; });
+    CDB.setCompileCommand(testPath("root"), Cmd);
+    ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  }
+  EXPECT_EQ(CacheHits, 2U); // Check both A.cc and A.h loaded from cache.
+
+  // Check if the new symbol has arrived.
+  auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
+  EXPECT_NE(ShardHeader, nullptr);
+  EXPECT_THAT(*ShardSource->Symbols,
+              Contains(AllOf(Named("f_b"), Declared(), Defined())));
+}
+
 } // namespace clangd
 } // namespace clang
Index: test/clangd/background-index.test
===================================================================
--- test/clangd/background-index.test
+++ test/clangd/background-index.test
@@ -16,6 +16,5 @@
 # RUN: ls %t/.clangd-index/foo.cpp.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
-# FIXME: This test currently fails as we don't read the index yet.
 # RUN: rm %t/foo.cpp
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | not FileCheck %t/definition.jsonrpc
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc
Index: clangd/index/Background.h
===================================================================
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -74,7 +74,6 @@
   // The indexing happens in a background thread, so the symbols will be
   // available sometime later.
   void enqueue(const std::vector<std::string> &ChangedFiles);
-  void enqueue(const std::string &File);
 
   // Cause background threads to stop after ther current task, any remaining
   // tasks will be discarded.
@@ -106,13 +105,19 @@
   std::mutex DigestsMu;
 
   BackgroundIndexStorage::Factory IndexStorageFactory;
+  // Loads the shards for all the sources and headers of the Cmd. Returns the
+  // list of dependencies for this Cmd and whether they need to be re-indexed.
+  std::vector<
+      std::pair</*Path to dependency*/ std::string, /*NeedsReIndexing*/ bool>>
+  loadShard(const tooling::CompileCommand &Cmd,
+            BackgroundIndexStorage *IndexStorage);
+  llvm::Error loadShards(std::vector<std::string> ChangedFiles);
+  void enqueue(tooling::CompileCommand Cmd, BackgroundIndexStorage *Storage);
 
   // queue management
   using Task = std::function<void()>;
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
   void enqueueTask(Task T);
-  void enqueueLocked(tooling::CompileCommand Cmd,
-                     BackgroundIndexStorage *IndexStorage);
   std::mutex QueueMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
   std::condition_variable QueueCV;
Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -87,6 +87,17 @@
 
   return IG;
 }
+
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
+  llvm::SmallString<128> AbsolutePath;
+  if (sys::path::is_absolute(Cmd.Filename)) {
+    AbsolutePath = Cmd.Filename;
+  } else {
+    AbsolutePath = Cmd.Directory;
+    sys::path::append(AbsolutePath, Cmd.Filename);
+  }
+  return AbsolutePath;
+}
 } // namespace
 
 BackgroundIndex::BackgroundIndex(
@@ -164,35 +175,23 @@
   enqueueTask([this, ChangedFiles] {
     trace::Span Tracer("BackgroundIndexEnqueue");
     // We're doing this asynchronously, because we'll read shards here too.
-    // FIXME: read shards here too.
-
     log("Enqueueing {0} commands for indexing", ChangedFiles.size());
     SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
 
-    // We shuffle the files because processing them in a random order should
-    // quickly give us good coverage of headers in the project.
-    std::vector<unsigned> Permutation(ChangedFiles.size());
-    std::iota(Permutation.begin(), Permutation.end(), 0);
-    std::mt19937 Generator(std::random_device{}());
-    std::shuffle(Permutation.begin(), Permutation.end(), Generator);
-
-    for (const unsigned I : Permutation)
-      enqueue(ChangedFiles[I]);
+    if (auto Error = loadShards(std::move(ChangedFiles)))
+      log("Loading shards failed: {0}", std::move(Error));
   });
 }
 
-void BackgroundIndex::enqueue(const std::string &File) {
-  ProjectInfo Project;
-  if (auto Cmd = CDB.getCompileCommand(File, &Project)) {
-    auto *Storage = IndexStorageFactory(Project.SourceRoot);
-    enqueueTask(Bind(
-        [this, File, Storage](tooling::CompileCommand Cmd) {
-          Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
-          if (auto Error = index(std::move(Cmd), Storage))
-            log("Indexing {0} failed: {1}", File, std::move(Error));
-        },
-        std::move(*Cmd)));
-  }
+void BackgroundIndex::enqueue(tooling::CompileCommand Cmd,
+                              BackgroundIndexStorage *Storage) {
+  enqueueTask(Bind(
+      [this, Storage](tooling::CompileCommand Cmd) {
+        Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
+        if (auto Error = index(std::move(Cmd), Storage))
+          log("Indexing {0} failed: {1}", Cmd.Filename, std::move(Error));
+      },
+      std::move(Cmd)));
 }
 
 void BackgroundIndex::enqueueTask(Task T) {
@@ -336,11 +335,6 @@
   llvm::StringMap<FileDigest> DigestsSnapshot;
   {
     std::lock_guard<std::mutex> Lock(DigestsMu);
-    if (IndexedFileDigests.lookup(AbsolutePath) == Hash) {
-      vlog("No need to index {0}, already up to date", AbsolutePath);
-      return Error::success();
-    }
-
     DigestsSnapshot = IndexedFileDigests;
   }
 
@@ -406,5 +400,103 @@
   return Error::success();
 }
 
+std::vector<std::pair<std::string, bool>>
+BackgroundIndex::loadShard(const tooling::CompileCommand &Cmd,
+                           BackgroundIndexStorage *IndexStorage) {
+  // Consumes Symbols and Refs in Shard.
+  auto LoadShard = [&](llvm::StringRef AbsolutePath, IndexFileIn *Shard,
+                       const IncludeGraphNode &IGN) {
+    if (!Shard)
+      return;
+    std::unique_ptr<SymbolSlab> SS;
+    std::unique_ptr<RefSlab> RS;
+    if (Shard->Symbols)
+      SS = llvm::make_unique<SymbolSlab>(std::move(*Shard->Symbols));
+    if (Shard->Refs)
+      RS = llvm::make_unique<RefSlab>(std::move(*Shard->Refs));
+    // Load shard information into background-index.
+    {
+      std::lock_guard<std::mutex> Lock(DigestsMu);
+      // This can override a newer version that is added in another thread,
+      // if this thread sees the older version but finishes later. This
+      // should be rare in practice.
+      IndexedFileDigests[AbsolutePath] = IGN.Digest;
+      IndexedSymbols.update(AbsolutePath, std::move(SS), std::move(RS));
+    }
+  };
+
+  auto FS = FSProvider.getFileSystem();
+  std::vector<std::pair<std::string, bool>> Dependencies;
+  Dependencies.push_back({getAbsolutePath(Cmd).str(), true});
+  for (size_t CurrentDependency = 0; CurrentDependency < Dependencies.size();
+       CurrentDependency++) {
+    auto Dependency = Dependencies[CurrentDependency];
+    auto Shard = IndexStorage->loadShard(Dependency.first);
+
+    if (!Shard || !Shard->Sources) {
+      elog("Failed to load shard for: {0}", Dependency.first);
+      continue;
+    }
+    for (const auto &I : *Shard->Sources) {
+      if (auto U = URI::parse(I.getKey())) {
+        if (auto AbsolutePath = URI::resolve(*U, Dependency.first)) {
+          if (*AbsolutePath != Dependency.first) {
+            Dependencies.push_back({AbsolutePath.get(), true});
+            continue;
+          }
+
+          // If we found source file info for current file.
+          LoadShard(AbsolutePath.get(), Shard.get(), I.getValue());
+          // Check if the source needs re-indexing.
+          // Get the digest, skip it if file doesn't exist.
+          auto Buf = FS->getBufferForFile(AbsolutePath.get());
+          if (!Buf)
+            continue;
+          // If digests match then dependency doesn't need re-indexing.
+          Dependency.second =
+              digest(Buf->get()->getBuffer()) != I.getValue().Digest;
+        }
+      }
+    }
+    Dependencies[CurrentDependency] = std::move(Dependency);
+  }
+
+  // FIXME: this should rebuild once-in-a-while, not after every file.
+  //       At that point we should use Dex, too.
+  vlog("Rebuilding automatic index");
+  reset(IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
+
+  return Dependencies;
+}
+
+llvm::Error BackgroundIndex::loadShards(std::vector<std::string> ChangedFiles) {
+  llvm::StringSet<> BeingReindexed;
+  for (const auto &File : ChangedFiles) {
+    ProjectInfo PI;
+    auto Cmd = CDB.getCompileCommand(File, &PI);
+    if (!Cmd)
+      continue;
+    BackgroundIndexStorage *IndexStorage = IndexStorageFactory(PI.SourceRoot);
+    auto Dependencies = loadShard(*Cmd, IndexStorage);
+    for (const auto &Dependency : Dependencies) {
+      // FIXME: Currently, we simply schedule indexing on a TU whenever any of
+      // its dependencies needs re-indexing. We might do it more smartly by
+      // figuring out a minimal set of TUs that will cover all the stale
+      // dependencies.
+      if (Dependency.second && !BeingReindexed.count(Dependency.first)) {
+        enqueue(std::move(*Cmd), IndexStorage);
+        // Mark all of this TUs dependencies as to-be-indexed so that we won't
+        // try to re-index those.
+        std::for_each(Dependencies.begin(), Dependencies.end(),
+                      [&BeingReindexed](decltype(Dependency) Dependency) {
+                        BeingReindexed.insert(Dependency.first);
+                      });
+        break;
+      }
+    }
+  }
+  return Error::success();
+}
+
 } // namespace clangd
 } // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to