diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -104,6 +104,9 @@ /// Cached preambles are potentially large. If false, store them on disk. bool StorePreamblesInMemory = true; + /// This throttler controls which preambles may be built at a given time. + clangd::PreambleThrottler *PreambleThrottler = nullptr; + /// If true, ClangdServer builds a dynamic in-memory index for symbols in /// opened files and uses the index to augment code completion results. bool BuildDynamicSymbolIndex = false; diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -166,6 +166,7 @@ Opts.StorePreamblesInMemory = StorePreamblesInMemory; Opts.UpdateDebounce = UpdateDebounce; Opts.ContextProvider = ContextProvider; + Opts.PreambleThrottler = PreambleThrottler; return Opts; } diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -87,9 +87,39 @@ static DebouncePolicy fixed(clock::duration); }; +// PreambleThrottler controls which preambles can build at any given time. +// This can be used to limit overall concurrency, and to prioritize some +// preambles over others. +// In a distributed environment, a throttler may be able to coordinate resource +// use across several clangd instances. +// +// This class is threadsafe. +class PreambleThrottler { +public: + virtual ~PreambleThrottler() = default; + + using RequestID = unsigned; + using Callback = llvm::unique_function; + // Attempt to acquire resources to build a file's preamble. + // + // Does not block, may eventually invoke the callback to satisfy the request. + // If cancel() is called, the callback will not be invoked afterwards. + // If the callback is invoked, release() must be called afterwards. + virtual RequestID acquire(llvm::StringRef Filename, Callback); + // Abandons the request/releases any resources that have been acquired. + // + // Must be called exactly once after acquire(). + // acquire()'s callback will not be invoked after release() returns. + virtual void release(RequestID) = 0; + + // FIXME: we may want to be able attach signals to filenames. + // this would allow the throttler to make better scheduling decisions. +}; + enum class PreambleAction { - Idle, + Queued, Building, + Idle, }; struct ASTAction { @@ -200,6 +230,9 @@ /// Determines when to keep idle ASTs in memory for future use. ASTRetentionPolicy RetentionPolicy; + /// This throttler controls which preambles may be built at a given time. + clangd::PreambleThrottler *PreambleThrottler = nullptr; + /// Used to create a context that wraps each single operation. /// Typically to inject per-file configuration. /// If the path is empty, context sholud be "generic". diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -381,6 +381,41 @@ ParsingCallbacks &Callbacks; }; +// An attempt to acquire resources for a task using PreambleThrottler. +// Initially it is unsatisfied, it (hopefully) becomes satisfied later but may +// be destroyed before then. Destruction releases all resources. +class PreambleThrottlerRequest { +public: + // The condition variable is signalled when the request is satisfied. + PreambleThrottlerRequest(llvm::StringRef Filename, + PreambleThrottler *Throttler, + std::condition_variable &CV) + : Throttler(Throttler), Satisfied(Throttler == nullptr) { + // If there is no throttler, this dummy request is always satisfied. + if (!Throttler) + return; + ID = Throttler->acquire(Filename, [&] { + Satisfied.store(true, std::memory_order_release); + CV.notify_all(); + }); + } + + bool satisfied() const { return Satisfied.load(std::memory_order_acquire); } + + // When the request is destroyed: + // - if resources are not yet obtained, stop trying to get them. + // - if resources were obtained, release them. + ~PreambleThrottlerRequest() { + if (Throttler) + Throttler->release(ID); + } + +private: + unsigned ID; + PreambleThrottler *Throttler; + std::atomic Satisfied = {false}; +}; + /// Responsible for building preambles. Whenever the thread is idle and the /// preamble is outdated, it starts to build a fresh preamble from the latest /// inputs. If RunSync is true, preambles are built synchronously in update() @@ -389,12 +424,13 @@ public: PreambleThread(llvm::StringRef FileName, ParsingCallbacks &Callbacks, bool StorePreambleInMemory, bool RunSync, - SynchronizedTUStatus &Status, + PreambleThrottler *Throttler, SynchronizedTUStatus &Status, TUScheduler::HeaderIncluderCache &HeaderIncluders, ASTWorker &AW) : FileName(FileName), Callbacks(Callbacks), - StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status), - ASTPeer(AW), HeaderIncluders(HeaderIncluders) {} + StoreInMemory(StorePreambleInMemory), RunSync(RunSync), + Throttler(Throttler), Status(Status), ASTPeer(AW), + HeaderIncluders(HeaderIncluders) {} /// It isn't guaranteed that each requested version will be built. If there /// are multiple update requests while building a preamble, only the last one @@ -426,15 +462,72 @@ ReqCV.notify_all(); } + // Helper to ensure we obey Throttler's API, calling Release even if this + // worker is completely destroyed before throttler lets us run. + class ThrottleState { + public: + ThrottleState(std::condition_variable &CV) : CV(&CV) {} + ~ThrottleState() { assert(CV == nullptr && "never abandoned?"); } + + // We have now acquired the resource. + // If we already abandoned this request, release the resource straight away. + // Else the release function will be called when we abandon the request. + void notifyReady(llvm::unique_function Release) { + { + std::lock_guard Lock(Mu); + assert(!this->Release && "multiple notifyReady?"); + if (CV != nullptr) { + Ready.store(true, std::memory_order_release); + this->Release = std::move(Release); + CV->notify_all(); + return; + } + } + Release(); + } + + bool ready() const { return Ready.load(std::memory_order_acquire); } + + // Give up on this request, releasing resources if any. + void abandon() { + std::lock_guard Lock(Mu); + assert(CV != nullptr && "multiple abandon?"); + CV = nullptr; + if (Release) + Release(); + } + + private: + // The external condition variable to signal when we acquire the resource. + // Cleared once this state has been abandoned (worker is shutting down). + std::condition_variable *CV; + // The release handler to call once we abandon the state (worker shutdown). + llvm::unique_function Release = nullptr; + std::mutex Mu; + std::atomic Ready = {false}; + }; + void run() { while (true) { + llvm::Optional Throttle; { std::unique_lock Lock(Mutex); assert(!CurrentReq && "Already processing a request?"); // Wait until stop is called or there is a request. - ReqCV.wait(Lock, [this] { return NextReq || Done; }); + ReqCV.wait(Lock, [&] { return NextReq || Done; }); + if (Done) + break; + + Throttle.emplace(FileName, Throttler, ReqCV); + // If acquire succeeded synchronously, avoid status jitter. + if (!Throttle->satisfied()) + Status.update([&](TUStatus &Status) { + Status.PreambleActivity = PreambleAction::Queued; + }); + ReqCV.wait(Lock, [&] { return Throttle->satisfied() || Done; }); if (Done) break; + CurrentReq = std::move(*NextReq); NextReq.reset(); } @@ -518,6 +611,7 @@ ParsingCallbacks &Callbacks; const bool StoreInMemory; const bool RunSync; + PreambleThrottler *Throttler; SynchronizedTUStatus &Status; ASTWorker &ASTPeer; @@ -778,7 +872,7 @@ ContextProvider(Opts.ContextProvider), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier), Done(false), Status(FileName, Callbacks), PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync, - Status, HeaderIncluders, *this) { + Opts.PreambleThrottler, Status, HeaderIncluders, *this) { // Set a fallback command because compile command can be accessed before // `Inputs` is initialized. Other fields are only used after initialization // from client inputs. @@ -1499,6 +1593,9 @@ case PreambleAction::Building: Result.push_back("parsing includes"); break; + case PreambleAction::Queued: + Result.push_back("headers are queued"); + break; case PreambleAction::Idle: // We handle idle specially below. break; diff --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp --- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -1372,6 +1372,141 @@ CheckNoFileActionsSeesLastActiveFile(LastActive); } } + +TEST_F(TUSchedulerTests, PreambleThrottle) { + const int NumRequests = 4; + // Silly throttler that waits for 4 requests, and services them in reverse. + // Doesn't honor cancellation but records it. + struct : public PreambleThrottler { + std::mutex Mu; + std::vector Acquires; + std::vector Releases; + llvm::DenseMap Callbacks; + // If set, the notification is signalled after acquiring the specified ID. + llvm::Optional> Notify; + + RequestID acquire(llvm::StringRef Filename, Callback CB) override { + RequestID ID; + Callback Invoke; + { + std::lock_guard Lock(Mu); + ID = Acquires.size(); + Acquires.emplace_back(Filename); + // If we're full, satisfy this request immediately. + if (Acquires.size() == NumRequests) { + Invoke = std::move(CB); + } else { + Callbacks.try_emplace(ID, std::move(CB)); + } + } + if (Invoke) + Invoke(); + if (Notify && ID == Notify->first) { + Notify->second->notify(); + Notify.reset(); + } + return ID; + } + + void release(RequestID ID) override { + Releases.push_back(ID); + Callback SatisfyNext; + { + std::lock_guard Lock(Mu); + if (ID > 0) + SatisfyNext = std::move(Callbacks[ID - 1]); + } + if (SatisfyNext) + SatisfyNext(); + } + + void reset() { + Acquires.clear(); + Releases.clear(); + Callbacks.clear(); + } + } Throttler; + + struct CaptureBuiltFilenames : public ParsingCallbacks { + std::vector &Filenames; + CaptureBuiltFilenames(std::vector &Filenames) + : Filenames(Filenames) {} + void onPreambleAST(PathRef Path, llvm::StringRef Version, + const CompilerInvocation &CI, ASTContext &Ctx, + Preprocessor &PP, const CanonicalIncludes &) override { + // Deliberately no synchronization. + // The PreambleThrottler should serialize these calls, if not then tsan + // will find a bug here. + Filenames.emplace_back(Path); + } + }; + + auto Opts = optsForTest(); + Opts.AsyncThreadsCount = 2 * NumRequests; // throttler is the bottleneck + Opts.PreambleThrottler = &Throttler; + + std::vector Filenames; + + { + std::vector BuiltFilenames; + TUScheduler S(CDB, Opts, + std::make_unique(BuiltFilenames)); + for (unsigned I = 0; I < NumRequests; ++I) { + auto Path = testPath(std::to_string(I) + ".cc"); + Filenames.push_back(Path); + S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes); + } + ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); + + // The throttler saw all files, and we built them. + EXPECT_THAT(Throttler.Acquires, + testing::UnorderedElementsAreArray(Filenames)); + EXPECT_THAT(BuiltFilenames, + testing::UnorderedElementsAreArray(Filenames)); + // We built the files in reverse order that the throttler saw them. + EXPECT_THAT(BuiltFilenames, + testing::ElementsAreArray(Throttler.Acquires.rbegin(), + Throttler.Acquires.rend())); + // Resources for each file were correctly released. + EXPECT_THAT(Throttler.Releases, ElementsAre(3, 2, 1, 0)); + } + + Throttler.reset(); + Filenames.clear(); + + // This time, enqueue 2 files, then cancel one of them while still waiting. + // Finally shut down the server. Observe that everything gets cleaned up. + Notification After2; + Throttler.Notify = {1, &After2}; + std::vector BuiltFilenames; + { + TUScheduler S(CDB, Opts, + std::make_unique(BuiltFilenames)); + for (unsigned I = 0; I < NumRequests / 2; ++I) { + auto Path = testPath(std::to_string(I) + ".cc"); + Filenames.push_back(Path); + S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes); + } + After2.wait(); + + // The throttler saw all files, but we built none. + EXPECT_THAT(Throttler.Acquires, + testing::UnorderedElementsAreArray(Filenames)); + EXPECT_THAT(BuiltFilenames, testing::IsEmpty()); + // We haven't released anything yet, we're still waiting. + EXPECT_THAT(Throttler.Releases, testing::IsEmpty()); + + S.remove(Filenames.back()); + // Now shut down the TU Scheduler. + } + // The throttler saw all files, but we built none. + EXPECT_THAT(Throttler.Acquires, + testing::UnorderedElementsAreArray(Filenames)); + EXPECT_THAT(BuiltFilenames, testing::IsEmpty()); + // We gave up waiting and everything got released (in some order). + EXPECT_THAT(Throttler.Releases, UnorderedElementsAre(1, 0)); +} + } // namespace } // namespace clangd } // namespace clang