Author: kadircet Date: Thu Jul 18 10:20:41 2019 New Revision: 366467 URL: http://llvm.org/viewvc/llvm-project?rev=366467&view=rev Log: [clangd] BackgroundIndex stores shards to the closest project
Summary: Changes persistance logic to store shards at the directory of closest CDB. Previously we were storing all shards to directory of the CDB that triggered indexing, it had its downsides. For example, if you had two TUs coming from a different CDB but depending on the same header foo.h, we will store the foo.h only for the first CDB, and it would be missing for the second and we would never persist it since it was actually present in the memory and persisted before. This patch still stores only a single copy of a shard, but makes the directory a function of the file name. So that the shard place will be unique even with multiple CDBs accessing the file. This directory is determined as the first directory containing a CDB in the file's parent directories, if no such directory exists we make use of the home directory. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D64745 Added: clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/ clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/compile_flags.txt clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/foo.h Removed: clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.h Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/index/Background.cpp clang-tools-extra/trunk/clangd/index/Background.h clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp clang-tools-extra/trunk/clangd/test/Inputs/background-index/definition.jsonrpc clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.cpp clang-tools-extra/trunk/clangd/test/background-index.test Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Jul 18 10:20:41 2019 @@ -127,7 +127,8 @@ ClangdServer::ClangdServer(const GlobalC if (Opts.BackgroundIndex) { BackgroundIdx = llvm::make_unique<BackgroundIndex>( Context::current().clone(), FSProvider, CDB, - BackgroundIndexStorage::createDiskBackedStorageFactory()); + BackgroundIndexStorage::createDiskBackedStorageFactory( + [&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); })); AddIndex(BackgroundIdx.get()); } if (DynamicIdx) 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=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Jul 18 10:20:41 2019 @@ -181,8 +181,8 @@ BackgroundQueue::Task BackgroundIndex::c std::mt19937(std::random_device{}())); std::vector<BackgroundQueue::Task> Tasks; Tasks.reserve(NeedsReIndexing.size()); - for (auto &Elem : NeedsReIndexing) - Tasks.push_back(indexFileTask(std::move(Elem.first), Elem.second)); + for (auto &Cmd : NeedsReIndexing) + Tasks.push_back(indexFileTask(std::move(Cmd))); Queue.append(std::move(Tasks)); }); @@ -197,13 +197,12 @@ static llvm::StringRef filenameWithoutEx } BackgroundQueue::Task -BackgroundIndex::indexFileTask(tooling::CompileCommand Cmd, - BackgroundIndexStorage *Storage) { - BackgroundQueue::Task T([this, Storage, Cmd] { +BackgroundIndex::indexFileTask(tooling::CompileCommand Cmd) { + BackgroundQueue::Task T([this, Cmd] { // We can't use llvm::StringRef here since we are going to // move from Cmd during the call below. const std::string FileName = Cmd.Filename; - if (auto Error = index(std::move(Cmd), Storage)) + if (auto Error = index(std::move(Cmd))) elog("Indexing {0} failed: {1}", FileName, std::move(Error)); }); T.QueuePri = IndexFile; @@ -226,7 +225,7 @@ void BackgroundIndex::boostRelated(llvm: void BackgroundIndex::update( llvm::StringRef MainFile, IndexFileIn Index, const llvm::StringMap<ShardVersion> &ShardVersionsSnapshot, - BackgroundIndexStorage *IndexStorage, bool HadErrors) { + bool HadErrors) { // Partition symbols/references into files. struct File { llvm::DenseSet<const Symbol *> Symbols; @@ -310,22 +309,21 @@ void BackgroundIndex::update( // We need to store shards before updating the index, since the latter // consumes slabs. // FIXME: Also skip serializing the shard if it is already up-to-date. - if (IndexStorage) { - IndexFileOut Shard; - Shard.Symbols = SS.get(); - Shard.Refs = RS.get(); - Shard.Relations = RelS.get(); - Shard.Sources = IG.get(); - - // Only store command line hash for main files of the TU, since our - // current model keeps only one version of a header file. - if (Path == MainFile) - Shard.Cmd = Index.Cmd.getPointer(); - - if (auto Error = IndexStorage->storeShard(Path, Shard)) - elog("Failed to write background-index shard for file {0}: {1}", Path, - std::move(Error)); - } + BackgroundIndexStorage *IndexStorage = IndexStorageFactory(Path); + IndexFileOut Shard; + Shard.Symbols = SS.get(); + Shard.Refs = RS.get(); + Shard.Relations = RelS.get(); + Shard.Sources = IG.get(); + + // Only store command line hash for main files of the TU, since our + // current model keeps only one version of a header file. + if (Path == MainFile) + Shard.Cmd = Index.Cmd.getPointer(); + + if (auto Error = IndexStorage->storeShard(Path, Shard)) + elog("Failed to write background-index shard for file {0}: {1}", Path, + std::move(Error)); { std::lock_guard<std::mutex> Lock(ShardVersionsMu); @@ -348,8 +346,7 @@ void BackgroundIndex::update( } } -llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd, - BackgroundIndexStorage *IndexStorage) { +llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) { trace::Span Tracer("BackgroundIndex"); SPAN_ATTACH(Tracer, "file", Cmd.Filename); auto AbsolutePath = getAbsolutePath(Cmd); @@ -443,8 +440,7 @@ llvm::Error BackgroundIndex::index(tooli for (auto &It : *Index.Sources) It.second.Flags |= IncludeGraphNode::SourceFlag::HadErrors; } - update(AbsolutePath, std::move(Index), ShardVersionsSnapshot, IndexStorage, - HadErrors); + update(AbsolutePath, std::move(Index), ShardVersionsSnapshot, HadErrors); Rebuilder.indexedTU(); return llvm::Error::success(); @@ -453,10 +449,9 @@ llvm::Error BackgroundIndex::index(tooli // Restores shards for \p MainFiles from index storage. Then checks staleness of // those shards and returns a list of TUs that needs to be indexed to update // staleness. -std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>> +std::vector<tooling::CompileCommand> BackgroundIndex::loadProject(std::vector<std::string> MainFiles) { - std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>> - NeedsReIndexing; + std::vector<tooling::CompileCommand> NeedsReIndexing; Rebuilder.startLoading(); // Load shards for all of the mainfiles. @@ -517,8 +512,7 @@ BackgroundIndex::loadProject(std::vector std::string ProjectRoot; if (auto PI = CDB.getProjectInfo(TU)) ProjectRoot = std::move(PI->SourceRoot); - BackgroundIndexStorage *Storage = IndexStorageFactory(ProjectRoot); - NeedsReIndexing.emplace_back(std::move(*Cmd), Storage); + NeedsReIndexing.emplace_back(std::move(*Cmd)); } 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=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Background.h (original) +++ clang-tools-extra/trunk/clangd/index/Background.h Thu Jul 18 10:20:41 2019 @@ -50,15 +50,17 @@ public: virtual std::unique_ptr<IndexFileIn> loadShard(llvm::StringRef ShardIdentifier) const = 0; - // The factory provides storage for each CDB. + // The factory provides storage for each File. // It keeps ownership of the storage instances, and should manage caching // itself. Factory must be threadsafe and never returns nullptr. - using Factory = - llvm::unique_function<BackgroundIndexStorage *(llvm::StringRef)>; + using Factory = llvm::unique_function<BackgroundIndexStorage *(PathRef)>; // Creates an Index Storage that saves shards into disk. Index storage uses - // CDBDirectory + ".clangd/index/" as the folder to save shards. - static Factory createDiskBackedStorageFactory(); + // CDBDirectory + ".clangd/index/" as the folder to save shards. CDBDirectory + // is the first directory containing a CDB in parent directories of a file, or + // user's home directory if none was found, e.g. standard library headers. + static Factory createDiskBackedStorageFactory( + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo); }; // A priority queue of tasks which can be run on (external) worker threads. @@ -158,15 +160,14 @@ private: /// information on IndexStorage. void update(llvm::StringRef MainFile, IndexFileIn Index, const llvm::StringMap<ShardVersion> &ShardVersionsSnapshot, - BackgroundIndexStorage *IndexStorage, bool HadErrors); + bool HadErrors); // configuration const FileSystemProvider &FSProvider; const GlobalCompilationDatabase &CDB; Context BackgroundContext; - llvm::Error index(tooling::CompileCommand, - BackgroundIndexStorage *IndexStorage); + llvm::Error index(tooling::CompileCommand); FileSymbols IndexedSymbols; BackgroundIndexRebuilder Rebuilder; @@ -175,13 +176,12 @@ private: BackgroundIndexStorage::Factory IndexStorageFactory; // Tries to load shards for the MainFiles and their dependencies. - std::vector<std::pair<tooling::CompileCommand, BackgroundIndexStorage *>> + std::vector<tooling::CompileCommand> loadProject(std::vector<std::string> MainFiles); BackgroundQueue::Task changedFilesTask(const std::vector<std::string> &ChangedFiles); - BackgroundQueue::Task indexFileTask(tooling::CompileCommand Cmd, - BackgroundIndexStorage *Storage); + BackgroundQueue::Task indexFileTask(tooling::CompileCommand Cmd); // from lowest to highest priority enum QueuePriority { Modified: clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp?rev=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp (original) +++ clang-tools-extra/trunk/clangd/index/BackgroundIndexLoader.cpp Thu Jul 18 10:20:41 2019 @@ -38,8 +38,10 @@ llvm::Optional<Path> uriToAbsolutePath(l /// inverse dependency mapping. class BackgroundIndexLoader { public: + BackgroundIndexLoader(BackgroundIndexStorage::Factory &IndexStorageFactory) + : IndexStorageFactory(IndexStorageFactory) {} /// Load the shards for \p MainFile and all of its dependencies. - void load(PathRef MainFile, BackgroundIndexStorage *Storage); + void load(PathRef MainFile); /// Consumes the loader and returns all shards. std::vector<LoadedShard> takeResult() &&; @@ -49,18 +51,19 @@ private: /// Storage. Also returns paths for dependencies of \p StartSourceFile if it /// wasn't cached yet. std::pair<const LoadedShard &, std::vector<Path>> - loadShard(PathRef StartSourceFile, BackgroundIndexStorage *Storage); + loadShard(PathRef StartSourceFile); /// Cache for Storage lookups. llvm::StringMap<LoadedShard> LoadedShards; /// References are into the AbsolutePaths in LoadedShards. llvm::DenseMap<PathRef, PathRef> FileToTU; + + BackgroundIndexStorage::Factory &IndexStorageFactory; }; std::pair<const LoadedShard &, std::vector<Path>> -BackgroundIndexLoader::loadShard(PathRef StartSourceFile, - BackgroundIndexStorage *Storage) { +BackgroundIndexLoader::loadShard(PathRef StartSourceFile) { auto It = LoadedShards.try_emplace(StartSourceFile); LoadedShard &LS = It.first->getValue(); std::vector<Path> Edges = {}; @@ -69,6 +72,7 @@ BackgroundIndexLoader::loadShard(PathRef return {LS, Edges}; LS.AbsolutePath = StartSourceFile.str(); + BackgroundIndexStorage *Storage = IndexStorageFactory(LS.AbsolutePath); auto Shard = Storage->loadShard(StartSourceFile); if (!Shard || !Shard->Sources) { vlog("Failed to load shard: {0}", StartSourceFile); @@ -96,8 +100,7 @@ BackgroundIndexLoader::loadShard(PathRef return {LS, Edges}; } -void BackgroundIndexLoader::load(PathRef MainFile, - BackgroundIndexStorage *Storage) { +void BackgroundIndexLoader::load(PathRef MainFile) { llvm::StringSet<> InQueue; // Following containers points to strings inside InQueue. std::queue<PathRef> ToVisit; @@ -108,7 +111,7 @@ void BackgroundIndexLoader::load(PathRef PathRef SourceFile = ToVisit.front(); ToVisit.pop(); - auto ShardAndEdges = loadShard(SourceFile, Storage); + auto ShardAndEdges = loadShard(SourceFile); FileToTU[ShardAndEdges.first.AbsolutePath] = MainFile; for (PathRef Edge : ShardAndEdges.second) { auto It = InQueue.insert(Edge); @@ -136,15 +139,10 @@ std::vector<LoadedShard> loadIndexShards(llvm::ArrayRef<Path> MainFiles, BackgroundIndexStorage::Factory &IndexStorageFactory, const GlobalCompilationDatabase &CDB) { - BackgroundIndexLoader Loader; + BackgroundIndexLoader Loader(IndexStorageFactory); for (llvm::StringRef MainFile : MainFiles) { assert(llvm::sys::path::is_absolute(MainFile)); - - std::string ProjectRoot; - if (auto PI = CDB.getProjectInfo(MainFile)) - ProjectRoot = std::move(PI->SourceRoot); - BackgroundIndexStorage *Storage = IndexStorageFactory(ProjectRoot); - Loader.load(MainFile, Storage); + Loader.load(MainFile); } return std::move(Loader).takeResult(); } 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=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp (original) +++ clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp Thu Jul 18 10:20:41 2019 @@ -6,13 +6,21 @@ // //===----------------------------------------------------------------------===// +#include "GlobalCompilationDatabase.h" #include "Logger.h" +#include "Path.h" #include "index/Background.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" +#include <functional> namespace clang { namespace clangd { @@ -118,12 +126,21 @@ public: // Creates and owns IndexStorages for multiple CDBs. class DiskBackedIndexStorageManager { public: - DiskBackedIndexStorageManager() - : IndexStorageMapMu(llvm::make_unique<std::mutex>()) {} + DiskBackedIndexStorageManager( + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo) + : IndexStorageMapMu(llvm::make_unique<std::mutex>()), + GetProjectInfo(std::move(GetProjectInfo)) { + llvm::SmallString<128> HomeDir; + llvm::sys::path::home_directory(HomeDir); + this->HomeDir = HomeDir.str().str(); + } - // Creates or fetches to storage from cache for the specified CDB. - BackgroundIndexStorage *operator()(llvm::StringRef CDBDirectory) { + // Creates or fetches to storage from cache for the specified project. + BackgroundIndexStorage *operator()(PathRef File) { std::lock_guard<std::mutex> Lock(*IndexStorageMapMu); + Path CDBDirectory = HomeDir; + if (auto PI = GetProjectInfo(File)) + CDBDirectory = PI->SourceRoot; auto &IndexStorage = IndexStorageMap[CDBDirectory]; if (!IndexStorage) IndexStorage = create(CDBDirectory); @@ -131,21 +148,28 @@ public: } private: - std::unique_ptr<BackgroundIndexStorage> create(llvm::StringRef CDBDirectory) { - if (CDBDirectory.empty()) + std::unique_ptr<BackgroundIndexStorage> create(PathRef CDBDirectory) { + if (CDBDirectory.empty()) { + elog("Tried to create storage for empty directory!"); return llvm::make_unique<NullStorage>(); + } return llvm::make_unique<DiskBackedIndexStorage>(CDBDirectory); } + Path HomeDir; + llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>> IndexStorageMap; std::unique_ptr<std::mutex> IndexStorageMapMu; + + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo; }; } // namespace BackgroundIndexStorage::Factory -BackgroundIndexStorage::createDiskBackedStorageFactory() { - return DiskBackedIndexStorageManager(); +BackgroundIndexStorage::createDiskBackedStorageFactory( + std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo) { + return DiskBackedIndexStorageManager(std::move(GetProjectInfo)); } } // namespace clangd Modified: clang-tools-extra/trunk/clangd/test/Inputs/background-index/definition.jsonrpc URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/Inputs/background-index/definition.jsonrpc?rev=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/test/Inputs/background-index/definition.jsonrpc (original) +++ clang-tools-extra/trunk/clangd/test/Inputs/background-index/definition.jsonrpc Thu Jul 18 10:20:41 2019 @@ -18,7 +18,7 @@ "uri": "file://DIRECTORY/bar.cpp", "languageId": "cpp", "version": 1, - "text": "#include \"foo.h\"\nint main(){\nreturn foo();\n}" + "text": "#include \"sub_dir/foo.h\"\nint main(){\nreturn foo();\n}" } } } Modified: clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.cpp?rev=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.cpp (original) +++ clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.cpp Thu Jul 18 10:20:41 2019 @@ -1,2 +1,2 @@ -#include "foo.h" +#include "sub_dir/foo.h" int foo() { return 42; } Removed: clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.h?rev=366466&view=auto ============================================================================== --- clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.h (original) +++ clang-tools-extra/trunk/clangd/test/Inputs/background-index/foo.h (removed) @@ -1,4 +0,0 @@ -#ifndef FOO_H -#define FOO_H -int foo(); -#endif Added: clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/compile_flags.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/compile_flags.txt?rev=366467&view=auto ============================================================================== (empty) Added: clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/foo.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/foo.h?rev=366467&view=auto ============================================================================== --- clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/foo.h (added) +++ clang-tools-extra/trunk/clangd/test/Inputs/background-index/sub_dir/foo.h Thu Jul 18 10:20:41 2019 @@ -0,0 +1,4 @@ +#ifndef FOO_H +#define FOO_H +int foo(); +#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=366467&r1=366466&r2=366467&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/test/background-index.test (original) +++ clang-tools-extra/trunk/clangd/test/background-index.test Thu Jul 18 10:20:41 2019 @@ -5,7 +5,8 @@ # RUN: rm -rf %t # RUN: cp -r %S/Inputs/background-index %t # Need to embed the correct temp path in the actual JSON-RPC requests. -# RUN: sed -i -e "s|DIRECTORY|%t|" %t/* +# RUN: sed -i -e "s|DIRECTORY|%t|" %t/definition.jsonrpc +# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json # We're editing bar.cpp, which includes foo.h. # foo() is declared in foo.h and defined in foo.cpp. @@ -14,6 +15,7 @@ # Test that the index is writing files in the expected location. # RUN: ls %t/.clangd/index/foo.cpp.*.idx +# RUN: ls %t/sub_dir/.clangd/index/foo.h.*.idx # Test the index is read from disk: delete code and restart clangd. # RUN: rm %t/foo.cpp _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
