[clang-tools-extra] r347567 - [clangd] Enable auto-index behind a flag.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 26 08:00:12 PST 2018
Author: sammccall
Date: Mon Nov 26 08:00:11 2018
New Revision: 347567
URL: http://llvm.org/viewvc/llvm-project?rev=347567&view=rev
Log:
[clangd] Enable auto-index behind a flag.
Summary:
Ownership and configuration:
The auto-index (background index) is maintained by ClangdServer, like Dynamic.
(This means ClangdServer will be able to enqueue preamble indexing in future).
For now it's enabled by a simple boolean flag in ClangdServer::Options, but
we probably want to eventually allow injecting the storage strategy.
New 'sync' command:
In order to meaningfully test the integration (not just unit-test components)
we need a way for tests to ensure the asynchronous index reads/writes occur
before a certain point.
Because these tests and assertions are few, I think exposing an explicit "sync"
command for use in tests is simpler than allowing threading to be completely
disabled in the background index (as we do for TUScheduler).
Bugs:
I fixed a couple of trivial bugs I found while testing, but there's one I can't.
JSONCompilationDatabase::getAllFiles() may return relative paths, and currently
we trigger an assertion that assumes they are absolute.
There's no efficient way to resolve them (you have to retrieve the corresponding
command and then resolve against its directory property). In general I think
this behavior is broken and we should fix it in JSONCompilationDatabase and
require CompilationDatabase::getAllFiles() to be absolute.
Reviewers: kadircet
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D54894
Added:
clang-tools-extra/trunk/test/clangd/Inputs/background-index/
clang-tools-extra/trunk/test/clangd/Inputs/background-index/compile_commands.json
clang-tools-extra/trunk/test/clangd/Inputs/background-index/definition.jsonrpc
clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.cpp
clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.h
clang-tools-extra/trunk/test/clangd/background-index.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
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/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Nov 26 08:00:11 2018
@@ -337,6 +337,17 @@ void ClangdLSPServer::onShutdown(const S
Reply(nullptr);
}
+// sync is a clangd extension: it blocks until all background work completes.
+// It blocks the calling thread, so no messages are processed until it returns!
+void ClangdLSPServer::onSync(const NoParams &Params,
+ Callback<std::nullptr_t> Reply) {
+ if (Server->blockUntilIdleForTest(/*TimeoutSeconds=*/60))
+ Reply(nullptr);
+ else
+ Reply(createStringError(llvm::inconvertibleErrorCode(),
+ "Not idle after a minute"));
+}
+
void ClangdLSPServer::onDocumentDidOpen(
const DidOpenTextDocumentParams &Params) {
PathRef File = Params.textDocument.uri.file();
@@ -701,6 +712,7 @@ ClangdLSPServer::ClangdLSPServer(class T
// clang-format off
MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
MsgHandler->bind("shutdown", &ClangdLSPServer::onShutdown);
+ MsgHandler->bind("sync", &ClangdLSPServer::onSync);
MsgHandler->bind("textDocument/rangeFormatting", &ClangdLSPServer::onDocumentRangeFormatting);
MsgHandler->bind("textDocument/onTypeFormatting", &ClangdLSPServer::onDocumentOnTypeFormatting);
MsgHandler->bind("textDocument/formatting", &ClangdLSPServer::onDocumentFormatting);
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Mon Nov 26 08:00:11 2018
@@ -57,6 +57,7 @@ private:
// Calls have signature void(const Params&, Callback<Response>).
void onInitialize(const InitializeParams &, Callback<llvm::json::Value>);
void onShutdown(const ShutdownParams &, Callback<std::nullptr_t>);
+ void onSync(const NoParams &, Callback<std::nullptr_t>);
void onDocumentDidOpen(const DidOpenTextDocumentParams &);
void onDocumentDidChange(const DidChangeTextDocumentParams &);
void onDocumentDidClose(const DidCloseTextDocumentParams &);
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Nov 26 08:00:11 2018
@@ -121,16 +121,25 @@ ClangdServer::ClangdServer(const GlobalC
llvm::make_unique<UpdateIndexCallbacks>(DynamicIdx.get(),
DiagConsumer),
Opts.UpdateDebounce, Opts.RetentionPolicy) {
- if (DynamicIdx && Opts.StaticIndex) {
- MergedIdx =
- llvm::make_unique<MergedIndex>(DynamicIdx.get(), Opts.StaticIndex);
- Index = MergedIdx.get();
- } else if (DynamicIdx)
- Index = DynamicIdx.get();
- else if (Opts.StaticIndex)
- Index = Opts.StaticIndex;
- else
- Index = nullptr;
+ // Adds an index to the stack, at higher priority than existing indexes.
+ auto AddIndex = [&](SymbolIndex *Idx) {
+ if (this->Index != nullptr) {
+ MergedIdx.push_back(llvm::make_unique<MergedIndex>(Idx, this->Index));
+ this->Index = MergedIdx.back().get();
+ } else {
+ this->Index = Idx;
+ }
+ };
+ if (Opts.StaticIndex)
+ AddIndex(Opts.StaticIndex);
+ if (Opts.BackgroundIndex) {
+ BackgroundIdx = llvm::make_unique<BackgroundIndex>(
+ Context::current().clone(), ResourceDir, FSProvider, CDB,
+ BackgroundIndexStorage::createDiskBackedStorageFactory());
+ AddIndex(BackgroundIdx.get());
+ }
+ if (DynamicIdx)
+ AddIndex(DynamicIdx.get());
}
void ClangdServer::addDocument(PathRef File, StringRef Contents,
@@ -501,7 +510,9 @@ ClangdServer::getUsedBytesPerFile() cons
LLVM_NODISCARD bool
ClangdServer::blockUntilIdleForTest(Optional<double> TimeoutSeconds) {
- return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds));
+ return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds)) &&
+ (!BackgroundIdx ||
+ BackgroundIdx->blockUntilIdleForTest(TimeoutSeconds));
}
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Nov 26 08:00:11 2018
@@ -18,6 +18,7 @@
#include "GlobalCompilationDatabase.h"
#include "Protocol.h"
#include "TUScheduler.h"
+#include "index/Background.h"
#include "index/FileIndex.h"
#include "index/Index.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -78,6 +79,9 @@ public:
/// Use a heavier and faster in-memory index implementation.
/// FIXME: we should make this true if it isn't too slow to build!.
bool HeavyweightDynamicSymbolIndex = false;
+ /// If true, ClangdServer automatically indexes files in the current project
+ /// on background threads. The index is stored in the project root.
+ bool BackgroundIndex = false;
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
@@ -234,11 +238,13 @@ private:
// - the dynamic index owned by ClangdServer (DynamicIdx)
// - the static index passed to the constructor
// - a merged view of a static and dynamic index (MergedIndex)
- const SymbolIndex *Index;
+ const SymbolIndex *Index = nullptr;
// If present, an index of symbols in open files. Read via *Index.
std::unique_ptr<FileIndex> DynamicIdx;
- // If present, storage for the merged static/dynamic index. Read via *Index.
- std::unique_ptr<SymbolIndex> MergedIdx;
+ // If present, the new "auto-index" maintained in background threads.
+ std::unique_ptr<BackgroundIndex> BackgroundIdx;
+ // Storage for merged views of the various indexes.
+ std::vector<std::unique_ptr<SymbolIndex>> MergedIdx;
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
llvm::StringMap<llvm::Optional<FuzzyFindRequest>>
Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
+++ clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp Mon Nov 26 08:00:11 2018
@@ -87,6 +87,8 @@ DirectoryBasedGlobalCompilationDatabase:
Project->SourceRoot = Path;
}
}
+ // FIXME: getAllFiles() may return relative paths, we need absolute paths.
+ // Hopefully the fix is to change JSONCompilationDatabase and the interface.
if (CDB && !Cached)
OnCommandChanged.broadcast(CDB->getAllFiles());
return CDB;
@@ -112,7 +114,7 @@ OverlayCDB::getCompileCommand(PathRef Fi
return It->second;
}
}
- return Base ? Base->getCompileCommand(File) : None;
+ return Base ? Base->getCompileCommand(File, Project) : None;
}
tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
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=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Nov 26 08:00:11 2018
@@ -99,9 +99,11 @@ void BackgroundIndex::run() {
}
}
-void BackgroundIndex::blockUntilIdleForTest() {
+bool BackgroundIndex::blockUntilIdleForTest(
+ llvm::Optional<double> TimeoutSeconds) {
std::unique_lock<std::mutex> Lock(QueueMu);
- QueueCV.wait(Lock, [&] { return Queue.empty() && NumActiveTasks == 0; });
+ return wait(Lock, QueueCV, timeoutSeconds(TimeoutSeconds),
+ [&] { return Queue.empty() && NumActiveTasks == 0; });
}
void BackgroundIndex::enqueue(const std::vector<std::string> &ChangedFiles) {
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=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Mon Nov 26 08:00:11 2018
@@ -81,7 +81,8 @@ public:
void stop();
// Wait until the queue is empty, to allow deterministic testing.
- void blockUntilIdleForTest();
+ LLVM_NODISCARD bool
+ blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds = 10);
using FileDigest = decltype(llvm::SHA1::hash({}));
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=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp Mon Nov 26 08:00:11 2018
@@ -30,7 +30,7 @@ std::string getShardPathFromFilePath(llv
llvm::sys::path::append(ShardRootSS, llvm::sys::path::filename(FilePath) +
"." + llvm::toHex(digest(FilePath)) +
".idx");
- return ShardRoot.str();
+ return ShardRootSS.str();
}
// Uses disk as a storage for index shards. Creates a directory called
@@ -101,6 +101,9 @@ public:
// Creates and owns IndexStorages for multiple CDBs.
class DiskBackedIndexStorageManager {
public:
+ DiskBackedIndexStorageManager()
+ : IndexStorageMapMu(llvm::make_unique<std::mutex>()) {}
+
// Creates or fetches to storage from cache for the specified CDB.
BackgroundIndexStorage *operator()(llvm::StringRef CDBDirectory) {
std::lock_guard<std::mutex> Lock(*IndexStorageMapMu);
Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Mon Nov 26 08:00:11 2018
@@ -164,6 +164,12 @@ static cl::opt<Path> IndexFile(
"eventually. Don't rely on it."),
cl::init(""), cl::Hidden);
+static cl::opt<bool> EnableBackgroundIndex(
+ "background-index",
+ cl::desc("Index project code in the background and persist index on disk. "
+ "Experimental"),
+ cl::init(false), cl::Hidden);
+
enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
static cl::opt<CompileArgsFrom> CompileArgsFrom(
"compile_args_from", cl::desc("The source of compile commands"),
@@ -344,6 +350,7 @@ int main(int argc, char *argv[]) {
Opts.ResourceDir = ResourceDir;
Opts.BuildDynamicSymbolIndex = EnableIndex;
Opts.HeavyweightDynamicSymbolIndex = UseDex;
+ Opts.BackgroundIndex = EnableBackgroundIndex;
std::unique_ptr<SymbolIndex> StaticIdx;
std::future<void> AsyncIndexLoad; // Block exit while loading the index.
if (EnableIndex && !IndexFile.empty()) {
Added: clang-tools-extra/trunk/test/clangd/Inputs/background-index/compile_commands.json
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Inputs/background-index/compile_commands.json?rev=347567&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/Inputs/background-index/compile_commands.json (added)
+++ clang-tools-extra/trunk/test/clangd/Inputs/background-index/compile_commands.json Mon Nov 26 08:00:11 2018
@@ -0,0 +1,5 @@
+[{
+ "directory": "DIRECTORY",
+ "command": "clang foo.cpp",
+ "file": "DIRECTORY/foo.cpp"
+}]
Added: clang-tools-extra/trunk/test/clangd/Inputs/background-index/definition.jsonrpc
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Inputs/background-index/definition.jsonrpc?rev=347567&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/Inputs/background-index/definition.jsonrpc (added)
+++ clang-tools-extra/trunk/test/clangd/Inputs/background-index/definition.jsonrpc Mon Nov 26 08:00:11 2018
@@ -0,0 +1,51 @@
+{
+ "jsonrpc": "2.0",
+ "id": 0,
+ "method": "initialize",
+ "params": {
+ "processId": 123,
+ "rootPath": "clangd",
+ "capabilities": {},
+ "trace": "off"
+ }
+}
+---
+{
+ "jsonrpc": "2.0",
+ "method": "textDocument/didOpen",
+ "params": {
+ "textDocument": {
+ "uri": "file://DIRECTORY/bar.cpp",
+ "languageId": "cpp",
+ "version": 1,
+ "text": "#include \"foo.h\"\nint main(){\nreturn foo();\n}"
+ }
+ }
+}
+---
+{
+ "jsonrpc": "2.0",
+ "id": 1,
+ "method": "sync",
+ "params": null
+}
+---
+{
+ "jsonrpc": "2.0",
+ "id": 2,
+ "method": "textDocument/definition",
+ "params": {
+ "textDocument": {
+ "uri": "file://DIRECTORY/bar.cpp"
+ },
+ "position": {
+ "line": 2,
+ "character": 8
+ }
+ }
+}
+# CHECK: "uri": "file://DIRECTORY/foo.cpp"
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Added: clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.cpp?rev=347567&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.cpp (added)
+++ clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.cpp Mon Nov 26 08:00:11 2018
@@ -0,0 +1,2 @@
+#include "foo.h"
+int foo() { return 42; }
Added: clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.h?rev=347567&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.h (added)
+++ clang-tools-extra/trunk/test/clangd/Inputs/background-index/foo.h Mon Nov 26 08:00:11 2018
@@ -0,0 +1,4 @@
+#ifndef FOO_H
+#define FOO_H
+int foo();
+#endif
Added: clang-tools-extra/trunk/test/clangd/background-index.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/background-index.test?rev=347567&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clangd/background-index.test (added)
+++ clang-tools-extra/trunk/test/clangd/background-index.test Mon Nov 26 08:00:11 2018
@@ -0,0 +1,21 @@
+# We need to splice paths into file:// URIs for this test.
+# UNSUPPORTED: win32
+
+# Use a copy of inputs, as we'll mutate it (as will the background index).
+# 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/*
+
+# We're editing bar.cpp, which includes foo.h.
+# foo() is declared in foo.h and defined in foo.cpp.
+# The background index should allow us to go-to-definition on foo().
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc
+
+# Test that the index is writing files in the expected location.
+# 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
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=347567&r1=347566&r2=347567&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Mon Nov 26 08:00:11 2018
@@ -90,7 +90,7 @@ TEST(BackgroundIndexTest, IndexTwoFiles)
Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
CDB.setCompileCommand(testPath("root"), Cmd);
- Idx.blockUntilIdleForTest();
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
EXPECT_THAT(
runFuzzyFind(Idx, ""),
UnorderedElementsAre(Named("common"), Named("A_CC"),
@@ -100,7 +100,7 @@ TEST(BackgroundIndexTest, IndexTwoFiles)
Cmd.CommandLine = {"clang++", Cmd.Filename};
CDB.setCompileCommand(testPath("root"), Cmd);
- Idx.blockUntilIdleForTest();
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
// B_CC is dropped as we don't collect symbols from A.h in this compilation.
EXPECT_THAT(runFuzzyFind(Idx, ""),
UnorderedElementsAre(Named("common"), Named("A_CC"),
@@ -141,7 +141,7 @@ TEST(BackgroundIndexTest, ShardStorageWr
BackgroundIndex Idx(Context::empty(), "", FS, CDB,
[&](llvm::StringRef) { return &MSS; });
CDB.setCompileCommand(testPath("root"), Cmd);
- Idx.blockUntilIdleForTest();
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
EXPECT_EQ(CacheHits, 0U);
EXPECT_EQ(Storage.size(), 2U);
More information about the cfe-commits
mailing list