Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <yao@codesourcery.com>
To: Doug Evans <dje@google.com>
Cc: gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH] mt set per-command remote-packets on|off
Date: Wed, 11 Dec 2013 01:08:00 -0000	[thread overview]
Message-ID: <52A7BAC1.9090900@codesourcery.com> (raw)
In-Reply-To: <CADPb22T3Z1+sK7+KyoBiX-fCf1yZpK4Mntc3xVcyN6w6fkhOUw@mail.gmail.com>

On 12/11/2013 05:12 AM, Doug Evans wrote:
> 
> 
> Good point.  OTOH going that route brings more complication that isn't
> needed at the moment (rsp perf data*is*  needed now).
> [I finally have some(!) time to return to this.]

That is good! :)

> 
> 
>> >
>> >IWBN to add a new interface target_print_statistics, and leave each target to decide what data to print.
> 
>  From a high level perspective the target collects the statistics, but
> consumers will want to print them their own way.
> I didn't want to go "whole hog" yet because it's not clear yet what
> the different consumers are.
> [And remote protocol performance is in real need of improvement so I
> wanted to start solving that problem now.]
> 

That is OK to me.  I am eager to see collect rsp perf data through
this and potential improvement, rather than designing an ideal
interface.

> 
>> >
>> >
>> >
>>> >>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"?
> 
> How about int?
>

If you prefer signed type here, "long" is fine.

>>> >>
>>> >>+/* 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.
> 
> It was done this way to separate the collector of the data from the
> printer of the data.
> If we later collect finer grained stats, callers that want a gross
> number can still call this, and this function could sum up the pieces.
> 

Even we just count these four, it is better to put them in a
structure, and associate the structure instance with remote_state.

-- 
Yao (齐尧)


      reply	other threads:[~2013-12-11  1:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-28  0:32 Doug Evans
2013-11-28  0:47 ` Tom Tromey
2013-11-28  1:36   ` Doug Evans
2013-11-28  5:36     ` Tom Tromey
2013-11-28  6:10 ` Eli Zaretskii
2013-11-28  8:42 ` Yao Qi
2013-12-10 21:12   ` Doug Evans
2013-12-11  1:08     ` Yao Qi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A7BAC1.9090900@codesourcery.com \
    --to=yao@codesourcery.com \
    --cc=dje@google.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox