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

[Pavel Labath via lldb-commits]

labath created this revision.
labath added a reviewer: clayborg.
labath added a subscriber: lldb-commits.
Herald added subscribers: danalbert, tberghammer.

This change adds the ability to the client to send multiple packets without waiting for the
response to the first one. The individual senders use a queue to register themselves when they
have sent a packet. When they reach the front of the queue it's their turn to read a packet.

This does not change the protocol in any way -- a well-behaved stub which just sits in a
ReadPacketAndSendResponse() loop will not notice any difference. It does however make a huge
difference on connections with a non-negligible latency (anything over 10ms certainly fits the
bill), because we will have multiple packets in flight at the same time. The queueing system
avoids the need for a separate thread doing the reading/writing and should add no overhead to the
common case of sequential packet streams.

No code except the unit test actually uses the new functionality yet, as the default is for all
senders to acquire an exclusive connection lock. Any packets using this will need to be enabled
on a one-off basis, making sure that there can be no bad interactions between other packets which
can get sent concurrently. Some obvious cases where this can *not* be enabled are:
- AckMode: when using unreliable connections, we can never be sure that the other side has
  recieved our packet and we may need to do retransmissions.
- packets which need to be sent as an atomic sequence (e.g. if we don't have thread suffix
  enabled, then we need to send Hg plus another packet together).

The first packet which I would try to enable is qModuleInfo, which is a read-only packet with no
side-effects, and we need to send a lot of them (over 150 when attaching to a typical Android
application), so it can give a big speed boost there. However, I can see this being useful in
other places as well (parallel thread backtrace computation?).

To make the code thread-sanitizer clean, I've needed to add a lock to the packet history class,
as now we can be sending and receiving a packet simultaneously.

---------- WIP ----------
This code does not actually work yet (apart from the unittests), because the read/write lock does
not like to be locked recursively. I'll need to weed those out first (which should be pretty
orthogonal to the changes here).
---------- WIP ----------

https://reviews.llvm.org/D22914

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22914.65917.patch
Type: text/x-patch
Size: 37372 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160728/48f200e9/attachment-0001.bin>