[clang-tools-extra] r347538 - [clangd] Auto-index watches global CDB for changes.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 26 01:51:50 PST 2018


Author: sammccall
Date: Mon Nov 26 01:51:50 2018
New Revision: 347538

URL: http://llvm.org/viewvc/llvm-project?rev=347538&view=rev
Log:
[clangd] Auto-index watches global CDB for changes.

Summary:
Instead of receiving compilation commands, auto-index is triggered by just
filenames to reindex, and gets commands from the global comp DB internally.
This has advantages:
 - more of the work can be done asynchronously (fetching compilation commands
   upfront can be slow for large CDBs)
 - we get access to the CDB which can be used to retrieve interpolated commands
   for headers (useful in some cases where the original TU goes away)
 - fits nicely with the filename-only change observation from r347297

The interface to GlobalCompilationDatabase gets extended: when retrieving a
compile command, the GCDB can optionally report the project the file belongs to.
This naturally fits together with getCompileCommand: it's hard to implement one
without the other. But because most callers don't care, I've ended up with an
awkward optional-out-param-in-virtual method pattern - maybe there's a better
one.

This is the main missing integration point between ClangdServer and
BackgroundIndex, after this we should be able to add an auto-index flag.

Reviewers: ioeric, kadircet

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

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

Modified:
    clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
    clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
    clang-tools-extra/trunk/clangd/index/Background.cpp
    clang-tools-extra/trunk/clangd/index/Background.h
    clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp
    clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
    clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
    clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
    clang-tools-extra/trunk/unittests/clangd/TestFS.h

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Mon Nov 26 01:51:50 2018
@@ -38,11 +38,13 @@ DirectoryBasedGlobalCompilationDatabase:
     ~DirectoryBasedGlobalCompilationDatabase() = default;
 
 Optional<tooling::CompileCommand>
