Re: [PATCH v2] gdb: infrun: consume multiple events at each pass in stop_all_threads

[Simon Marchi <simark@simark.ca>]

On 2020-05-14 2:14 p.m., Pedro Alves wrote:
> On 5/14/20 7:02 PM, Simon Marchi wrote:
> 
>> Here's a rebased version.
> 
> Thanks Simon.  Please go ahead and merge.
> 
> BTW, did you consider coming up with a mechanism similar to
> make_scoped_defer_target_commit_resume()/target_commit_resume()
> for target_stop, so that we could coalesce the multiple vCont;t
> requests in a single remote packet?  That could also cut down
> on latency.
> 
> (gdbserver's resume interface is better for that, in that
> a stop, continues and steps all go via the same interface:
> 
>  static void
>  resume (struct thread_resume *actions, size_t num_actions)
>  {
> 
> I've pondered adjusting GDB's resume interface in a similar
> way.  That would make target_commit_resume Just Work for
> stops too.
> )

No, I haven't considered it, but I think I see what you mean.  To illustrate this case using
the remote target, I had to set "maintenance set target-non-stop on", while using
"target non-stop off".  Am I missing something, is there a more common scenario where
it gets called, using the remote target?

With the above, when hitting a breakpoint, I do see the stops sent in sequence as part
of stop_all_threads:

Sending packet: $vCont;t:p25a703.25a71c#86...Packet received: OK
Sending packet: $vCont;t:p25a703.25a71d#87...Packet received: OK
Sending packet: $vCont;t:p25a703.25a71e#88...Packet received: OK
Sending packet: $vCont;t:p25a703.25a71f#89...Packet received: OK
Sending packet: $vCont;t:p25a703.25a720#54...Packet received: OK

which could easily be coalesced.  I think a `target_commit_stop` approach that mimics
`target_commit_resume` would work, without being too invasive.  But maybe changing
the `target_stop` interface to accept multiple ptids would be a better approach for
the future, since it's more of a step towards the gdbserver-style interface that you
talked about.  In stop_all_threads, it would be quite easy to use: build a vector of
ptid in this loop:

4815           /* Go through all threads looking for threads that we need
4816              to tell the target to stop.  */
4817           for (thread_info *t : all_non_exited_threads ())

and call target_stop once after the loop.

Simon