labath wrote: Thanks for checking this out, Jason.
(1) makes most sense to me as well. I'll just add that, if you want to reduce roundtrips, in principle it should possible to include the eager breakpoint request into the multi-packet flushing the delayed requests. The response includes individual error messages for each sub-request, so it should be possible to see whether the eager request failed or not. > @labath Felipe still needs to merge (this one final?) PR #192988 to have > ProcessGDBRemote start sending MultiBreakpoint's; we are still doing the > breakpoint enabling and disabling with separate packets right now. The > behavior should be the same as the final goal, though: all breakpoint > enabling/disabling is queued, until we go to resume the target, and then lldb > sends all of them all together. I understand that multi-packet is not enabled yet, but my point is that you don't *need* the multi packet to optimize this sort of sequence. The code can already see it is going to disable and reenable a breakpoint at the same address, so it could just cancel those two out locally. In fact, the code already tries to do that (`Process::ExecuteBreakpointSiteAction`), but it somehow doesn't work here. The matching logic is site-based, so I expect this will not catch the cases where a breakpoint deleted and a new one gets created in the same place, but I haven't checked if this is what's happening here or if there's some other issue in play. This isn't a regression, since those packets would get sent even before this series, but I think you could call it a missed opportunity. Seeing the two packets back-to-back is sort of ridiculous and I can't help but wonder how much speedup you could get by eliminating these -- even without the new packet. Even if adding the packet is still worthwhile, this sort of thing could help with older stubs, which I believe is something that you care about. I definitely haven't thought through all the implications, but an obvious idea is to change to logic to cluster breakpoint requests based on breakpoint address (as that is what ultimately gets sent), rather than the BreakpointSite pointer. https://github.com/llvm/llvm-project/pull/192971 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
