Skip to content

Commit 5da29ab

Browse files
[lldb][swift] Cache and lazily-evaluate Task names in OperatingSystemSwiftTasks
Computing a Task name can be an expensive operation, especially over slower connections to the inferior process. To improve this situation, this patch: * Makes the computation lazy, and * Caches the result of the computation (a task name is immutable). The first point is the most important, as prior to this we were computing the task name on every stop, even when the same MemoryThread was reused between two consecutive stops. On local testing with a wired connection, this reduces stepping time by around 20-30ms, out of an average of 230ms.
1 parent 45d8e62 commit 5da29ab

File tree

2 files changed

+58
-30
lines changed

2 files changed

+58
-30
lines changed

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,55 @@ void OperatingSystemSwiftTasks::Terminate() {
3838
PluginManager::UnregisterPlugin(CreateInstance);
3939
}
4040

41+
/// A wrapper around ThreadMemory providing lazy name evaluation, as this is
42+
/// expensive to compute for Swift Tasks.
43+
class SwiftTaskThreadMemory : public ThreadMemory {
44+
public:
45+
SwiftTaskThreadMemory(lldb_private::Process &process, lldb::tid_t tid,
46+
lldb::addr_t register_data_addr)
47+
: ThreadMemory(process, tid, register_data_addr) {}
48+
49+
/// Updates the backing thread of this Task, as well as the location where the
50+
/// task pointer is stored.
51+
void UpdateBackingThread(const ThreadSP &new_backing_thread,
52+
lldb::addr_t task_addr) {
53+
SetBackingThread(new_backing_thread);
54+
m_task_addr = task_addr;
55+
}
56+
57+
const char *GetName() override {
58+
if (m_task_name.empty())
59+
m_task_name = FindTaskName();
60+
return m_task_name.c_str();
61+
}
62+
63+
private:
64+
std::string GetDefaultTaskName() const {
65+
return llvm::formatv("Task {0}", GetID());
66+
}
67+
68+
/// If possible, read a user-provided task name from memory, otherwise use a
69+
/// default name. This never returns an empty string.
70+
std::string FindTaskName() const {
71+
llvm::Expected<std::optional<std::string>> task_name =
72+
GetTaskName(m_task_addr, *GetProcess());
73+
if (auto err = task_name.takeError()) {
74+
LLDB_LOG_ERROR(GetLog(LLDBLog::OS), std::move(err),
75+
"OperatingSystemSwiftTasks: failed while looking for name "
76+
"of task {1:x}: {0}",
77+
m_task_addr);
78+
return GetDefaultTaskName();
79+
}
80+
81+
if (!task_name->has_value())
82+
return GetDefaultTaskName();
83+
return llvm::formatv("{0} (Task {1})", *task_name, GetID());
84+
}
85+
86+
std::string m_task_name = "";
87+
lldb::addr_t m_task_addr = LLDB_INVALID_ADDRESS;
88+
};
89+
4190
OperatingSystem *OperatingSystemSwiftTasks::CreateInstance(Process *process,
4291
bool force) {
4392
if (!process || !process->GetTarget().GetSwiftUseTasksPlugin())
@@ -78,9 +127,9 @@ OperatingSystemSwiftTasks::OperatingSystemSwiftTasks(
78127
lldb_private::Process &process)
79128
: OperatingSystem(&process) {}
80129

81-
ThreadSP OperatingSystemSwiftTasks::FindOrCreateSwiftThread(
82-
ThreadList &old_thread_list, uint64_t task_id,
83-
std::optional<std::string> task_name) {
130+
ThreadSP
131+
OperatingSystemSwiftTasks::FindOrCreateSwiftThread(ThreadList &old_thread_list,
132+
uint64_t task_id) {
84133
// Mask higher bits to avoid conflicts with core thread IDs.
85134
uint64_t masked_task_id = 0x0000000f00000000 | task_id;
86135

@@ -89,15 +138,8 @@ ThreadSP OperatingSystemSwiftTasks::FindOrCreateSwiftThread(
89138
IsOperatingSystemPluginThread(old_thread))
90139
return old_thread;
91140

92-
std::string name;
93-
if (task_name)
94-
name = llvm::formatv("{0} (Task {1})", *task_name, task_id);
95-
else
96-
name = llvm::formatv("Task {0}", task_id);
97-
98-
return std::make_shared<ThreadMemoryProvidingName>(*m_process, masked_task_id,
99-
/*register_data_addr*/ 0,
100-
name);
141+
return std::make_shared<SwiftTaskThreadMemory>(*m_process, masked_task_id,
142+
/*register_data_addr*/ 0);
101143
}
102144

103145
static std::optional<addr_t> FindTaskAddress(TaskInspector &task_inspector,
@@ -132,19 +174,6 @@ static std::optional<uint64_t> FindTaskId(addr_t task_addr, Process &process) {
132174
return task_id;
133175
}
134176

135-
static std::optional<std::string> FindTaskName(addr_t task_addr,
136-
Process &process) {
137-
auto task_name_or_err = GetTaskName(task_addr, process);
138-
if (auto err = task_name_or_err.takeError()) {
139-
LLDB_LOG_ERROR(GetLog(LLDBLog::OS), std::move(err),
140-
"OperatingSystemSwiftTasks: failed while looking for name "
141-
"of task {1:x}: {0}",
142-
task_addr);
143-
return {};
144-
}
145-
return *task_name_or_err;
146-
}
147-
148177
bool OperatingSystemSwiftTasks::UpdateThreadList(ThreadList &old_thread_list,
149178
ThreadList &core_thread_list,
150179
ThreadList &new_thread_list) {
@@ -173,9 +202,9 @@ bool OperatingSystemSwiftTasks::UpdateThreadList(ThreadList &old_thread_list,
173202
continue;
174203
}
175204

176-
ThreadSP swift_thread = FindOrCreateSwiftThread(
177-
old_thread_list, *task_id, FindTaskName(*task_addr, *m_process));
178-
swift_thread->SetBackingThread(real_thread);
205+
ThreadSP swift_thread = FindOrCreateSwiftThread(old_thread_list, *task_id);
206+
static_cast<SwiftTaskThreadMemory &>(*swift_thread)
207+
.UpdateBackingThread(real_thread, *task_addr);
179208
new_thread_list.AddThread(swift_thread);
180209
LLDB_LOGF(log,
181210
"OperatingSystemSwiftTasks: mapping thread IDs: %" PRIx64

lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ class OperatingSystemSwiftTasks : public OperatingSystem {
5050
/// If a thread for task_id had been created in the last stop, return it.
5151
/// Otherwise, create a new MemoryThread for it.
5252
lldb::ThreadSP FindOrCreateSwiftThread(ThreadList &old_thread_list,
53-
uint64_t task_id,
54-
std::optional<std::string> task_name);
53+
uint64_t task_id);
5554

5655
/// A cache for task addr locations, which are expensive to compute but
5756
/// immutable.

0 commit comments

Comments
 (0)