[Lldb-commits] [PATCH] D75711: [NFC] Have ThreadPlans hold onto the Process & TID, rather than the Thread

Fri Mar 6 12:09:04 PST 2020

jingham added a comment.

In D75711#1910156 <https://reviews.llvm.org/D75711#1910156>, @clayborg wrote:

> As long as the keeping thread plans around for threads that aren't there is only enabled when OS plug-in is around, I am fine with this direction. One questions: do we currently only allow stepping on real threads or OS threads that are backed by real threads right now? And if an OS thread is backed by a real thread, do we associate the step plan with the OS thread ID or the real thread ID (the core)? If we use the OS thread ID, another way to potentially fix this is to know in a thread plan that we are stepping an OS thread, and avoid fetching the complete OS list of threads for non public stops, but only fetch the thread list for any OS threads that have active thread plans and any OS threads for threads that are active on a core. Then we don't end up fetching all OS threads all the time for stepping, but it does allow us to maintain and track our OS thread correctly. This would require changes to the OS plug-ins where getting the thread list needs to be able to grab only the OS threads that are on core, and we can manually fetch the OS thread info for any thread plans, and the plug-in would still need to be able to fetch the complete list on public stop events.

The way I have it in another patch in the series is when UpdateThreadList decides a thread is not present, I ask whether there is an OS Plugin, if there is I preserve the ThreadPlanStack while deleting the Thread, and if there isn't I ditch the ThreadPlanStack along with the Thread.  We could do other things if we want to get cleverer, but that's what I do at present.

Core threads are really the fiction in the case of the xnu OS Plugin.  They are just the activations that happen to be on a processor when something stops.  You can't do anything persistent with them because an activation could move from one core to another, be off core at a stop, etc.  lldb_private::Threads are tricky, they can be an OS Plugin Thread, a Core Thread with a backing OS Plugin thread, an Core thread with no OS Plugin thread...  And the same TID can be change modes across stops.

The thing that isn't a fiction is the TID for the thread.  That's why it makes more sense to me that the persistent part of the thread data in lldb be linked to the TID and not the Thread object we happen to have made for it this time.  Pulling the persistent part out and having it just depend on the TID, not on the details of how we produced the lldb_private::Thread representing that TID, makes all the updating logic easier to manage.

> Stepping on Android could really benefit from some of the plans you mention where we might not end up fetching all of the thread IDs on a stop from the GDB remote. One idea is, when we do a continue where we specify only one thread to run, or only a few threads get to run, then the stop reply would only need to mention these threads and any new threads that showed up during the last resume. We can safely assume any other threads were suspended and have the exact same state. This will take some careful tracking in the Process class, but it could be done. For some reason Android has 150 threads running a lot of times which causes stepping to take a while on many cases. A lot of these issues were fixed with my patches submitted a while back where when we are stepping over stuff, we don't always stop at all branches, but only stop at branches that are not function calls, but stepping can still be quite slow.

Right, I think this is an enabling step for that sort of optimization.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75711/new/

https://reviews.llvm.org/D75711