[clang-tools-extra] b36e22d - [clangd] Extract BackgroundIndex::Options struct. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 13 09:13:02 PDT 2020
Author: Sam McCall
Date: 2020-08-13T18:12:54+02:00
New Revision: b36e22d64458fb87119eddc383229b6d0493967b
URL: https://github.com/llvm/llvm-project/commit/b36e22d64458fb87119eddc383229b6d0493967b
DIFF: https://github.com/llvm/llvm-project/commit/b36e22d64458fb87119eddc383229b6d0493967b.diff
LOG: [clangd] Extract BackgroundIndex::Options struct. NFC
I've dropped the background context parameter, since we in practice just pass the
current context there, and we now have a different way to specify context too.
While here, clean up a couple of comments.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D83157
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index cd6cba8c2ed3..300d345f8eeb 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -171,16 +171,20 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
if (Opts.StaticIndex)
AddIndex(Opts.StaticIndex);
if (Opts.BackgroundIndex) {
+ BackgroundIndex::Options BGOpts;
+ BGOpts.ThreadPoolSize = std::max(Opts.AsyncThreadsCount, 1u);
+ BGOpts.OnProgress = [Callbacks](BackgroundQueue::Stats S) {
+ if (Callbacks)
+ Callbacks->onBackgroundIndexProgress(S);
+ };
+ BGOpts.ContextProvider = [this](PathRef P) {
+ return createProcessingContext(P);
+ };
BackgroundIdx = std::make_unique<BackgroundIndex>(
- Context::current().clone(), TFS, CDB,
+ TFS, CDB,
BackgroundIndexStorage::createDiskBackedStorageFactory(
[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }),
- std::max(Opts.AsyncThreadsCount, 1u),
- [Callbacks](BackgroundQueue::Stats S) {
- if (Callbacks)
- Callbacks->onBackgroundIndexProgress(S);
- },
- [this](PathRef P) { return createProcessingContext(P); });
+ std::move(BGOpts));
AddIndex(BackgroundIdx.get());
}
if (DynamicIdx)
diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index a22785b01d64..18037d694c11 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -91,28 +91,25 @@ bool shardIsStale(const LoadedShard &LS, llvm::vfs::FileSystem *FS) {
} // namespace
BackgroundIndex::BackgroundIndex(
- Context BackgroundContext, const ThreadsafeFS &TFS,
- const GlobalCompilationDatabase &CDB,
- BackgroundIndexStorage::Factory IndexStorageFactory, size_t ThreadPoolSize,
- std::function<void(BackgroundQueue::Stats)> OnProgress,
- std::function<Context(PathRef)> ContextProvider)
+ const ThreadsafeFS &TFS, const GlobalCompilationDatabase &CDB,
+ BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts)
: SwapIndex(std::make_unique<MemIndex>()), TFS(TFS), CDB(CDB),
- BackgroundContext(std::move(BackgroundContext)),
- ContextProvider(std::move(ContextProvider)),
- Rebuilder(this, &IndexedSymbols, ThreadPoolSize),
+ ContextProvider(std::move(Opts.ContextProvider)),
+ Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize),
IndexStorageFactory(std::move(IndexStorageFactory)),
- Queue(std::move(OnProgress)),
+ Queue(std::move(Opts.OnProgress)),
CommandsChanged(
CDB.watch([&](const std::vector<std::string> &ChangedFiles) {
enqueue(ChangedFiles);
})) {
- assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
+ assert(Opts.ThreadPoolSize > 0 && "Thread pool size can't be zero.");
assert(this->IndexStorageFactory && "Storage factory can not be null!");
- for (unsigned I = 0; I < ThreadPoolSize; ++I) {
- ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1), [this] {
- WithContext Ctx(this->BackgroundContext.clone());
- Queue.work([&] { Rebuilder.idle(); });
- });
+ for (unsigned I = 0; I < Opts.ThreadPoolSize; ++I) {
+ ThreadPool.runAsync("background-worker-" + llvm::Twine(I + 1),
+ [this, Ctx(Context::current().clone())]() mutable {
+ WithContext BGContext(std::move(Ctx));
+ Queue.work([&] { Rebuilder.idle(); });
+ });
}
}
diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h
index 9adad1737686..72fe84466959 100644
--- a/clang-tools-extra/clangd/index/Background.h
+++ b/clang-tools-extra/clangd/index/Background.h
@@ -124,22 +124,26 @@ class BackgroundQueue {
// Builds an in-memory index by by running the static indexer action over
// all commands in a compilation database. Indexing happens in the background.
-// FIXME: it should also persist its state on disk for fast start.
// 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.
- BackgroundIndex(
- Context BackgroundContext, const ThreadsafeFS &,
- const GlobalCompilationDatabase &CDB,
- BackgroundIndexStorage::Factory IndexStorageFactory,
- // Arbitrary value to ensure some concurrency in tests.
- // In production an explicit value is passed.
- size_t ThreadPoolSize = 4,
- std::function<void(BackgroundQueue::Stats)> OnProgress = nullptr,
- std::function<Context(PathRef)> ContextProvider = nullptr);
+ struct Options {
+ // Arbitrary value to ensure some concurrency in tests.
+ // In production an explicit value is specified.
+ size_t ThreadPoolSize = 4;
+ // Callback that provides notifications as indexing makes progress.
+ std::function<void(BackgroundQueue::Stats)> OnProgress = nullptr;
+ // Function called to obtain the Context to use while indexing the specified
+ // file. Called with the empty string for other tasks.
+ // (When called, the context from BackgroundIndex construction is active).
+ std::function<Context(PathRef)> ContextProvider = nullptr;
+ };
+
+ /// Creates a new background index and starts its threads.
+ /// The current Context will be propagated to each worker thread.
+ BackgroundIndex(const ThreadsafeFS &, const GlobalCompilationDatabase &CDB,
+ BackgroundIndexStorage::Factory IndexStorageFactory,
+ Options Opts);
~BackgroundIndex(); // Blocks while the current task finishes.
// Enqueue translation units for indexing.
@@ -183,7 +187,6 @@ class BackgroundIndex : public SwapIndex {
// configuration
const ThreadsafeFS &TFS;
const GlobalCompilationDatabase &CDB;
- Context BackgroundContext;
std::function<Context(PathRef)> ContextProvider;
llvm::Error index(tooling::CompileCommand);
diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index f1c582ef1abe..06614872363f 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -96,8 +96,8 @@ TEST_F(BackgroundIndexTest, NoCrashOnErrorFile) {
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
tooling::CompileCommand Cmd;
Cmd.Filename = testPath("root/A.cc");
@@ -127,7 +127,8 @@ TEST_F(BackgroundIndexTest, Config) {
// Context provider that installs a configuration mutating foo's command.
// This causes it to define foo::two instead of foo::one.
// It also disables indexing of baz entirely.
- auto ContextProvider = [](PathRef P) {
+ BackgroundIndex::Options Opts;
+ Opts.ContextProvider = [](PathRef P) {
Config C;
if (P.endswith("foo.cpp"))
C.CompileFlags.Edits.push_back(
@@ -143,9 +144,9 @@ TEST_F(BackgroundIndexTest, Config) {
// We need the CommandMangler, because that applies the config we're testing.
OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{},
tooling::ArgumentsAdjuster(CommandMangler::forTests()));
+
BackgroundIndex Idx(
- Context::empty(), FS, CDB, [&](llvm::StringRef) { return &MSS; },
- /*ThreadPoolSize=*/4, /*OnProgress=*/nullptr, std::move(ContextProvider));
+ FS, CDB, [&](llvm::StringRef) { return &MSS; }, std::move(Opts));
// Index the two files.
for (auto &Cmd : Cmds) {
std::string FullPath = testPath(Cmd.Filename);
@@ -186,8 +187,8 @@ TEST_F(BackgroundIndexTest, IndexTwoFiles) {
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
tooling::CompileCommand Cmd;
Cmd.Filename = testPath("root/A.cc");
@@ -253,8 +254,8 @@ TEST_F(BackgroundIndexTest, ShardStorageTest) {
// Check nothing is loaded from Storage, but A.cc and A.h has been stored.
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -263,8 +264,8 @@ TEST_F(BackgroundIndexTest, ShardStorageTest) {
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -321,8 +322,8 @@ TEST_F(BackgroundIndexTest, DirectIncludesTest) {
Cmd.CommandLine = {"clang++", testPath("root/A.cc")};
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -371,8 +372,8 @@ TEST_F(BackgroundIndexTest, ShardStorageLoad) {
// Check nothing is loaded from Storage, but A.cc and A.h has been stored.
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -386,8 +387,8 @@ TEST_F(BackgroundIndexTest, ShardStorageLoad) {
)cpp";
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -404,8 +405,8 @@ TEST_F(BackgroundIndexTest, ShardStorageLoad) {
{
CacheHits = 0;
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -445,8 +446,8 @@ TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) {
// Check that A.cc, A.h and B.h has been stored.
{
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -461,8 +462,8 @@ TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) {
{
CacheHits = 0;
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -477,8 +478,8 @@ TEST_F(BackgroundIndexTest, ShardStorageEmptyFile) {
{
CacheHits = 0;
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
ASSERT_TRUE(Idx.blockUntilIdleForTest());
}
@@ -495,8 +496,8 @@ TEST_F(BackgroundIndexTest, NoDotsInAbsPath) {
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
ASSERT_TRUE(Idx.blockUntilIdleForTest());
tooling::CompileCommand Cmd;
@@ -526,8 +527,8 @@ TEST_F(BackgroundIndexTest, UncompilableFiles) {
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
tooling::CompileCommand Cmd;
FS.Files[testPath("A.h")] = "void foo();";
@@ -589,8 +590,8 @@ TEST_F(BackgroundIndexTest, CmdLineHash) {
size_t CacheHits = 0;
MemoryShardStorage MSS(Storage, CacheHits);
OverlayCDB CDB(/*Base=*/nullptr);
- BackgroundIndex Idx(Context::empty(), FS, CDB,
- [&](llvm::StringRef) { return &MSS; });
+ BackgroundIndex Idx(FS, CDB, [&](llvm::StringRef) { return &MSS; },
+ /*Opts=*/{});
tooling::CompileCommand Cmd;
FS.Files[testPath("A.cc")] = "#include \"A.h\"";
More information about the cfe-commits
mailing list