From: Sam McCall Date: Mon, 26 Nov 2018 09:51:50 +0000 (+0000) Subject: [clangd] Auto-index watches global CDB for changes. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6e2d2a33b6be376e52688705db22534b336f5529;p=platform%2Fupstream%2Fllvm.git [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 llvm-svn: 347538 --- diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index a21ea52..d2a10ba 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -38,11 +38,13 @@ DirectoryBasedGlobalCompilationDatabase:: ~DirectoryBasedGlobalCompilationDatabase() = default; Optional -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::getCDBInDirLocked(PathRef Dir) const { } 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::getCDBForFile(PathRef File) const { std::lock_guard 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 GlobalCompilationDatabase *Base, } Optional -OverlayCDB::getCompileCommand(PathRef File) const { +OverlayCDB::getCompileCommand(PathRef File, ProjectInfo *Project) const { { std::lock_guard 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; } diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h index 84b1742..181b178 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -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 - 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 - 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 getCDBInDirLocked(PathRef File) const; @@ -93,7 +101,7 @@ public: std::vector FallbackFlags = {}); llvm::Optional - 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. diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp index 4410391..1f850eb 100644 --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -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()), 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 &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::blockUntilIdleForTest() { QueueCV.wait(Lock, [&] { return Queue.empty() && NumActiveTasks == 0; }); } -void BackgroundIndex::enqueue(StringRef Directory, - tooling::CompileCommand Cmd) { - BackgroundIndexStorage *IndexStorage = IndexStorageFactory(Directory); - { - std::lock_guard Lock(QueueMu); - enqueueLocked(std::move(Cmd), IndexStorage); +void BackgroundIndex::enqueue(const std::vector &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 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 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()}); } diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h index 01b0ca7..eb9ce3e 100644 --- a/clang-tools-extra/clangd/index/Background.h +++ b/clang-tools-extra/clangd/index/Background.h @@ -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 &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 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 Queue; std::vector ThreadPool; // FIXME: Abstract this away. + GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp b/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp index ee25570..64512e8 100644 --- a/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp +++ b/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp @@ -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 + 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 Lock(*IndexStorageMapMu); auto &IndexStorage = IndexStorageMap[CDBDirectory]; if (!IndexStorage) - IndexStorage = llvm::make_unique(CDBDirectory); + IndexStorage = create(CDBDirectory); return IndexStorage.get(); } @@ -97,6 +114,12 @@ public: BackgroundIndexStorage *createStorage(llvm::StringRef CDBDirectory); private: + std::unique_ptr create(llvm::StringRef CDBDirectory) { + if (CDBDirectory.empty()) + return llvm::make_unique(); + return llvm::make_unique(CDBDirectory); + } + llvm::StringMap> IndexStorageMap; std::unique_ptr IndexStorageMapMu; }; diff --git a/clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp b/clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp index 577f99d..493d7c2 100644 --- a/clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp +++ b/clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp @@ -80,14 +80,15 @@ TEST(BackgroundIndexTest, IndexTwoFiles) { llvm::StringMap 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, ShardStorageWriteTest) { 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); diff --git a/clang-tools-extra/unittests/clangd/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/unittests/clangd/GlobalCompilationDatabaseTests.cpp index f502f79..f3916108 100644 --- a/clang-tools-extra/unittests/clangd/GlobalCompilationDatabaseTests.cpp +++ b/clang-tools-extra/unittests/clangd/GlobalCompilationDatabaseTests.cpp @@ -41,9 +41,12 @@ class OverlayCDBTest : public ::testing::Test { class BaseCDB : public GlobalCompilationDatabase { public: Optional - 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; } diff --git a/clang-tools-extra/unittests/clangd/TestFS.cpp b/clang-tools-extra/unittests/clangd/TestFS.cpp index 86e1072..b0938a5 100644 --- a/clang-tools-extra/unittests/clangd/TestFS.cpp +++ b/clang-tools-extra/unittests/clangd/TestFS.cpp @@ -39,7 +39,8 @@ MockCompilationDatabase::MockCompilationDatabase(StringRef Directory, } Optional -MockCompilationDatabase::getCompileCommand(PathRef File) const { +MockCompilationDatabase::getCompileCommand(PathRef File, + ProjectInfo *Project) const { if (ExtraClangFlags.empty()) return None; @@ -58,6 +59,8 @@ MockCompilationDatabase::getCompileCommand(PathRef File) const { 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), "")}; diff --git a/clang-tools-extra/unittests/clangd/TestFS.h b/clang-tools-extra/unittests/clangd/TestFS.h index 56bf8a3..0226fc3 100644 --- a/clang-tools-extra/unittests/clangd/TestFS.h +++ b/clang-tools-extra/unittests/clangd/TestFS.h @@ -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 - getCompileCommand(PathRef File) const override; + getCompileCommand(PathRef File, ProjectInfo * = nullptr) const override; std::vector ExtraClangFlags;