[clang-tools-extra] r349496 - [clangd] BackgroundIndex rebuilds symbol index periodically.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 18 07:39:33 PST 2018
Author: ioeric
Date: Tue Dec 18 07:39:33 2018
New Revision: 349496
URL: http://llvm.org/viewvc/llvm-project?rev=349496&view=rev
Log:
[clangd] BackgroundIndex rebuilds symbol index periodically.
Summary:
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.
Reviewers: ilya-biryukov, kadircet
Reviewed By: kadircet
Subscribers: dblaikie, MaskRay, jkorous, arphaman, jfb, cfe-commits
Differential Revision: https://reviews.llvm.org/D55770
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/background-index.test
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Dec 18 07:39:33 2018
@@ -139,7 +139,8 @@ ClangdServer::ClangdServer(const GlobalC
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)
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Dec 18 07:39:33 2018
@@ -85,6 +85,10 @@ public:
/// 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;
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=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Tue Dec 18 07:39:33 2018
@@ -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 @@ createFileFilter(const llvm::StringMap<F
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 @@ BackgroundIndex::BackgroundIndex(
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 @@ BackgroundIndex::~BackgroundIndex() {
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::update(StringRef M
}
}
+void BackgroundIndex::buildIndex() {
+ assert(BuildIndexPeriodMs > 0);
+ while (true) {
+ {
+ std::unique_lock<std::mutex> Lock(IndexMu);
+ if (ShouldStop) // Avoid waiting if stopped.
+ break;
+ // Wait until this is notified to stop or `BuildIndexPeriodMs` has past.
+ IndexCV.wait_for(Lock, std::chrono::milliseconds(BuildIndexPeriodMs));
+ if (ShouldStop) // Avoid rebuilding index if stopped.
+ 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 @@ Error BackgroundIndex::index(tooling::Co
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 @@ Error BackgroundIndex::index(tooling::Co
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();
}
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=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Tue Dec 18 07:39:33 2018
@@ -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 @@ public:
// 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 @@ private:
// 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.
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=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Dec 18 07:39:33 2018
@@ -170,6 +170,14 @@ static cl::opt<bool> EnableBackgroundInd
"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 @@ int main(int argc, char *argv[]) {
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()) {
Modified: 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=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/background-index.test (original)
+++ clang-tools-extra/trunk/test/clangd/background-index.test Tue Dec 18 07:39:33 2018
@@ -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
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=349496&r1=349495&r2=349496&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Tue Dec 18 07:39:33 2018
@@ -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 @@ TEST_F(BackgroundIndexTest, DirectInclud
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
More information about the cfe-commits
mailing list