[Lldb-commits] [PATCH] D22914: [WIP] Add concurrent packets support to gdb-remote client

[Greg Clayton via lldb-commits]

clayborg added a comment.

A few things that worry me:

- There are no sequence IDs on any packets so when receiving responses, I am not sure how you are going to match up packets with their responses especially when you are sending from multiple threads. I didn't look through your implementation, but I am guessing you didn't add that?
- What not just make a qModulesInfo that gets all module info from a process and avoid the many callbacks? When we were reducing packets for Apple Watch we did a lot of consolidation packets where we get all of the data we needed in fewer packets (we only get 22-50 packet per second on the watch!).

So I am kind of against this patch because there is no way to take advantage of it from a API standpoint. Take qModuleInfo for example. ProcessGDBRemote::GetModuleSpec() and PlatformRemoteGDBServer::GetModuleSpec() are the only two callers of this. How would you ever take advantage of sending multiple packets at the same time? Make the access multi-threaded? I was sooner see a new functions that get more than one module spec at a time:

ProcessGDBRemote::GetModuleSpecs(...)
PlatformRemoteGDBServer::GetModuleSpecs(...)

Then this could call the qModulesInfo (note the extra "s") with all of the module infos encoded as JSON, and the reply can contain all module infos needed.

As soon as we start adding sequence numbers and extra stuff, this stops becoming GDB remote protocol and becomes our own. We might as well switch over to a new JSON based protocol that fixes all of the issues with GDB remote...


https://reviews.llvm.org/D22914