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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits