kadircet updated this revision to Diff 210621.
kadircet marked 3 inline comments as done.
kadircet added a comment.
- Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64745/new/
https://reviews.llvm.org/D64745
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
clang-tools-extra/clangd/test/Inputs/background-index/definition.jsonrpc
clang-tools-extra/clangd/test/Inputs/background-index/foo.cpp
clang-tools-extra/clangd/test/Inputs/background-index/foo.h
clang-tools-extra/clangd/test/Inputs/background-index/sub_dir/compile_flags.txt
clang-tools-extra/clangd/test/Inputs/background-index/sub_dir/foo.h
clang-tools-extra/clangd/test/background-index.test
Index: clang-tools-extra/clangd/test/background-index.test
===================================================================
--- clang-tools-extra/clangd/test/background-index.test
+++ clang-tools-extra/clangd/test/background-index.test
@@ -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
Index: clang-tools-extra/clangd/test/Inputs/background-index/foo.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/background-index/foo.h
@@ -1,4 +0,0 @@
-#ifndef FOO_H
-#define FOO_H
-int foo();
-#endif
Index: clang-tools-extra/clangd/test/Inputs/background-index/foo.cpp
===================================================================
--- clang-tools-extra/clangd/test/Inputs/background-index/foo.cpp
+++ clang-tools-extra/clangd/test/Inputs/background-index/foo.cpp
@@ -1,2 +1,2 @@
-#include "foo.h"
+#include "sub_dir/foo.h"
int foo() { return 42; }
Index: clang-tools-extra/clangd/test/Inputs/background-index/definition.jsonrpc
===================================================================
--- clang-tools-extra/clangd/test/Inputs/background-index/definition.jsonrpc
+++ clang-tools-extra/clangd/test/Inputs/background-index/definition.jsonrpc
@@ -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}"
}
}
}
Index: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
===================================================================
--- clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
+++ clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
@@ -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 @@
// 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 @@
}
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
Index: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
===================================================================
--- clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
+++ clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
@@ -38,8 +38,10 @@
/// 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 @@
/// 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 @@
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 @@
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 @@
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 @@
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();
}
Index: clang-tools-extra/clangd/index/Background.h
===================================================================
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -50,15 +50,17 @@
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 @@
/// 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 @@
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 {
Index: clang-tools-extra/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -181,8 +181,8 @@
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 @@
}
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::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 @@
// 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 @@
}
}
-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 @@
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 @@
// 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 @@
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;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -127,7 +127,8 @@
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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
