[Lldb-commits] [lldb] Improving performance of multiple threads stepping over the same brea… (PR #180101)

[via lldb-commits]

================
@@ -589,14 +621,69 @@ bool ThreadList::WillResume(RunDirection &direction) {
           assert(thread_sp->GetCurrentPlan()->GetDirection() == direction);
           // You can't say "stop others" and also want yourself to be suspended.
           assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
+
----------------
jimingham wrote:

Well that was dumb...

By bad fortune the GitHub diff display elided JUST line 617, and I didn't notice the elision...  Grrr...

With that line actually present, this makes more sense.

I don't think it affects your analysis, but this isn't necessarily correct:

`StopOthers scans: we scan for threads whose current plan has stopothers()=true. since we just popped all stepoverbreakpoint plans, these wont contibue here. the only way thread_to_run gets set in this scan is if some thread has non-breapoint stopothers plan. in our case, we shouldnt have a thread that has such plan, so thread_to_ryb remains nullptr and we fall to the else branch.
`

You might have one thread that's trying to stop the other threads, for instance because it's running a function on one thread only, or doing a "run this thread only step" and might have inserted a breakpoint to do that.  It might have even hit its breakpoint on one stop, but then didn't get a chance to run so that at the next stop, after the ThreadPlanStepOverBreakpoint is popped, there will still be a plan that wants to run solo on that thread.

But I don't think that wouldn't be a problem, because even if there were one or more "run on their own" threads after the ThreadPlanStepOverBreakpoints are removed, we'd pick one of them and it will go through the "if (thread_to_run)" branch and set only its ThreadPlanStepOverBreakpoint and then it will run solo.  If that thread were sitting at a breakpoint that other threads shared, you would in this case lose the "run past all the breakpoints with only one breakpoint insertion/removal" but that is appropriate because the remaining plans after removing the step over breakpoint plans are the ones that are doing actual work as directed by the user.  So they should be of higher priority.

This all looks right to me once I see it all...

But I want @jasonmolenda to take a look because he did all the work to make the breakpoint hit accounting work correctly.  I can't see how this would mess that up, but Jason will be much more likely to see any subtle problems I might miss.

https://github.com/llvm/llvm-project/pull/180101