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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to