-DirectoryBasedGlobalCompilationDatabase::getCompileCommand(PathRef File) const {
-  if (auto CDB = getCDBForFile(File)) {
+DirectoryBasedGlobalCompilationDatabase::getCompileCommand(
+    PathRef File, ProjectInfo *Project) const {
+  if (auto CDB = getCDBForFile(File, Project)) {
     auto Candidates = CDB->getCompileCommands(File);
-    if (!Candidates.empty())
+    if (!Candidates.empty()) {
       return std::move(Candidates.front());
+    }
   } else {
     log("Failed to find compilation database for {0}", File);
   }
@@ -63,7 +65,8 @@ DirectoryBasedGlobalCompilationDatabase:
 }
 
 tooling::CompilationDatabase *
-DirectoryBasedGlobalCompilationDatabase::getCDBForFile(PathRef File) const {
+DirectoryBasedGlobalCompilationDatabase::getCDBForFile(
+    PathRef File, ProjectInfo *Project) const {
   namespace path = sys::path;
   assert((path::is_absolute(File, path::Style::posix) ||
           path::is_absolute(File, path::Style::windows)) &&
@@ -74,10 +77,14 @@ DirectoryBasedGlobalCompilationDatabase:
   std::lock_guard<std::mutex> Lock(Mutex);
   if (CompileCommandsDir) {
     std::tie(CDB, Cached) = getCDBInDirLocked(*CompileCommandsDir);
+    if (Project && CDB)
+      Project->SourceRoot = *CompileCommandsDir;
   } else {
     for (auto Path = path::parent_path(File); !CDB && !Path.empty();
          Path = path::parent_path(Path)) {
       std::tie(CDB, Cached) = getCDBInDirLocked(Path);
+      if (Project && CDB)
+        Project->SourceRoot = Path;
     }
   }
   if (CDB && !Cached)
@@ -95,12 +102,15 @@ OverlayCDB::OverlayCDB(const GlobalCompi
 }
 
 Optional<tooling::CompileCommand>
-OverlayCDB::getCompileCommand(PathRef File) const {
+OverlayCDB::getCompileCommand(PathRef File, ProjectInfo *Project) const {
   {
     std::lock_guard<std::mutex> Lock(Mutex);
     auto It = Commands.find(File);
-    if (It != Commands.end())
+    if (It != Commands.end()) {
+      if (Project)
+        Project->SourceRoot = "";
       return It->second;
+    }
   }
   return Base ? Base->getCompileCommand(File) : None;
 }

Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h Mon Nov 26 01:51:50 2018
@@ -28,14 +28,21 @@ namespace clangd {
 
 class Logger;
 
+struct ProjectInfo {
+  // The directory in which the compilation database was discovered.
+  // Empty if directory-based compilation database discovery was not used.
+  std::string SourceRoot;
+};
+
 /// Provides compilation arguments used for parsing C and C++ files.
 class GlobalCompilationDatabase {
 public:
   virtual ~GlobalCompilationDatabase() = default;
 
   /// If there are any known-good commands for building this file, returns one.
+  /// If the ProjectInfo pointer is set, it will also be populated.
   virtual llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File) const = 0;
+  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const = 0;
 
   /// Makes a guess at how to build a file.
   /// The default implementation just runs clang on the file.
@@ -65,10 +72,11 @@ public:
   /// Scans File's parents looking for compilation databases.
   /// Any extra flags will be added.
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File) const override;
+  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
 
 private:
-  tooling::CompilationDatabase *getCDBForFile(PathRef File) const;
+  tooling::CompilationDatabase *getCDBForFile(PathRef File,
+                                              ProjectInfo *) const;
   std::pair<tooling::CompilationDatabase *, /*Cached*/ bool>
   getCDBInDirLocked(PathRef File) const;
 
@@ -93,7 +101,7 @@ public:
              std::vector<std::string> FallbackFlags = {});
 
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File) const override;
+  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
   tooling::CompileCommand getFallbackCommand(PathRef File) const override;
 
   /// Sets or clears the compilation command for a particular file.

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=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Nov 26 01:51:50 2018
@@ -36,11 +36,16 @@ namespace clangd {
 
 BackgroundIndex::BackgroundIndex(
     Context BackgroundContext, StringRef ResourceDir,
-    const FileSystemProvider &FSProvider,
+    const FileSystemProvider &FSProvider, const GlobalCompilationDatabase &CDB,
     BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize)
     : SwapIndex(make_unique<MemIndex>()), ResourceDir(ResourceDir),
-      FSProvider(FSProvider), BackgroundContext(std::move(BackgroundContext)),
-      IndexStorageFactory(std::move(IndexStorageFactory)) {
+      FSProvider(FSProvider), CDB(CDB),
+      BackgroundContext(std::move(BackgroundContext)),
+      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!");
   while (ThreadPoolSize--) {
@@ -98,47 +103,49 @@ void BackgroundIndex::blockUntilIdleForT
   QueueCV.wait(Lock, [&] { return Queue.empty() && NumActiveTasks == 0; });
 }
 
-void BackgroundIndex::enqueue(StringRef Directory,
-                              tooling::CompileCommand Cmd) {
-  BackgroundIndexStorage *IndexStorage = IndexStorageFactory(Directory);
-  {
-    std::lock_guard<std::mutex> Lock(QueueMu);
-    enqueueLocked(std::move(Cmd), IndexStorage);
+void BackgroundIndex::enqueue(const std::vector<std::string> &ChangedFiles) {
+  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]);
+  });
+}
+
+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)));
   }
-  QueueCV.notify_all();
 }
 
