ioeric updated this revision to Diff 178676.
ioeric marked an inline comment as done.
ioeric added a comment.
- Add comment about double-check of ShouldStop.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55770/new/
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) // 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 @@
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
