diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 3e22a4dfe6675..00da7af067416 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -106,7 +106,7 @@ struct UpdateIndexCallbacks : public ParsingCallbacks { ClangdServer::Options ClangdServer::optsForTest() { ClangdServer::Options Opts; - Opts.UpdateDebounce = DebouncePolicy::fixed(/*zero*/ {}); + Opts.UpdateDebounce = std::chrono::steady_clock::duration::zero(); // Faster! Opts.StorePreamblesInMemory = true; Opts.AsyncThreadsCount = 4; // Consistent! Opts.SemanticHighlighting = true; diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 3c3505295a75f..2d0957f441bb2 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -130,8 +130,8 @@ class ClangdServer { llvm::Optional ResourceDir = llvm::None; /// Time to wait after a new file version before computing diagnostics. - DebouncePolicy UpdateDebounce = - DebouncePolicy::fixed(std::chrono::milliseconds(500)); + std::chrono::steady_clock::duration UpdateDebounce = + std::chrono::milliseconds(500); bool SuggestMissingIncludes = false; diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 237c4c4d7ba6f..9218ae9a03ce2 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -61,7 +61,6 @@ #include "llvm/Support/Threading.h" #include #include -#include #include #include @@ -165,7 +164,7 @@ class ASTWorker { friend class ASTWorkerHandle; ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync, - DebouncePolicy UpdateDebounce, bool StorePreamblesInMemory, + steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory, ParsingCallbacks &Callbacks); public: @@ -177,7 +176,7 @@ class ASTWorker { static ASTWorkerHandle create(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks, - Semaphore &Barrier, DebouncePolicy UpdateDebounce, + Semaphore &Barrier, steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory, ParsingCallbacks &Callbacks); ~ASTWorker(); @@ -243,7 +242,7 @@ class ASTWorker { TUScheduler::ASTCache &IdleASTs; const bool RunSync; /// Time to wait after an update to see whether another update obsoletes it. - const DebouncePolicy UpdateDebounce; + const steady_clock::duration UpdateDebounce; /// File that ASTWorker is responsible for. const Path FileName; const GlobalCompilationDatabase &CDB; @@ -264,9 +263,6 @@ class ASTWorker { /// be consumed by clients of ASTWorker. std::shared_ptr FileInputs; /* GUARDED_BY(Mutex) */ std::shared_ptr LastBuiltPreamble; /* GUARDED_BY(Mutex) */ - /// Times of recent AST rebuilds, used for UpdateDebounce computation. - llvm::SmallVector - RebuildTimes; /* GUARDED_BY(Mutex) */ /// Becomes ready when the first preamble build finishes. Notification PreambleWasBuilt; /// Set to true to signal run() to finish processing. @@ -330,7 +326,7 @@ class ASTWorkerHandle { ASTWorkerHandle ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks, - Semaphore &Barrier, DebouncePolicy UpdateDebounce, + Semaphore &Barrier, steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory, ParsingCallbacks &Callbacks) { std::shared_ptr Worker( new ASTWorker(FileName, CDB, IdleASTs, Barrier, /*RunSync=*/!Tasks, @@ -344,7 +340,7 @@ ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB, ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, - bool RunSync, DebouncePolicy UpdateDebounce, + bool RunSync, steady_clock::duration UpdateDebounce, bool StorePreamblesInMemory, ParsingCallbacks &Callbacks) : IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce), FileName(FileName), CDB(CDB), @@ -492,7 +488,6 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) { // Get the AST for diagnostics. llvm::Optional> AST = IdleASTs.take(this); - auto RebuildStartTime = DebouncePolicy::clock::now(); if (!AST) { llvm::Optional NewAST = buildAST(FileName, std::move(Invocation), CompilerInvocationDiags, @@ -515,19 +510,6 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) { // spam us with updates. // Note *AST can still be null if buildAST fails. if (*AST) { - { - // Try to record the AST-build time, to inform future update debouncing. - // This is best-effort only: if the lock is held, don't bother. - auto RebuildDuration = DebouncePolicy::clock::now() - RebuildStartTime; - std::unique_lock Lock(Mutex, std::try_to_lock); - if (Lock.owns_lock()) { - // Do not let RebuildTimes grow beyond its small-size (i.e. capacity). - if (RebuildTimes.size() == RebuildTimes.capacity()) - RebuildTimes.erase(RebuildTimes.begin()); - RebuildTimes.push_back(RebuildDuration); - Mutex.unlock(); - } - } trace::Span Span("Running main AST callback"); Callbacks.onMainAST(FileName, **AST, RunPublish); @@ -768,13 +750,13 @@ Deadline ASTWorker::scheduleLocked() { assert(!Requests.empty() && "skipped the whole queue"); // Some updates aren't dead yet, but never end up being used. // e.g. the first keystroke is live until obsoleted by the second. - // We debounce "maybe-unused" writes, sleeping in case they become dead. + // We debounce "maybe-unused" writes, sleeping 500ms in case they become dead. // But don't delay reads (including updates where diagnostics are needed). for (const auto &R : Requests) if (R.UpdateType == None || R.UpdateType == WantDiagnostics::Yes) return Deadline::zero(); // Front request needs to be debounced, so determine when we're ready. - Deadline D(Requests.front().AddTime + UpdateDebounce.compute(RebuildTimes)); + Deadline D(Requests.front().AddTime + UpdateDebounce); return D; } @@ -1054,33 +1036,5 @@ std::vector TUScheduler::getFilesWithCachedAST() const { return Result; } -DebouncePolicy::clock::duration -DebouncePolicy::compute(llvm::ArrayRef History) const { - assert(Min <= Max && "Invalid policy"); - if (History.empty()) - return Max; // Arbitrary. - - // Base the result on the median rebuild. - // nth_element needs a mutable array, take the chance to bound the data size. - History = History.take_back(15); - llvm::SmallVector Recent(History.begin(), History.end()); - auto Median = Recent.begin() + Recent.size() / 2; - std::nth_element(Recent.begin(), Median, Recent.end()); - - clock::duration Target = - std::chrono::duration_cast(RebuildRatio * *Median); - if (Target > Max) - return Max; - if (Target < Min) - return Min; - return Target; -} - -DebouncePolicy DebouncePolicy::fixed(clock::duration T) { - DebouncePolicy P; - P.Min = P.Max = T; - return P; -} - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index 5082612b0ccc3..e59bebaea3305 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -61,28 +61,6 @@ struct ASTRetentionPolicy { unsigned MaxRetainedASTs = 3; }; -/// Clangd may wait after an update to see if another one comes along. -/// This is so we rebuild once the user stops typing, not when they start. -/// Debounce may be disabled/interrupted if we must build this version. -/// The debounce time is responsive to user preferences and rebuild time. -/// In the future, we could also consider different types of edits. -struct DebouncePolicy { - using clock = std::chrono::steady_clock; - - /// The minimum time that we always debounce for. - clock::duration Min = /*zero*/ {}; - /// The maximum time we may debounce for. - clock::duration Max = /*zero*/ {}; - /// Target debounce, as a fraction of file rebuild time. - /// e.g. RebuildRatio = 2, recent builds took 200ms => debounce for 400ms. - float RebuildRatio = 1; - - /// Compute the time to debounce based on this policy and recent build times. - clock::duration compute(llvm::ArrayRef History) const; - /// A policy that always returns the same duration, useful for tests. - static DebouncePolicy fixed(clock::duration); -}; - struct TUAction { enum State { Queued, // The TU is pending in the thread task queue to be built. @@ -180,7 +158,7 @@ class TUScheduler { /// Time to wait after an update to see if another one comes along. /// This tries to ensure we rebuild once the user stops typing. - DebouncePolicy UpdateDebounce; + std::chrono::steady_clock::duration UpdateDebounce = /*zero*/ {}; /// Determines when to keep idle ASTs in memory for future use. ASTRetentionPolicy RetentionPolicy; @@ -295,7 +273,7 @@ class TUScheduler { // asynchronously. llvm::Optional PreambleTasks; llvm::Optional WorkerThreads; - DebouncePolicy UpdateDebounce; + std::chrono::steady_clock::duration UpdateDebounce; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp index 6605ccf65860f..053d4aaefaa60 100644 --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -23,7 +23,6 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include -#include #include namespace clang { @@ -209,7 +208,7 @@ TEST_F(TUSchedulerTests, Debounce) { std::atomic CallbackCount(0); { auto Opts = optsForTest(); - Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1)); + 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"); @@ -362,7 +361,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) { // Run TUScheduler and collect some stats. { auto Opts = optsForTest(); - Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::milliseconds(50)); + Opts.UpdateDebounce = std::chrono::milliseconds(50); TUScheduler S(CDB, Opts, captureDiags()); std::vector Files; @@ -755,32 +754,6 @@ TEST_F(TUSchedulerTests, CommandLineWarnings) { EXPECT_THAT(Diagnostics, IsEmpty()); } -TEST(DebouncePolicy, Compute) { - namespace c = std::chrono; - std::vector History = { - c::seconds(0), - c::seconds(5), - c::seconds(10), - c::seconds(20), - }; - DebouncePolicy Policy; - Policy.Min = c::seconds(3); - Policy.Max = c::seconds(25); - // Call Policy.compute(History) and return seconds as a float. - auto Compute = [&](llvm::ArrayRef History) { - using FloatingSeconds = c::duration; - return static_cast(Policy.compute(History) / FloatingSeconds(1)); - }; - EXPECT_NEAR(10, Compute(History), 0.01) << "(upper) median = 10"; - Policy.RebuildRatio = 1.5; - EXPECT_NEAR(15, Compute(History), 0.01) << "median = 10, ratio = 1.5"; - Policy.RebuildRatio = 3; - EXPECT_NEAR(25, Compute(History), 0.01) << "constrained by max"; - Policy.RebuildRatio = 0; - EXPECT_NEAR(3, Compute(History), 0.01) << "constrained by min"; - EXPECT_NEAR(25, Compute({}), 0.01) << "no history -> max"; -} - } // namespace } // namespace clangd } // namespace clang