-void BackgroundIndex::enqueueAll(StringRef Directory,
-                                 const tooling::CompilationDatabase &CDB) {
-  trace::Span Tracer("BackgroundIndexEnqueueCDB");
-  // FIXME: this function may be slow. Perhaps enqueue a task to re-read the CDB
-  // from disk and enqueue the commands asynchronously?
-  auto Cmds = CDB.getAllCompileCommands();
-  BackgroundIndexStorage *IndexStorage = IndexStorageFactory(Directory);
-  SPAN_ATTACH(Tracer, "commands", int64_t(Cmds.size()));
-  std::mt19937 Generator(std::random_device{}());
-  std::shuffle(Cmds.begin(), Cmds.end(), Generator);
-  log("Enqueueing {0} commands for indexing from {1}", Cmds.size(), Directory);
+void BackgroundIndex::enqueueTask(Task T) {
   {
     std::lock_guard<std::mutex> Lock(QueueMu);
-    for (auto &Cmd : Cmds)
-      enqueueLocked(std::move(Cmd), IndexStorage);
+    Queue.push_back(std::move(T));
   }
   QueueCV.notify_all();
 }
 
-void BackgroundIndex::enqueueLocked(tooling::CompileCommand Cmd,
-                                    BackgroundIndexStorage *IndexStorage) {
-  Queue.push_back(Bind(
-      [this, IndexStorage](tooling::CompileCommand Cmd) {
-        std::string Filename = Cmd.Filename;
-        Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
-        if (auto Error = index(std::move(Cmd), IndexStorage))
-          log("Indexing {0} failed: {1}", Filename, std::move(Error));
-      },
-      std::move(Cmd)));
-}
-
 static BackgroundIndex::FileDigest digest(StringRef Content) {
   return SHA1::hash({(const uint8_t *)Content.data(), Content.size()});
 }

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=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Mon Nov 26 01:51:50 2018
@@ -12,6 +12,7 @@
 
 #include "Context.h"
 #include "FSProvider.h"
+#include "GlobalCompilationDatabase.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -64,17 +65,16 @@ public:
   // FIXME: resource-dir injection should be hoisted somewhere common.
   BackgroundIndex(Context BackgroundContext, llvm::StringRef ResourceDir,
                   const FileSystemProvider &,
+                  const GlobalCompilationDatabase &CDB,
                   BackgroundIndexStorage::Factory IndexStorageFactory,
                   size_t ThreadPoolSize = llvm::hardware_concurrency());
   ~BackgroundIndex(); // Blocks while the current task finishes.
 
-  // Enqueue a translation unit for indexing.
+  // Enqueue translation units for indexing.
   // The indexing happens in a background thread, so the symbols will be
   // available sometime later.
-  void enqueue(llvm::StringRef Directory, tooling::CompileCommand);
-  // Index all TUs described in the compilation database.
-  void enqueueAll(llvm::StringRef Directory,
-                  const tooling::CompilationDatabase &);
+  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.
@@ -94,6 +94,7 @@ private:
   // configuration
   std::string ResourceDir;
   const FileSystemProvider &FSProvider;
+  const GlobalCompilationDatabase &CDB;
   Context BackgroundContext;
 
   // index state
@@ -109,6 +110,7 @@ private:
   // 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;
@@ -117,6 +119,7 @@ private:
   bool ShouldStop = false;
   std::deque<Task> Queue;
   std::vector<std::thread> ThreadPool; // FIXME: Abstract this away.
+  GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
 };
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp Mon Nov 26 01:51:50 2018
@@ -81,6 +81,23 @@ public:
   }
 };
 
+// Doesn't persist index shards anywhere (used when the CDB dir is unknown).
+// We could consider indexing into ~/.clangd/ or so instead.
+class NullStorage : public BackgroundIndexStorage {
+public:
+  std::unique_ptr<IndexFileIn>
+  loadShard(llvm::StringRef ShardIdentifier) const override {
+    return nullptr;
+  }
+
+  llvm::Error storeShard(llvm::StringRef ShardIdentifier,
+                         IndexFileOut Shard) const override {
+    vlog("Couldn't find project for {0}, indexing in-memory only",
+         ShardIdentifier);
+    return llvm::Error::success();
+  }
+};
+
 // Creates and owns IndexStorages for multiple CDBs.
 class DiskBackedIndexStorageManager {
 public:
@@ -89,7 +106,7 @@ public:
     std::lock_guard<std::mutex> Lock(*IndexStorageMapMu);
     auto &IndexStorage = IndexStorageMap[CDBDirectory];
     if (!IndexStorage)
-      IndexStorage = llvm::make_unique<DiskBackedIndexStorage>(CDBDirectory);
+      IndexStorage = create(CDBDirectory);
     return IndexStorage.get();
   }
 
@@ -97,6 +114,12 @@ public:
   BackgroundIndexStorage *createStorage(llvm::StringRef CDBDirectory);
 
 private:
+  std::unique_ptr<BackgroundIndexStorage> create(llvm::StringRef CDBDirectory) {
+    if (CDBDirectory.empty())
+      return llvm::make_unique<NullStorage>();
+    return llvm::make_unique<DiskBackedIndexStorage>(CDBDirectory);
+  }
+
   llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>> IndexStorageMap;
   std::unique_ptr<std::mutex> IndexStorageMapMu;
 };

Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Mon Nov 26 01:51:50 2018
@@ -80,14 +80,15 @@ TEST(BackgroundIndexTest, IndexTwoFiles)
   llvm::StringMap<std::string> Storage;
   size_t CacheHits = 0;
   MemoryShardStorage MSS(Storage, CacheHits);
-  BackgroundIndex Idx(Context::empty(), "", FS,
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(Context::empty(), "", FS, CDB,
                       [&](llvm::StringRef) { return &MSS; });
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
   Cmd.Directory = testPath("root");
   Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
-  Idx.enqueue(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root"), Cmd);
 
   Idx.blockUntilIdleForTest();
   EXPECT_THAT(
@@ -97,7 +98,7 @@ TEST(BackgroundIndexTest, IndexTwoFiles)
 
   Cmd.Filename = testPath("root/B.cc");
   Cmd.CommandLine = {"clang++", Cmd.Filename};
-  Idx.enqueue(testPath("root"), Cmd);
+  CDB.setCompileCommand(testPath("root"), Cmd);
 
   Idx.blockUntilIdleForTest();
   // B_CC is dropped as we don't collect symbols from A.h in this compilation.
@@ -136,9 +137,10 @@ TEST(BackgroundIndexTest, ShardStorageWr
   Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
   // Check nothing is loaded from Storage, but A.cc and A.h has been stored.
   {
-    BackgroundIndex Idx(Context::empty(), "", FS,
+    OverlayCDB CDB(/*Base=*/nullptr);
+    BackgroundIndex Idx(Context::empty(), "", FS, CDB,
                         [&](llvm::StringRef) { return &MSS; });
-    Idx.enqueue(testPath("root"), Cmd);
+    CDB.setCompileCommand(testPath("root"), Cmd);
     Idx.blockUntilIdleForTest();
   }
   EXPECT_EQ(CacheHits, 0U);

Modified: clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp Mon Nov 26 01:51:50 2018
@@ -41,9 +41,12 @@ class OverlayCDBTest : public ::testing:
   class BaseCDB : public GlobalCompilationDatabase {
   public:
     Optional<tooling::CompileCommand>
-    getCompileCommand(StringRef File) const override {
-      if (File == testPath("foo.cc"))
+    getCompileCommand(StringRef File, ProjectInfo *Project) const override {
+      if (File == testPath("foo.cc")) {
+        if (Project)
+          Project->SourceRoot = testRoot();
         return cmd(File, "-DA=1");
+      }
       return None;
     }
 

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Mon Nov 26 01:51:50 2018
@@ -39,7 +39,8 @@ MockCompilationDatabase::MockCompilation
 }
 
 Optional<tooling::CompileCommand>
-MockCompilationDatabase::getCompileCommand(PathRef File) const {
+MockCompilationDatabase::getCompileCommand(PathRef File,
+                                           ProjectInfo *Project) const {
   if (ExtraClangFlags.empty())
     return None;
 
@@ -58,6 +59,8 @@ MockCompilationDatabase::getCompileComma
     CommandLine.push_back(RelativeFilePath.str());
   }
 
+  if (Project)
+    Project->SourceRoot = Directory;
   return {tooling::CompileCommand(
       Directory != StringRef() ? Directory : sys::path::parent_path(File),
       FileName, std::move(CommandLine), "")};

Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.h?rev=347538&r1=347537&r2=347538&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TestFS.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.h Mon Nov 26 01:51:50 2018
@@ -41,7 +41,7 @@ public:
 class MockCompilationDatabase : public GlobalCompilationDatabase {
 public:
   /// If \p Directory is not empty, use that as the Directory field of the
-  /// CompileCommand.
+  /// CompileCommand, and as project SourceRoot.
   ///
   /// If \p RelPathPrefix is not empty, use that as a prefix in front of the
   /// source file name, instead of using an absolute path.
@@ -49,7 +49,7 @@ public:
                           StringRef RelPathPrefix = StringRef());
 
   llvm::Optional<tooling::CompileCommand>
-  getCompileCommand(PathRef File) const override;
+  getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override;
 
   std::vector<std::string> ExtraClangFlags;
 




More information about the cfe-commits mailing list