[OB] Improve remote attach round-trips without btrace
Commit Message
Hello list,
For remotes which do not support 'qXfer:btrace-conf:read', we can save
several round trips for each thread. This is especially significant
when your remote is a kernel with 100s or 1000s of threads and latency
is intercontinental.
Without the change, with target, remote, and infrun debugging enabled,
one might see:
Sending packet: $Hg18aee#43...Ack
Packet received: OK
Sending packet: $Hg186f7#eb...Ack
Packet received: OK
remote:target_xfer_partial (24, , 0x805454000, 0x0, 0x0, 4096) = -1, 0
(Repeated for all non-exited threads.)
What's happening here is that remote_target::remote_btrace_maybe_reopen is:
* walking all non-exited threads
* for each, invoking set_general_thread () ("HgNNNNN" packet sent to remote)
* btrace_config is initialized to zero with memset
* btrace_read_config -> ... -> remote_target::remote_read_qxfer
checks if PACKET_qXfer_btrace_conf is disabled, and does not send any
packet; does not modify btrace_config
* (The same check results in the "remote:target_xfer_partial (...) =
-1" error return printed with debugging enabled)
* Finally, the 'format == BTRACE_FORMAT_NONE' condition
short-circuits the loop with 'continue' because BTRACE_FORMAT_NONE
happens to have a zero value, matching the earlier memset.
With the change, if the remote does not specify
'qXfer:btrace-conf:read+' in qSupported stub features, these spurious
thread switches are avoided.
Empirically, this improves "target remote" attach time on a particular
real-world configuration with ~770 threads from ~2 minutes to ~4
seconds:
$ ministat control.dat test.dat
x control.dat
+ test.dat
+--------------------------------------------------------------------------+
|+ x|
|+ x|
|+ x|
|A A|
+--------------------------------------------------------------------------+
N Min Max Median Avg Stddev
x 3 117.58 117.99 117.84 117.80333 0.20744477
+ 3 4.2 4.22 4.21 4.21 0.01
Difference at 95.0% confidence
-113.593 +/- 0.332863
-96.4262% +/- 0.0169361%
(Student's t, pooled s = 0.146856)
Please find the 'git format-patch' formatted patch attached to this
email, and please let me know if you have any follow-up questions. I
have already sent a copyright disclaimer form email for this change to
assign@gnu.
Thanks,
Conrad
P.S., I am not subscribed to the list.
gdb/ChangeLog:
* remote.c (remote_target::remote_btrace_maybe_reopen): Avoid
unnecessary thread walk if remote doesn't support the packet.
Comments
On 8/19/19 8:18 PM, Conrad Meyer wrote:
> Please find the 'git format-patch' formatted patch attached to this
> email, and please let me know if you have any follow-up questions. I
> have already sent a copyright disclaimer form email for this change to
> assign@gnu.
Thanks, this looks good.
I don't see your assignment/disclaimer in fencepost yet, but given that
change is small, that should not be a problem, so I went ahead and
merged it.
Pedro Alves
On Tue, Aug 20, 2019 at 7:28 AM Pedro Alves <palves@redhat.com> wrote:
>
> On 8/19/19 8:18 PM, Conrad Meyer wrote:
>
> > Please find the 'git format-patch' formatted patch attached to this
> > email, and please let me know if you have any follow-up questions. I
> > have already sent a copyright disclaimer form email for this change to
> > assign@gnu.
>
> Thanks, this looks good.
>
> I don't see your assignment/disclaimer in fencepost yet, but given that
> change is small, that should not be a problem, so I went ahead and
> merged it.
Thanks, Pedro!
Best,
Conrad
From 2f3b4b0472a9cbc0288a787487a1bc584e2c17f3 Mon Sep 17 00:00:00 2001
From: Conrad Meyer <cem@FreeBSD.org>
Date: Mon, 19 Aug 2019 00:02:19 -0700
Subject: [PATCH] Improve remote attach round-trips without btrace
For remotes which do not support btrace at all, we can save several
round trips for each thread. This is especially significant when your
remote is a kernel with 100s or 1000s of threads and latency is
intercontinental.
Previously, with target, remote, and infrun debugging enabled, one might
see:
Sending packet: $Hg18aee#43...Ack
Packet received: OK
Sending packet: $Hg186f7#eb...Ack
Packet received: OK
remote:target_xfer_partial (24, , 0x805454000, 0x0, 0x0, 4096) = -1, 0
Repeated for all non-exited threads.
Afterwards, if the remote does not specify 'qXfer:btrace-conf:read+' in
qSupported stub features, these spurious thread switches should be
avoided.
gdb/ChangeLog:
* remote.c (remote_target::remote_btrace_maybe_reopen): Avoid
unnecessary thread walk if remote doesn't support the packet.
---
gdb/ChangeLog | 5 +++++
gdb/remote.c | 6 ++++++
2 files changed, 11 insertions(+)
@@ -1,10 +1,15 @@
+2019-08-18 Conrad Meyer <cem@FreeBSD.org>
+
+ * remote.c (remote_target::remote_btrace_maybe_reopen): Avoid
+ unnecessary thread walk if remote doesn't support the packet.
+
2019-07-15 Tom Tromey <tromey@adacore.com>
* mi/mi-out.c (mi_ui_out::do_field_int): Use plongest.
2019-07-15 Tom Tromey <tromey@adacore.com>
* mi/mi-out.h (class mi_ui_out) <do_field_unsigned>: Declare.
* mi/mi-out.c (mi_ui_out::do_field_unsigned): New method.
* cli-out.h (class cli_ui_out) <do_field_unsigned>: Declare.
* cli-out.c (cli_ui_out::do_field_int): New method.
@@ -13693,20 +13693,26 @@ btrace_read_config (struct btrace_config *conf)
void
remote_target::remote_btrace_maybe_reopen ()
{
struct remote_state *rs = get_remote_state ();
int btrace_target_pushed = 0;
#if !defined (HAVE_LIBIPT)
int warned = 0;
#endif
+ /* Don't walk the entirety of the remote thread list when we know the feature
+ isn't supported by the remote. With thousands of threads and a high
+ latency remote, this is a huge waste. */
+ if (packet_support (PACKET_qXfer_btrace_conf) != PACKET_ENABLE)
+ return;
+
scoped_restore_current_thread restore_thread;
for (thread_info *tp : all_non_exited_threads ())
{
set_general_thread (tp->ptid);
memset (&rs->btrace_config, 0x00, sizeof (struct btrace_config));
btrace_read_config (&rs->btrace_config);
if (rs->btrace_config.format == BTRACE_FORMAT_NONE)
--
2.21.0