ioeric created this revision.
ioeric added reviewers: ilya-biryukov, kadircet.
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay.
Currently, background index rebuilds symbol index on every indexed file,
which can be inefficient. This patch makes it only rebuild symbol index
periodically.
As the rebuild no longer happens too often, we could also build more efficient
dex index.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D55770
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/index/Background.cpp
clangd/index/Background.h
clangd/tool/ClangdMain.cpp
test/clangd/background-index.test
unittests/clangd/BackgroundIndexTests.cpp
Index: unittests/clangd/BackgroundIndexTests.cpp
===================================================================
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -2,11 +2,14 @@
#include "TestFS.h"
#include "index/Background.h"
#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Threading.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <thread>
using testing::_;
using testing::AllOf;
+using testing::ElementsAre;
using testing::Not;
using testing::UnorderedElementsAre;
@@ -240,5 +243,39 @@
EmptyIncludeNode());
}
+TEST_F(BackgroundIndexTest, PeriodicalIndex) {
+ MockFSProvider FS;
+ llvm::StringMap<std::string> Storage;
+ size_t CacheHits = 0;
+ MemoryShardStorage MSS(Storage, CacheHits);
+ OverlayCDB CDB(/*Base=*/nullptr);
+ BackgroundIndex Idx(
+ Context::empty(), "", FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*BuildIndexPeriodMs=*/10);
+
+ FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
+
+ tooling::CompileCommand Cmd;
+ FS.Files[testPath("root/A.h")] = "class X {};";
+ Cmd.Filename = testPath("root/A.cc");
+ Cmd.CommandLine = {"clang++", Cmd.Filename};
+ CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre());
+ std::this_thread::sleep_for(std::chrono::milliseconds(15));
+ EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X")));
+
+ FS.Files[testPath("root/A.h")] = "class Y {};";
+ FS.Files[testPath("root/A.cc")] += " "; // Force reindex the file.
+ Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
+ CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X")));
+ std::this_thread::sleep_for(std::chrono::milliseconds(15));
+ EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("Y")));
+}
+
} // namespace clangd
} // namespace clang
Index: test/clangd/background-index.test
===================================================================
--- test/clangd/background-index.test
+++ test/clangd/background-index.test
@@ -10,7 +10,7 @@
# 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
+# RUN: clangd -background-index -background-index-rebuild-period=0 -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
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -170,6 +170,14 @@
"Experimental"),
cl::init(false), cl::Hidden);
+static cl::opt<int> BackgroundIndexRebuildPeriod(
+ "background-index-rebuild-period",
+ cl::desc(
+ "If set to non-zero, the background index rebuilds the symbol index "
+ "periodically every X milliseconds; otherwise, the "
+ "symbol index will be updated for each indexed file."),
+ cl::init(5000), cl::Hidden);
+
enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
static cl::opt<CompileArgsFrom> CompileArgsFrom(
"compile_args_from", cl::desc("The source of compile commands"),
@@ -352,6 +360,7 @@
Opts.BuildDynamicSymbolIndex = EnableIndex;
Opts.HeavyweightDynamicSymbolIndex = UseDex;
Opts.BackgroundIndex = EnableBackgroundIndex;
+ Opts.BackgroundIndexRebuildPeriodMs = BackgroundIndexRebuildPeriod;
std::unique_ptr<SymbolIndex> StaticIdx;
std::future<void> AsyncIndexLoad; // Block exit while loading the index.
if (EnableIndex && !IndexFile.empty()) {
Index: clangd/index/Background.h
===================================================================
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -21,8 +21,10 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/SHA1.h"
#include "llvm/Support/Threading.h"
+#include <atomic>
#include <condition_variable>
#include <deque>
+#include <mutex>
#include <string>
#include <thread>
#include <vector>
@@ -63,11 +65,15 @@
// FIXME: it should watch for changes to files on disk.
class BackgroundIndex : public SwapIndex {
public:
+ /// If BuildIndexPeriodMs is greater than 0, the symbol index will only be
+ /// rebuilt periodically (one per \p BuildIndexPeriodMs); otherwise, index is
+ /// rebuilt for each indexed file.
// 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 BuildIndexPeriodMs = 0,
size_t ThreadPoolSize = llvm::hardware_concurrency());
~BackgroundIndex(); // Blocks while the current task finishes.
@@ -101,6 +107,11 @@
// index state
llvm::Error index(tooling::CompileCommand,
BackgroundIndexStorage *IndexStorage);
+ void buildIndex(); // Rebuild index periodically every BuildIndexPeriodMs.
+ const size_t BuildIndexPeriodMs;
+ std::atomic<bool> SymbolsUpdatedSinceLastIndex;
+ std::mutex IndexMu;
+ std::condition_variable IndexCV;
FileSymbols IndexedSymbols;
llvm::StringMap<FileDigest> IndexedFileDigests; // Key is absolute file path.
Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -27,11 +27,13 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/SHA1.h"
+#include <chrono>
#include <memory>
#include <numeric>
#include <queue>
#include <random>
#include <string>
+#include <thread>
using namespace llvm;
namespace clang {
@@ -125,10 +127,13 @@
BackgroundIndex::BackgroundIndex(
Context BackgroundContext, StringRef ResourceDir,
const FileSystemProvider &FSProvider, const GlobalCompilationDatabase &CDB,
- BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize)
+ BackgroundIndexStorage::Factory IndexStorageFactory,
+ size_t BuildIndexPeriodMs, size_t ThreadPoolSize)
: SwapIndex(make_unique<MemIndex>()), ResourceDir(ResourceDir),
FSProvider(FSProvider), CDB(CDB),
BackgroundContext(std::move(BackgroundContext)),
+ BuildIndexPeriodMs(BuildIndexPeriodMs),
+ SymbolsUpdatedSinceLastIndex(false),
IndexStorageFactory(std::move(IndexStorageFactory)),
CommandsChanged(
CDB.watch([&](const std::vector<std::string> &ChangedFiles) {
@@ -138,6 +143,11 @@
assert(this->IndexStorageFactory && "Storage factory can not be null!");
while (ThreadPoolSize--)
ThreadPool.emplace_back([this] { run(); });
+ if (BuildIndexPeriodMs > 0) {
+ log("BackgroundIndex: build symbol index periodically every {0} ms.",
+ BuildIndexPeriodMs);
+ ThreadPool.emplace_back([this] { buildIndex(); });
+ }
}
BackgroundIndex::~BackgroundIndex() {
@@ -148,10 +158,12 @@
void BackgroundIndex::stop() {
{
- std::lock_guard<std::mutex> Lock(QueueMu);
+ std::lock_guard<std::mutex> QueueLock(QueueMu);
+ std::lock_guard<std::mutex> IndexLock(IndexMu);
ShouldStop = true;
}
QueueCV.notify_all();
+ IndexCV.notify_all();
}
void BackgroundIndex::run() {
@@ -332,6 +344,30 @@
}
}
+void BackgroundIndex::buildIndex() {
+ assert(BuildIndexPeriodMs > 0);
+ while (true) {
+ {
+ std::unique_lock<std::mutex> Lock(IndexMu);
+ if (ShouldStop)
+ break;
+ // Wait until this is notified to stop or `BuildIndexPeriodMs` has past.
+ IndexCV.wait_for(Lock, std::chrono::milliseconds(BuildIndexPeriodMs));
+ if (ShouldStop)
+ break;
+ }
+ if (!SymbolsUpdatedSinceLastIndex.exchange(false))
+ continue;
+ // There can be symbol update right after the flag is reset above and before
+ // index is rebuilt below. The new index would contain the updated symbols
+ // but the flag would still be true. This is fine as we would simply run an
+ // extra index build.
+ reset(
+ IndexedSymbols.buildIndex(IndexType::Heavy, DuplicateHandling::Merge));
+ log("BackgroundIndex: rebuilt symbol index.");
+ }
+}
+
Error BackgroundIndex::index(tooling::CompileCommand Cmd,
BackgroundIndexStorage *IndexStorage) {
trace::Span Tracer("BackgroundIndex");
@@ -362,7 +398,7 @@
DigestsSnapshot = IndexedFileDigests;
}
- log("Indexing {0}", Cmd.Filename, toHex(Hash));
+ log("Indexing {0} (digest:={1})", Cmd.Filename, toHex(Hash));
ParseInputs Inputs;
Inputs.FS = std::move(FS);
Inputs.FS->setCurrentWorkingDirectory(Cmd.Directory);
@@ -424,10 +460,11 @@
IndexedFileDigests[AbsolutePath] = Hash;
}
- // FIXME: this should rebuild once-in-a-while, not after every file.
- // At that point we should use Dex, too.
- vlog("Rebuilding automatic index");
- reset(IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
+ if (BuildIndexPeriodMs > 0)
+ SymbolsUpdatedSinceLastIndex = true;
+ else
+ reset(
+ IndexedSymbols.buildIndex(IndexType::Light, DuplicateHandling::Merge));
return Error::success();
}
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -85,6 +85,10 @@
/// 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 to non-zero, the background index rebuilds the symbol index
+ /// periodically every BuildIndexPeriodMs milliseconds; otherwise, the
+ /// symbol index will be updated for each indexed file.
+ size_t BackgroundIndexRebuildPeriodMs = 0;
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -139,7 +139,8 @@
if (Opts.BackgroundIndex) {
BackgroundIdx = llvm::make_unique<BackgroundIndex>(
Context::current().clone(), ResourceDir, FSProvider, CDB,
- BackgroundIndexStorage::createDiskBackedStorageFactory());
+ BackgroundIndexStorage::createDiskBackedStorageFactory(),
+ Opts.BackgroundIndexRebuildPeriodMs);
AddIndex(BackgroundIdx.get());
}
if (DynamicIdx)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
