Call target_can_download_tracepoint if there are tracepoints to download
Commit Message
Nowadays, GDB calls target_can_download_tracepoint at the entry of
download_tracepoint_locations, which is called by.
update_global_location_list. Sometimes, it is not needed to call
target_can_download_tracepoint at all because there is no tracepoint
created. In remote target, target_can_download_tracepoint send
qTStatus to the remote in order to know whether tracepoint can be
downloaded or not. This means some redundant qTStatus packets are
sent.
This patch is to teach GDB to call target_can_download_tracepoint
lazily, only on the moment there are tracepoint to download.
gdb.perf/single-step.exp (with a local patch to measure RSP packets)
shows the number of RSP packets is reduced because there is no
tracepoint at all, so GDB doesn't send qTStatus any more.
# of RSP packets
original patched
single-step rsp 1000 7000 6000
single-step rsp 2000 14000 12000
single-step rsp 3000 21000 18000
single-step rsp 4000 28000 24000
Regression tested on x86_64-linux, both native and gdbserver.
gdb:
2015-09-08 Yao Qi <yao.qi@linaro.org>
* breakpoint.c (download_tracepoint_locations): New local
can_download_tracepoint. Check the result of
target_can_download_tracepoint and save it in
can_download_tracepoint if there are tracepoints to download.
* linux-nat.h (enum tribool): Move it to ...
* defs.h: ... here.
---
gdb/breakpoint.c | 15 ++++++++++++---
gdb/defs.h | 2 ++
gdb/linux-nat.h | 1 -
3 files changed, 14 insertions(+), 4 deletions(-)
Comments
On 09/08/2015 03:13 PM, Yao Qi wrote:
> Nowadays, GDB calls target_can_download_tracepoint at the entry of
> download_tracepoint_locations, which is called by.
> update_global_location_list. Sometimes, it is not needed to call
> target_can_download_tracepoint at all because there is no tracepoint
> created. In remote target, target_can_download_tracepoint send
> qTStatus to the remote in order to know whether tracepoint can be
> downloaded or not. This means some redundant qTStatus packets are
> sent.
>
> This patch is to teach GDB to call target_can_download_tracepoint
> lazily, only on the moment there are tracepoint to download.
> gdb.perf/single-step.exp (with a local patch to measure RSP packets)
> shows the number of RSP packets is reduced because there is no
> tracepoint at all, so GDB doesn't send qTStatus any more.
>
> # of RSP packets
> original patched
> single-step rsp 1000 7000 6000
> single-step rsp 2000 14000 12000
> single-step rsp 3000 21000 18000
> single-step rsp 4000 28000 24000
>
> Regression tested on x86_64-linux, both native and gdbserver.
Awesome. These qTStatus packets have bothered me before too.
The patch looks fine to me.
>
> gdb:
>
> 2015-09-08 Yao Qi <yao.qi@linaro.org>
>
> * breakpoint.c (download_tracepoint_locations): New local
> can_download_tracepoint. Check the result of
> target_can_download_tracepoint and save it in
> can_download_tracepoint if there are tracepoints to download.
> * linux-nat.h (enum tribool): Move it to ...
> * defs.h: ... here.
I'd suggest moving tribool in common/common-types.h. It's just
be a matter of time before we want to use this in gdbserver too..
Thanks,
Pedro Alves
@@ -12078,9 +12078,7 @@ download_tracepoint_locations (void)
{
struct breakpoint *b;
struct cleanup *old_chain;
-
- if (!target_can_download_tracepoint ())
- return;
+ enum tribool can_download_tracepoint = TRIBOOL_UNKNOWN;
old_chain = save_current_space_and_thread ();
@@ -12095,6 +12093,17 @@ download_tracepoint_locations (void)
: !may_insert_tracepoints))
continue;
+ if (can_download_tracepoint == TRIBOOL_UNKNOWN)
+ {
+ if (target_can_download_tracepoint ())
+ can_download_tracepoint = TRIBOOL_TRUE;
+ else
+ can_download_tracepoint = TRIBOOL_FALSE;
+ }
+
+ if (can_download_tracepoint == TRIBOOL_FALSE)
+ break;
+
for (bl = b->loc; bl; bl = bl->next)
{
/* In tracepoint, locations are _never_ duplicated, so
@@ -234,6 +234,8 @@ enum return_value_convention
RETURN_VALUE_ABI_PRESERVES_ADDRESS,
};
+enum tribool { TRIBOOL_UNKNOWN = -1, TRIBOOL_FALSE = 0, TRIBOOL_TRUE = 1 };
+
/* Needed for various prototypes */
struct symtab;
@@ -116,7 +116,6 @@ struct lwp_info
extern struct lwp_info *lwp_list;
/* Does the current host support PTRACE_GETREGSET? */
-enum tribool { TRIBOOL_UNKNOWN = -1, TRIBOOL_FALSE = 0, TRIBOOL_TRUE = 1 };
extern enum tribool have_ptrace_getregset;
/* Iterate over each active thread (light-weight process). */