Re: [PATCH] mt set per-command remote-packets on|off

[Yao Qi <yao@codesourcery.com>]

On 11/28/2013 06:43 AM, Doug Evans wrote:
> +
> +  if (start_stats->remote_packets_enabled)
> +    {
> +      long packets_sent, packets_rcvd, bytes_sent, bytes_rcvd;
> +
> +      get_remote_packet_stats (&packets_sent, &packets_rcvd,
> +			       &bytes_sent, &bytes_rcvd);
> +      if (packets_sent > 0)
> +	{
> +	  printf_unfiltered (_("#pkts sent: %ld (+%ld),"
> +			       " #pkts rcvd: %ld (+%ld)\n"
> +			       "#bytes sent: %ld (+%ld),"
> +			       " #bytes rcvd: %ld (+%ld)\n"),
> +			     packets_sent,
> +			     packets_sent - start_stats->start_packets_sent,
> +			     packets_rcvd,
> +			     packets_rcvd - start_stats->start_packets_rcvd,
> +			     bytes_sent,
> +			     bytes_sent - start_stats->start_packet_bytes_sent,
> +			     bytes_rcvd,
> +			     (bytes_rcvd
> +			      - start_stats->start_packet_bytes_rcvd));
> +	}
> +    }

It might not be good to bypass target interface to get the data about
remote target in target independent code.  We want to print some 
statistics of target after each command.  For "remote" target, we print 
statistics on packet, and for other targets, we may want to print 
something else.  IWBN to add a new interface target_print_statistics, 
and leave each target to decide what data to print.

> diff --git a/gdb/remote.c b/gdb/remote.c
> index 186c058..429a49c 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -426,6 +426,14 @@ struct remote_state
>
>     /* The state of remote notification.  */
>     struct remote_notif_state *notif_state;
> +
> +  /* Statistics for measuring protocol efficiency.
> +     Note: These record data at the protocol level, so to speak.
> +     If we're communicating over a flaky channel and have to resent packets
> +     that is not accounted for here.  It's recommended to track resends, etc.
> +     separately.  */
> +  long packets_sent, packets_received;
> +  long bytes_sent, bytes_received;

Use "unsigned long"?

IWBN to do statistics in a fine granularity, to count for each 
frequently-used type of packet.  For example, we can count the sent 'm', 
'x', and 'g' packet, then we'll get more clues from the the statistics 
messages.

Probably we can create a new structure, say "remote_packet_statistics", 
for these fields....

>   };
>
>   /* Private data that we'll store in (struct thread_info)->private.  */
> @@ -629,6 +637,20 @@ get_remote_state (void)
>     return get_remote_state_raw ();
>   }
>
> +/* Fetch the current packet statistics.  */
> +
> +void
> +get_remote_packet_stats (long *packets_sent, long *packets_received,
> +			 long *bytes_sent, long *bytes_received)
> +{

... In this way, the number of arguments of function can be reduced.

> +  struct remote_state *rs = get_remote_state ();
> +
> +  *packets_sent = rs->packets_sent;
> +  *packets_received = rs->packets_received;
> +  *bytes_sent = rs->bytes_sent;
> +  *bytes_received = rs->bytes_received;
> +}
> +

I am trying to add a python binding to get the statistics of rsp 
packets, which can be used as a new measurement in perf testsuite.
If one change causes the number of packets increased dramatically, the 
change may cause some performance regressions in remote debugging.
Looks I can base my work on top of this patch.

-- 
Yao (齐尧)