[lldb] Cache Task pointer location and Task names #10894
Conversation
felipepiovezan
force-pushed
the
felipe/cache_tls
branch
from
20192af to
654b520
Compare
…tTasks Obtaining the TLS from debugserver over a wired (or slower) conection was identified as a major source of slowdown after introducing OperatingSystemSwiftTasks. This is due to multiple memory reads being necessary: 1. Memory reads to obtain auxiliary information from libpthread that is then sent to the jThreadExtendedInfo packet. See "plo_pthread_tsd_base_address_offset","plo_pthread_tsd_base_offset" and "plo_pthread_tsd_entry_size". 2. A packet requesting jThreadExtendedInfo. Both 1 and 2 are done per thread, on every stop. However, there is strong evidence that we can safely cache the TLS pointer after a certain point in the thread lifetime, which is what this patch accomplishes. Caching is done at the OperatingSystemSwiftTasks level, not at the core Thread class. This is done for two reasons: 1. To minimize the surface area / risk of this change. 2. jThreadExtendedInfo can produce an invalid TLS address earlier in the thread lifetime, so we can only cache this if a memory read at that location is successful. On local testing over a wired connection, this reduces stepping time by about 100ms, from an average of 300ms.
This is a useful fallback for debugging.
felipepiovezan
force-pushed
the
felipe/cache_tls
branch
2 times, most recently
from
4b86b4f to
4d4f808
Compare
…SwiftTasks 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.
felipepiovezan
force-pushed
the
felipe/cache_tls
branch
from
4d4f808 to
5da29ab
Compare
|
@swift-ci test |
|
|
||
| if (auto it = m_tid_to_task_addr_location.find(real_thread.GetID()); | ||
| it != m_tid_to_task_addr_location.end()) { | ||
| #ifndef NDEBUG |
There was a problem hiding this comment.
The asserts look unlikely to break, if they're going to make asserts builds slow, should this be #ifdef EXPENSIVE_CHECKS instead?
There was a problem hiding this comment.
The asserts look unlikely to break
The uncertainty here is sort of why we thought of putting the assert: can we truly cache TLS? Nobody I talked to was able to give me a definitive answer, and experimenting failed to produce counter examples... Looking at the code of libpthread also did not produce a good definitive answer :/
they're going to make asserts builds slow,
Sort of! They will be "slow" for slow connections only. On a wired connection, the slowdown we're talking about is a step operating going from 200ms to 300ms.
Completely disabling the Task plugin would make the step be about 100ms.
| #endif | ||
| return ReadTaskAddr(it->second, *thread.GetProcess()); |
There was a problem hiding this comment.
if this read fails, should the cache entry be evicted?
This PR reduces the number of packets that need to be communicated between debugserver and LLDB when deciding whether a thread is executing a task. This is accomplished in two ways:
A) Caching the TLS location of the task pointer for each real thread.
B) Caching and lazily evaluating the name of a task
rdar://151400459