This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE325480: [clangd] Tracing: name worker threads, and enforce
naming scheduled async tasks (authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43388?vs=134617&id=134868#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43388
Files:
clangd/ClangdServer.cpp
clangd/TUScheduler.cpp
clangd/TUScheduler.h
clangd/Threading.cpp
clangd/Threading.h
clangd/Trace.cpp
clangd/Trace.h
unittests/clangd/TUSchedulerTests.cpp
unittests/clangd/ThreadingTests.cpp
Index: unittests/clangd/ThreadingTests.cpp
===================================================================
--- unittests/clangd/ThreadingTests.cpp
+++ unittests/clangd/ThreadingTests.cpp
@@ -29,7 +29,7 @@
AsyncTaskRunner Tasks;
auto scheduleIncrements = [&]() {
for (int TaskI = 0; TaskI < TasksCnt; ++TaskI) {
- Tasks.runAsync([&Counter, &Mutex, IncrementsPerTask]() {
+ Tasks.runAsync("task", [&Counter, &Mutex, IncrementsPerTask]() {
for (int Increment = 0; Increment < IncrementsPerTask; ++Increment) {
std::lock_guard<std::mutex> Lock(Mutex);
++Counter;
Index: unittests/clangd/TUSchedulerTests.cpp
===================================================================
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -54,31 +54,33 @@
// Assert each operation for missing file is an error (even if it's available
// in VFS).
- S.runWithAST(Missing, [&](llvm::Expected<InputsAndAST> AST) {
+ S.runWithAST("", Missing, [&](llvm::Expected<InputsAndAST> AST) {
ASSERT_FALSE(bool(AST));
ignoreError(AST.takeError());
});
- S.runWithPreamble(Missing, [&](llvm::Expected<InputsAndPreamble> Preamble) {
- ASSERT_FALSE(bool(Preamble));
- ignoreError(Preamble.takeError());
- });
+ S.runWithPreamble("", Missing,
+ [&](llvm::Expected<InputsAndPreamble> Preamble) {
+ ASSERT_FALSE(bool(Preamble));
+ ignoreError(Preamble.takeError());
+ });
// remove() shouldn't crash on missing files.
S.remove(Missing);
// Assert there aren't any errors for added file.
- S.runWithAST(
- Added, [&](llvm::Expected<InputsAndAST> AST) { EXPECT_TRUE(bool(AST)); });
- S.runWithPreamble(Added, [&](llvm::Expected<InputsAndPreamble> Preamble) {
+ S.runWithAST("", Added, [&](llvm::Expected<InputsAndAST> AST) {
+ EXPECT_TRUE(bool(AST));
+ });
+ S.runWithPreamble("", Added, [&](llvm::Expected<InputsAndPreamble> Preamble) {
EXPECT_TRUE(bool(Preamble));
});
S.remove(Added);
// Assert that all operations fail after removing the file.
- S.runWithAST(Added, [&](llvm::Expected<InputsAndAST> AST) {
+ S.runWithAST("", Added, [&](llvm::Expected<InputsAndAST> AST) {
ASSERT_FALSE(bool(AST));
ignoreError(AST.takeError());
});
- S.runWithPreamble(Added, [&](llvm::Expected<InputsAndPreamble> Preamble) {
+ S.runWithPreamble("", Added, [&](llvm::Expected<InputsAndPreamble> Preamble) {
ASSERT_FALSE(bool(Preamble));
ignoreError(Preamble.takeError());
});
@@ -143,24 +145,27 @@
{
WithContextValue WithNonce(NonceKey, ++Nonce);
- S.runWithAST(File, [Inputs, Nonce, &Mut, &TotalASTReads](
- llvm::Expected<InputsAndAST> AST) {
- EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
-
- ASSERT_TRUE((bool)AST);
- EXPECT_EQ(AST->Inputs.FS, Inputs.FS);
- EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
-
- std::lock_guard<std::mutex> Lock(Mut);
- ++TotalASTReads;
- });
+ S.runWithAST("CheckAST", File,
+ [Inputs, Nonce, &Mut,
+ &TotalASTReads](llvm::Expected<InputsAndAST> AST) {
+ EXPECT_THAT(Context::current().get(NonceKey),
+ Pointee(Nonce));
+
+ ASSERT_TRUE((bool)AST);
+ EXPECT_EQ(AST->Inputs.FS, Inputs.FS);
+ EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
+
+ std::lock_guard<std::mutex> Lock(Mut);
+ ++TotalASTReads;
+ });
}
{
WithContextValue WithNonce(NonceKey, ++Nonce);
S.runWithPreamble(
- File, [Inputs, Nonce, &Mut, &TotalPreambleReads](
- llvm::Expected<InputsAndPreamble> Preamble) {
+ "CheckPreamble", File,
+ [Inputs, Nonce, &Mut, &TotalPreambleReads](
+ llvm::Expected<InputsAndPreamble> Preamble) {
EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
ASSERT_TRUE((bool)Preamble);
Index: clangd/Threading.cpp
===================================================================
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -34,7 +34,8 @@
[&] { return InFlightTasks == 0; });
}
-void AsyncTaskRunner::runAsync(UniqueFunction<void()> Action) {
+void AsyncTaskRunner::runAsync(llvm::Twine Name,
+ UniqueFunction<void()> Action) {
{
std::lock_guard<std::mutex> Lock(Mutex);
++InFlightTasks;
@@ -51,13 +52,14 @@
});
std::thread(
- [](decltype(Action) Action, decltype(CleanupTask)) {
+ [](std::string Name, decltype(Action) Action, decltype(CleanupTask)) {
+ llvm::set_thread_name(Name);
Action();
// Make sure function stored by Action is destroyed before CleanupTask
// is run.
Action = nullptr;
},
- std::move(Action), std::move(CleanupTask))
+ Name.str(), std::move(Action), std::move(CleanupTask))
.detach();
}
Index: clangd/Trace.h
===================================================================
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -82,7 +82,7 @@
/// SomeJSONExpr is evaluated and copied only if actually needed.
class Span {
public:
- Span(llvm::StringRef Name);
+ Span(llvm::Twine Name);
~Span();
/// Mutable metadata, if this span is interested.
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -44,8 +44,10 @@
#include "TUScheduler.h"
#include "Logger.h"
+#include "Trace.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "llvm/Support/Errc.h"
+#include "llvm/Support/Path.h"
#include <memory>
#include <queue>
#include <thread>
@@ -67,22 +69,24 @@
/// worker.
class ASTWorker {
friend class ASTWorkerHandle;
- ASTWorker(Semaphore &Barrier, CppFile AST, bool RunSync);
+ ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST,
+ bool RunSync);
public:
/// Create a new ASTWorker and return a handle to it.
/// The processing thread is spawned using \p Tasks. However, when \p Tasks
/// is null, all requests will be processed on the calling thread
/// synchronously instead. \p Barrier is acquired when processing each
/// request, it is be used to limit the number of actively running threads.
- static ASTWorkerHandle Create(AsyncTaskRunner *Tasks, Semaphore &Barrier,
- CppFile AST);
+ static ASTWorkerHandle Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
+ Semaphore &Barrier, CppFile AST);
~ASTWorker();
void update(ParseInputs Inputs,
UniqueFunction<void(llvm::Optional<std::vector<DiagWithFixIts>>)>
OnUpdated);
- void runWithAST(UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action);
+ void runWithAST(llvm::StringRef Name,
+ UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action);
bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr<const PreambleData> getPossiblyStalePreamble() const;
@@ -96,11 +100,16 @@
/// Signal that run() should finish processing pending requests and exit.
void stop();
/// Adds a new task to the end of the request queue.
- void startTask(UniqueFunction<void()> Task, bool isUpdate,
- llvm::Optional<CancellationFlag> CF);
+ void startTask(llvm::StringRef Name, UniqueFunction<void()> Task,
+ bool isUpdate, llvm::Optional<CancellationFlag> CF);
- using RequestWithCtx = std::pair<UniqueFunction<void()>, Context>;
+ struct Request {
+ UniqueFunction<void()> Action;
+ std::string Name;
+ Context Ctx;
+ };
+ std::string File;
const bool RunSync;
Semaphore &Barrier;
// AST and FileInputs are only accessed on the processing thread from run().
@@ -114,7 +123,7 @@
std::size_t LastASTSize; /* GUARDED_BY(Mutex) */
// Set to true to signal run() to finish processing.
bool Done; /* GUARDED_BY(Mutex) */
- std::queue<RequestWithCtx> Requests; /* GUARDED_BY(Mutex) */
+ std::queue<Request> Requests; /* GUARDED_BY(Mutex) */
// Only set when last request is an update. This allows us to cancel an update
// that was never read, if a subsequent update comes in.
llvm::Optional<CancellationFlag> LastUpdateCF; /* GUARDED_BY(Mutex) */
@@ -163,18 +172,21 @@
std::shared_ptr<ASTWorker> Worker;
};
-ASTWorkerHandle ASTWorker::Create(AsyncTaskRunner *Tasks, Semaphore &Barrier,
- CppFile AST) {
+ASTWorkerHandle ASTWorker::Create(llvm::StringRef File, AsyncTaskRunner *Tasks,
+ Semaphore &Barrier, CppFile AST) {
std::shared_ptr<ASTWorker> Worker(
- new ASTWorker(Barrier, std::move(AST), /*RunSync=*/!Tasks));
+ new ASTWorker(File, Barrier, std::move(AST), /*RunSync=*/!Tasks));
if (Tasks)
- Tasks->runAsync([Worker]() { Worker->run(); });
+ Tasks->runAsync("worker:" + llvm::sys::path::filename(File),
+ [Worker]() { Worker->run(); });
return ASTWorkerHandle(std::move(Worker));
}
-ASTWorker::ASTWorker(Semaphore &Barrier, CppFile AST, bool RunSync)
- : RunSync(RunSync), Barrier(Barrier), AST(std::move(AST)), Done(false) {
+ASTWorker::ASTWorker(llvm::StringRef File, Semaphore &Barrier, CppFile AST,
+ bool RunSync)
+ : File(File), RunSync(RunSync), Barrier(Barrier), AST(std::move(AST)),
+ Done(false) {
if (RunSync)
return;
}
@@ -212,11 +224,12 @@
};
CancellationFlag UpdateCF;
- startTask(BindWithForward(Task, UpdateCF, std::move(OnUpdated)),
+ startTask("Update", BindWithForward(Task, UpdateCF, std::move(OnUpdated)),
/*isUpdate=*/true, UpdateCF);
}
void ASTWorker::runWithAST(
+ llvm::StringRef Name,
UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action) {
auto Task = [=](decltype(Action) Action) {
ParsedAST *ActualAST = AST.getAST();
@@ -233,8 +246,8 @@
LastASTSize = ActualAST->getUsedBytes();
};
- startTask(BindWithForward(Task, std::move(Action)), /*isUpdate=*/false,
- llvm::None);
+ startTask(Name, BindWithForward(Task, std::move(Action)),
+ /*isUpdate=*/false, llvm::None);
}
std::shared_ptr<const PreambleData>
@@ -257,13 +270,14 @@
RequestsCV.notify_all();
}
-void ASTWorker::startTask(UniqueFunction<void()> Task, bool isUpdate,
- llvm::Optional<CancellationFlag> CF) {
+void ASTWorker::startTask(llvm::StringRef Name, UniqueFunction<void()> Task,
+ bool isUpdate, llvm::Optional<CancellationFlag> CF) {
assert(isUpdate == CF.hasValue() &&
"Only updates are expected to pass CancellationFlag");
if (RunSync) {
assert(!Done && "running a task after stop()");
+ trace::Span Tracer(Name + ":" + llvm::sys::path::filename(File));
Task();
return;
}
@@ -281,14 +295,14 @@
} else {
LastUpdateCF = llvm::None;
}
- Requests.emplace(std::move(Task), Context::current().clone());
+ Requests.push({std::move(Task), Name, Context::current().clone()});
} // unlock Mutex.
RequestsCV.notify_all();
}
void ASTWorker::run() {
while (true) {
- RequestWithCtx Req;
+ Request Req;
{
std::unique_lock<std::mutex> Lock(Mutex);
RequestsCV.wait(Lock, [&]() { return Done || !Requests.empty(); });
@@ -303,9 +317,12 @@
// Leave it on the queue for now, so waiters don't see an empty queue.
} // unlock Mutex
- std::lock_guard<Semaphore> BarrierLock(Barrier);
- WithContext Guard(std::move(Req.second));
- Req.first();
+ {
+ std::lock_guard<Semaphore> BarrierLock(Barrier);
+ WithContext Guard(std::move(Req.Ctx));
+ trace::Span Tracer(Req.Name);
+ Req.Action();
+ }
{
std::lock_guard<std::mutex> Lock(Mutex);
@@ -379,7 +396,7 @@
if (!FD) {
// Create a new worker to process the AST-related tasks.
ASTWorkerHandle Worker = ASTWorker::Create(
- WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
+ File, WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier,
CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback));
FD = std::unique_ptr<FileData>(new FileData{Inputs, std::move(Worker)});
} else {
@@ -396,20 +413,21 @@
}
void TUScheduler::runWithAST(
- PathRef File, UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action) {
+ llvm::StringRef Name, PathRef File,
+ UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action) {
auto It = Files.find(File);
if (It == Files.end()) {
Action(llvm::make_error<llvm::StringError>(
"trying to get AST for non-added document",
llvm::errc::invalid_argument));
return;
}
- It->second->Worker->runWithAST(std::move(Action));
+ It->second->Worker->runWithAST(Name, std::move(Action));
}
void TUScheduler::runWithPreamble(
- PathRef File,
+ llvm::StringRef Name, PathRef File,
UniqueFunction<void(llvm::Expected<InputsAndPreamble>)> Action) {
auto It = Files.find(File);
if (It == Files.end()) {
@@ -420,25 +438,32 @@
}
if (!PreambleTasks) {
+ trace::Span Tracer(Name);
+ SPAN_ATTACH(Tracer, "file", File);
std::shared_ptr<const PreambleData> Preamble =
It->second->Worker->getPossiblyStalePreamble();
Action(InputsAndPreamble{It->second->Inputs, Preamble.get()});
return;
}
ParseInputs InputsCopy = It->second->Inputs;
std::shared_ptr<const ASTWorker> Worker = It->second->Worker.lock();
- auto Task = [InputsCopy, Worker, this](Context Ctx,
+ auto Task = [InputsCopy, Worker, this](std::string Name, std::string File,
+ Context Ctx,
decltype(Action) Action) mutable {
std::lock_guard<Semaphore> BarrierLock(Barrier);
WithContext Guard(std::move(Ctx));
+ trace::Span Tracer(Name);
+ SPAN_ATTACH(Tracer, "file", File);
std::shared_ptr<const PreambleData> Preamble =
Worker->getPossiblyStalePreamble();
Action(InputsAndPreamble{InputsCopy, Preamble.get()});
};
PreambleTasks->runAsync(
- BindWithForward(Task, Context::current().clone(), std::move(Action)));
+ "task:" + llvm::sys::path::filename(File),
+ BindWithForward(Task, std::string(Name), std::string(File),
+ Context::current().clone(), std::move(Action)));
}
std::vector<std::pair<Path, std::size_t>>
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -179,9 +179,10 @@
Callback(make_tagged(std::move(Result), std::move(TaggedFS.Tag)));
};
- WorkScheduler.runWithPreamble(File, BindWithForward(Task, std::move(Contents),
- File.str(),
- std::move(Callback)));
+ WorkScheduler.runWithPreamble("CodeComplete", File,
+ BindWithForward(Task, std::move(Contents),
+ File.str(),
+ std::move(Callback)));
}
void ClangdServer::signatureHelp(
@@ -222,7 +223,8 @@
};
WorkScheduler.runWithPreamble(
- File, BindWithForward(Action, File.str(), std::move(Callback)));
+ "SignatureHelp", File,
+ BindWithForward(Action, File.str(), std::move(Callback)));
}
llvm::Expected<tooling::Replacements>
@@ -307,7 +309,7 @@
};
WorkScheduler.runWithAST(
- File,
+ "Rename", File,
BindWithForward(Action, File.str(), NewName.str(), std::move(Callback)));
}
@@ -376,7 +378,8 @@
Callback(Result);
};
- WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback)));
+ WorkScheduler.runWithAST("DumpAST", File,
+ BindWithForward(Action, std::move(Callback)));
}
void ClangdServer::findDefinitions(
@@ -392,7 +395,8 @@
Callback(make_tagged(std::move(Result), TaggedFS.Tag));
};
- WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback)));
+ WorkScheduler.runWithAST("Definitions", File,
+ BindWithForward(Action, std::move(Callback)));
}
llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
@@ -488,7 +492,8 @@
Callback(make_tagged(std::move(Result), TaggedFS.Tag));
};
- WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback)));
+ WorkScheduler.runWithAST("Highlights", File,
+ BindWithForward(Action, std::move(Callback)));
}
void ClangdServer::findHover(
@@ -511,7 +516,8 @@
Callback(make_tagged(std::move(Result), TaggedFS.Tag));
};
- WorkScheduler.runWithAST(File, BindWithForward(Action, std::move(Callback)));
+ WorkScheduler.runWithAST("Hover", File,
+ BindWithForward(Action, std::move(Callback)));
}
void ClangdServer::scheduleReparseAndDiags(
Index: clangd/Threading.h
===================================================================
--- clangd/Threading.h
+++ clangd/Threading.h
@@ -12,6 +12,7 @@
#include "Context.h"
#include "Function.h"
+#include "llvm/ADT/Twine.h"
#include <atomic>
#include <cassert>
#include <condition_variable>
@@ -81,7 +82,8 @@
void wait() const { (void) wait(llvm::None); }
LLVM_NODISCARD bool wait(Deadline D) const;
- void runAsync(UniqueFunction<void()> Action);
+ // The name is used for tracing and debugging (e.g. to name a spawned thread).
+ void runAsync(llvm::Twine Name, UniqueFunction<void()> Action);
private:
mutable std::mutex Mutex;
Index: clangd/Trace.cpp
===================================================================
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -208,17 +208,19 @@
}
// Returned context owns Args.
-static Context makeSpanContext(llvm::StringRef Name, json::obj *Args) {
+static Context makeSpanContext(llvm::Twine Name, json::obj *Args) {
if (!T)
return Context::current().clone();
WithContextValue WithArgs{std::unique_ptr<json::obj>(Args)};
- return T->beginSpan(Name, Args);
+ return T->beginSpan(Name.isSingleStringRef() ? Name.getSingleStringRef()
+ : llvm::StringRef(Name.str()),
+ Args);
}
// Span keeps a non-owning pointer to the args, which is how users access them.
// The args are owned by the context though. They stick around until the
// beginSpan() context is destroyed, when the tracing engine will consume them.
-Span::Span(llvm::StringRef Name)
+Span::Span(llvm::Twine Name)
: Args(T ? new json::obj() : nullptr),
RestoreCtx(makeSpanContext(Name, Args)) {}
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -37,7 +37,7 @@
/// TUScheduler is not thread-safe, only one thread should be providing updates
/// and scheduling tasks.
/// Callbacks are run on a threadpool and it's appropriate to do slow work in
-/// them.
+/// them. Each task has a name, used for tracing (should be UpperCamelCase).
class TUScheduler {
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
@@ -65,16 +65,16 @@
/// \p Action is executed.
/// If an error occurs during processing, it is forwarded to the \p Action
/// callback.
- void runWithAST(PathRef File,
+ void runWithAST(llvm::StringRef Name, PathRef File,
UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action);
/// Schedule an async read of the Preamble. Preamble passed to \p Action may
/// be built for any version of the file, callers must not rely on it being
/// consistent with the current version of the file.
/// If an error occurs during processing, it is forwarded to the \p Action
/// callback.
void runWithPreamble(
- PathRef File,
+ llvm::StringRef Name, PathRef File,
UniqueFunction<void(llvm::Expected<InputsAndPreamble>)> Action);
/// Wait until there are no scheduled or running tasks.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
