[clang-tools-extra] b79cb54 - [clangd] Refactor TUScheduler options into a struct. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 3 02:25:56 PST 2020
Author: Sam McCall
Date: 2020-02-03T11:20:48+01:00
New Revision: b79cb547121dbd9f4bd4eaaf05354829ab74ac8e
URL: https://github.com/llvm/llvm-project/commit/b79cb547121dbd9f4bd4eaaf05354829ab74ac8e
DIFF: https://github.com/llvm/llvm-project/commit/b79cb547121dbd9f4bd4eaaf05354829ab74ac8e.diff
LOG: [clangd] Refactor TUScheduler options into a struct. NFC
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index cf883f130da8..00da7af06741 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -113,6 +113,15 @@ ClangdServer::Options ClangdServer::optsForTest() {
return Opts;
}
+ClangdServer::Options::operator TUScheduler::Options() const {
+ TUScheduler::Options Opts;
+ Opts.AsyncThreadsCount = AsyncThreadsCount;
+ Opts.RetentionPolicy = RetentionPolicy;
+ Opts.StorePreamblesInMemory = StorePreamblesInMemory;
+ Opts.UpdateDebounce = UpdateDebounce;
+ return Opts;
+}
+
ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
const FileSystemProvider &FSProvider,
const Options &Opts, Callbacks *Callbacks)
@@ -129,10 +138,10 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
// is parsed.
// FIXME(ioeric): this can be slow and we may be able to index on less
// critical paths.
- WorkScheduler(CDB, Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
- std::make_unique<UpdateIndexCallbacks>(
- DynamicIdx.get(), Callbacks, Opts.SemanticHighlighting),
- Opts.UpdateDebounce, Opts.RetentionPolicy) {
+ WorkScheduler(
+ CDB, TUScheduler::Options(Opts),
+ std::make_unique<UpdateIndexCallbacks>(DynamicIdx.get(), Callbacks,
+ Opts.SemanticHighlighting)) {
// Adds an index to the stack, at higher priority than existing indexes.
auto AddIndex = [&](SymbolIndex *Idx) {
if (this->Index != nullptr) {
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 7b870e5b300f..2d0957f441bb 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -149,6 +149,8 @@ class ClangdServer {
std::function<bool(const Tweak &)> TweakFilter = [](const Tweak &T) {
return !T.hidden(); // only enable non-hidden tweaks.
};
+
+ explicit operator TUScheduler::Options() const;
};
// Sensible default options for use in tests.
// Features like indexing must be enabled if desired.
diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index 9ea1582e9643..69dec9b677fb 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -847,18 +847,16 @@ struct TUScheduler::FileData {
};
TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB,
- unsigned AsyncThreadsCount,
- bool StorePreamblesInMemory,
- std::unique_ptr<ParsingCallbacks> Callbacks,
- std::chrono::steady_clock::duration UpdateDebounce,
- ASTRetentionPolicy RetentionPolicy)
- : CDB(CDB), StorePreamblesInMemory(StorePreamblesInMemory),
+ const Options &Opts,
+ std::unique_ptr<ParsingCallbacks> Callbacks)
+ : CDB(CDB), StorePreamblesInMemory(Opts.StorePreamblesInMemory),
Callbacks(Callbacks ? move(Callbacks)
: std::make_unique<ParsingCallbacks>()),
- Barrier(AsyncThreadsCount),
- IdleASTs(std::make_unique<ASTCache>(RetentionPolicy.MaxRetainedASTs)),
- UpdateDebounce(UpdateDebounce) {
- if (0 < AsyncThreadsCount) {
+ Barrier(Opts.AsyncThreadsCount),
+ IdleASTs(
+ std::make_unique<ASTCache>(Opts.RetentionPolicy.MaxRetainedASTs)),
+ UpdateDebounce(Opts.UpdateDebounce) {
+ if (0 < Opts.AsyncThreadsCount) {
PreambleTasks.emplace();
WorkerThreads.emplace();
}
diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h
index 65d87df5a939..e59bebaea330 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -20,6 +20,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
+#include <chrono>
namespace clang {
namespace clangd {
@@ -143,14 +144,28 @@ class ParsingCallbacks {
/// and scheduling tasks.
/// Callbacks are run on a threadpool and it's appropriate to do slow work in
/// them. Each task has a name, used for tracing (should be UpperCamelCase).
-/// FIXME(sammccall): pull out a scheduler options struct.
class TUScheduler {
public:
- TUScheduler(const GlobalCompilationDatabase &CDB, unsigned AsyncThreadsCount,
- bool StorePreamblesInMemory,
- std::unique_ptr<ParsingCallbacks> ASTCallbacks,
- std::chrono::steady_clock::duration UpdateDebounce,
- ASTRetentionPolicy RetentionPolicy);
+ struct Options {
+ /// Number of concurrent actions.
+ /// Governs per-file worker threads and threads spawned for other tasks.
+ /// (This does not prevent threads being spawned, but rather blocks them).
+ /// If 0, executes actions synchronously on the calling thread.
+ unsigned AsyncThreadsCount = getDefaultAsyncThreadsCount();
+
+ /// Cache (large) preamble data in RAM rather than temporary files on disk.
+ bool StorePreamblesInMemory = false;
+
+ /// Time to wait after an update to see if another one comes along.
+ /// This tries to ensure we rebuild once the user stops typing.
+ std::chrono::steady_clock::duration UpdateDebounce = /*zero*/ {};
+
+ /// Determines when to keep idle ASTs in memory for future use.
+ ASTRetentionPolicy RetentionPolicy;
+ };
+
+ TUScheduler(const GlobalCompilationDatabase &CDB, const Options &Opts,
+ std::unique_ptr<ParsingCallbacks> ASTCallbacks = nullptr);
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 1e1cef0cd412..053d4aaefaa6 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -42,6 +42,10 @@ MATCHER_P2(TUState, State, ActionName, "") {
return arg.Action.S == State && arg.Action.Name == ActionName;
}
+TUScheduler::Options optsForTest() {
+ return TUScheduler::Options(ClangdServer::optsForTest());
+}
+
class TUSchedulerTests : public ::testing::Test {
protected:
ParseInputs getInputs(PathRef File, std::string Contents) {
@@ -125,10 +129,7 @@ Key<llvm::unique_function<void(PathRef File, std::vector<Diag>)>>
TUSchedulerTests::DiagsCallbackKey;
TEST_F(TUSchedulerTests, MissingFiles) {
- TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest());
auto Added = testPath("added.cpp");
Files[Added] = "x";
@@ -179,11 +180,7 @@ TEST_F(TUSchedulerTests, WantDiagnostics) {
// To avoid a racy test, don't allow tasks to actually run on the worker
// thread until we've scheduled them all.
Notification Ready;
- TUScheduler S(
- CDB, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, captureDiags(),
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest(), captureDiags());
auto Path = testPath("foo.cpp");
updateWithDiags(S, Path, "", WantDiagnostics::Yes,
[&](std::vector<Diag>) { Ready.wait(); });
@@ -210,10 +207,9 @@ TEST_F(TUSchedulerTests, WantDiagnostics) {
TEST_F(TUSchedulerTests, Debounce) {
std::atomic<int> CallbackCount(0);
{
- TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, captureDiags(),
- /*UpdateDebounce=*/std::chrono::seconds(1),
- ASTRetentionPolicy());
+ auto Opts = optsForTest();
+ Opts.UpdateDebounce = std::chrono::seconds(1);
+ TUScheduler S(CDB, Opts, captureDiags());
// FIXME: we could probably use timeouts lower than 1 second here.
auto Path = testPath("foo.cpp");
updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto,
@@ -245,11 +241,7 @@ TEST_F(TUSchedulerTests, PreambleConsistency) {
std::atomic<int> CallbackCount(0);
{
Notification InconsistentReadDone; // Must live longest.
- TUScheduler S(
- CDB, getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
- /*ASTCallbacks=*/nullptr,
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest());
auto Path = testPath("foo.cpp");
// Schedule two updates (A, B) and two preamble reads (stale, consistent).
// The stale read should see A, and the consistent read should see B.
@@ -302,11 +294,7 @@ TEST_F(TUSchedulerTests, Cancellation) {
std::vector<std::string> DiagsSeen, ReadsSeen, ReadsCanceled;
{
Notification Proceed; // Ensure we schedule everything.
- TUScheduler S(
- CDB, getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
- /*ASTCallbacks=*/captureDiags(),
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest(), captureDiags());
auto Path = testPath("foo.cpp");
// Helper to schedule a named update and return a function to cancel it.
auto Update = [&](std::string ID) -> Canceler {
@@ -372,10 +360,9 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
// Run TUScheduler and collect some stats.
{
- TUScheduler S(CDB, getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, captureDiags(),
- /*UpdateDebounce=*/std::chrono::milliseconds(50),
- ASTRetentionPolicy());
+ auto Opts = optsForTest();
+ Opts.UpdateDebounce = std::chrono::milliseconds(50);
+ TUScheduler S(CDB, Opts, captureDiags());
std::vector<std::string> Files;
for (int I = 0; I < FilesCount; ++I) {
@@ -461,13 +448,10 @@ TEST_F(TUSchedulerTests, ManyUpdates) {
TEST_F(TUSchedulerTests, EvictedAST) {
std::atomic<int> BuiltASTCounter(0);
- ASTRetentionPolicy Policy;
- Policy.MaxRetainedASTs = 2;
- TUScheduler S(CDB,
- /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
- /*ASTCallbacks=*/nullptr,
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- Policy);
+ auto Opts = optsForTest();
+ Opts.AsyncThreadsCount = 1;
+ Opts.RetentionPolicy.MaxRetainedASTs = 2;
+ TUScheduler S(CDB, Opts);
llvm::StringLiteral SourceContents = R"cpp(
int* a;
@@ -514,11 +498,7 @@ TEST_F(TUSchedulerTests, EvictedAST) {
}
TEST_F(TUSchedulerTests, EmptyPreamble) {
- TUScheduler S(CDB,
- /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
- /*ASTCallbacks=*/nullptr,
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest());
auto Foo = testPath("foo.cpp");
auto Header = testPath("foo.h");
@@ -559,11 +539,7 @@ TEST_F(TUSchedulerTests, EmptyPreamble) {
TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
// Testing strategy: we update the file and schedule a few preamble reads at
// the same time. All reads should get the same non-null preamble.
- TUScheduler S(CDB,
- /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
- /*ASTCallbacks=*/nullptr,
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest());
auto Foo = testPath("foo.cpp");
auto NonEmptyPreamble = R"cpp(
#define FOO 1
@@ -591,11 +567,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
}
TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
- TUScheduler S(CDB,
- /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, captureDiags(),
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest(), captureDiags());
auto Source = testPath("foo.cpp");
auto Header = testPath("foo.h");
@@ -644,11 +616,7 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
}
TEST_F(TUSchedulerTests, NoChangeDiags) {
- TUScheduler S(CDB,
- /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, captureDiags(),
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest(), captureDiags());
auto FooCpp = testPath("foo.cpp");
auto Contents = "int a; int b;";
@@ -679,10 +647,7 @@ TEST_F(TUSchedulerTests, NoChangeDiags) {
}
TEST_F(TUSchedulerTests, Run) {
- TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ TUScheduler S(CDB, optsForTest());
std::atomic<int> Counter(0);
S.run("add 1", [&] { ++Counter; });
S.run("add 2", [&] { Counter += 2; });
@@ -753,11 +718,7 @@ TEST_F(TUSchedulerTests, CommandLineErrors) {
// (!) 'Ready' must live longer than TUScheduler.
Notification Ready;
- TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/captureDiags(),
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
-
+ TUScheduler S(CDB, optsForTest(), captureDiags());
std::vector<Diag> Diagnostics;
updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
WantDiagnostics::Yes, [&](std::vector<Diag> D) {
@@ -781,11 +742,7 @@ TEST_F(TUSchedulerTests, CommandLineWarnings) {
// (!) 'Ready' must live longer than TUScheduler.
Notification Ready;
- TUScheduler S(CDB, /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/captureDiags(),
- /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
-
+ TUScheduler S(CDB, optsForTest(), captureDiags());
std::vector<Diag> Diagnostics;
updateWithDiags(S, testPath("foo.cpp"), "void test() {}",
WantDiagnostics::Yes, [&](std::vector<Diag> D) {
More information about the cfe-commits
mailing list