sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, javed.absar.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Building preambles is the most resource-intensive thing clangd does, driving
peak RAM and sustained CPU usage.
In a hosted environment where multiple clangd instances are packed into the same
container, it's useful to be able to limit the *aggregate* resource peaks.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129100
Files:
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
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1372,6 +1372,106 @@
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<std::string> Acquires;
+ std::vector<std::string> Releases;
+ std::vector<std::string> Cancellations;
+ std::vector<std::pair<std::string, AcquireCallback>> PendingRequests;
+
+ CancelFn acquire(llvm::StringRef Filename,
+ AcquireCallback Callback) override {
+ // Don't honor cancellations, but record them.
+ auto Cancel = [Filename(std::string(Filename)), this] {
+ Cancellations.push_back(Filename);
+ };
+ {
+ std::lock_guard<std::mutex> Lock(Mu);
+ // Record the order we saw acquires.
+ Acquires.emplace_back(Filename);
+ // If our queue isn't full yet, keep queueing.
+ if (PendingRequests.size() + 1 < NumRequests) {
+ PendingRequests.emplace_back(Filename, std::move(Callback));
+ return Cancel;
+ }
+ }
+ // OK, we're up to our request limit, allow this last request to proceed.
+ Callback(/*Release=*/[&, Filename(std::string(Filename))] {
+ this->release(Filename);
+ });
+ return Cancel;
+ }
+
+ void reset() {
+ Acquires.clear();
+ Releases.clear();
+ Cancellations.clear();
+ PendingRequests.clear();
+ }
+
+ private:
+ // When one request finishes, allow the next one to proceed.
+ void release(llvm::StringRef Filename) {
+ std::pair<std::string, AcquireCallback> Next;
+ {
+ std::lock_guard<std::mutex> Lock(Mu);
+ Releases.emplace_back(Filename);
+ if (!PendingRequests.empty()) {
+ Next = std::move(PendingRequests.back());
+ PendingRequests.pop_back();
+ }
+ }
+ if (Next.second)
+ Next.second([&, Filename(std::string(Next.first))] {
+ this->release(Filename);
+ });
+ }
+ } Throttler;
+
+ struct CaptureBuiltFilenames : public ParsingCallbacks {
+ std::vector<std::string> &Filenames;
+ CaptureBuiltFilenames(std::vector<std::string> &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<std::string> BuiltFilenames;
+ TUScheduler S(CDB, Opts,
+ std::make_unique<CaptureBuiltFilenames>(BuiltFilenames));
+ for (unsigned I = 0; I < NumRequests; ++I) {
+ auto Path = testPath(std::to_string(I) + ".cc");
+ S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes);
+ }
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+
+ // Now that we're done, we expect to see preambles were built in reverse.
+ EXPECT_THAT(Throttler.PendingRequests, testing::IsEmpty());
+ EXPECT_THAT(Throttler.Acquires, testing::SizeIs(NumRequests));
+ EXPECT_THAT(BuiltFilenames,
+ testing::ElementsAreArray(Throttler.Acquires.rbegin(),
+ Throttler.Acquires.rend()));
+ EXPECT_EQ(BuiltFilenames, Throttler.Releases);
+ EXPECT_THAT(Throttler.Cancellations, testing::IsEmpty());
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.h
===================================================================
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -87,9 +87,40 @@
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 ReleaseFn = llvm::unique_function<void()>;
+ using AcquireCallback = llvm::unique_function<void(ReleaseFn)>;
+ using CancelFn = llvm::unique_function<void()>;
+
+ // Attempts to acquire resources to parse the file specified by params.
+ //
+ // Does not block, but eventually invokes AcquireCallback on another thread.
+ // The AcquireCallback delivers resources, which must be released exactly once
+ // by calling the ReleaseFn.
+ //
+ // Calling the returned CancelFn attempts to abort the acquire(), best-effort.
+ // If successful the AcquireCallback will not be invoked.
+ // If it fails (e.g. races), resources are acquired and must be released.
+ virtual CancelFn acquire(llvm::StringRef Filename, AcquireCallback) = 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 +231,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".
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -389,12 +389,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 +427,83 @@
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<void()> Release) {
+ {
+ std::lock_guard<std::mutex> 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<std::mutex> 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<void()> Release = nullptr;
+ std::mutex Mu;
+ std::atomic<bool> Ready = {false};
+ };
+
void run() {
while (true) {
+ auto TState = std::make_shared<ThrottleState>(ReqCV);
+ auto Abandon = llvm::make_scope_exit([TState] { TState->abandon(); });
{
std::unique_lock<std::mutex> 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;
+
+ // Acquire permission from the throttler.
+ // Note the callback might outlive this whole TUScheduler!
+ if (Throttler) {
+ auto Cancel = Throttler->acquire(
+ FileName, [TState](PreambleThrottler::ReleaseFn Release) {
+ TState->notifyReady(std::move(Release));
+ });
+ // If acquire succeeded synchronously, avoid status jitter.
+ if (!TState->ready())
+ Status.update([&](TUStatus &Status) {
+ Status.PreambleActivity = PreambleAction::Queued;
+ });
+
+ ReqCV.wait(Lock, [&] { return TState->ready() || Done; });
+ if (Done) {
+ Cancel();
+ break;
+ }
+ }
+
CurrentReq = std::move(*NextReq);
NextReq.reset();
}
@@ -518,6 +587,7 @@
ParsingCallbacks &Callbacks;
const bool StoreInMemory;
const bool RunSync;
+ PreambleThrottler *Throttler;
SynchronizedTUStatus &Status;
ASTWorker &ASTPeer;
@@ -778,7 +848,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 +1569,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;
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ 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;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -166,6 +166,7 @@
Opts.StorePreamblesInMemory = StorePreamblesInMemory;
Opts.UpdateDebounce = UpdateDebounce;
Opts.ContextProvider = ContextProvider;
+ Opts.PreambleThrottler = PreambleThrottler;
return Opts;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
