"Transfer rate" patch

"Transfer rate" patch

[Ilko Iliev]

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]

Hi,

I found a small bug by the calculating of the transfer rate at the 
"load" command - if the download image size or the download speed is 
higher then occurs overflow and the printed information is wrong.

I attached a patch for this problem.


regards,
Ilko Iliev
www.ronetix.at



[-- Attachment #2: gdb_download_speed.patch --]
[-- Type: text/plain, Size: 826 bytes --]

--- symfile.c.orig	2006-08-31 15:29:12.000000000 +0200
+++ symfile.c	2006-08-31 15:47:28.000000000 +0200
@@ -1769,14 +1769,14 @@
   ui_out_text (uiout, "Transfer rate: ");
   if (time_count > 0)
     {
-      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-			1000 * (data_count * 8) / time_count);
-      ui_out_text (uiout, " bits/sec");
+      ui_out_field_fmt (uiout, "transfer-rate", "%lu", 
+	(unsigned long)((((unsigned long long)data_count)*1000)/time_count)/1024);
+      ui_out_text (uiout, " Kbytes/sec");
     }
   else
     {
-      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count * 8));
-      ui_out_text (uiout, " bits in <1 sec");
+      ui_out_field_fmt (uiout, "transferred-bits", "%lu", data_count);
+      ui_out_text (uiout, " bytes in <1 sec");
     }
   if (write_count > 0)
     {

Re: "Transfer rate" patch

[Michael Snyder]

On Tue, 2006-09-05 at 09:24 +0200, Ilko Iliev wrote:
> Hi,
> 
> I found a small bug by the calculating of the transfer rate at the 
> "load" command - if the download image size or the download speed is 
> higher then occurs overflow and the printed information is wrong.
> 
> I attached a patch for this problem.

Thank you Ilko -- two requests.
1) The gnu coding standard calls for a white space on each side
of an operator, eg. not "(foo)*1000" but (foo) * 1000".  Could
you add those please?  If you need to break a long line, break
it at an operator, and put the operator on the new line.  And,
2) All patches require a ChangeLog entry.  It should be short 
and descriptive, eg. "Fixed overflow problem".  See the file, 
ChangeLog, to get the format, and please include an entry when
you re-submit.

Thanks,
Michael Snyder

Re: "Transfer rate" patch

[Vladimir Prus]

Ilko Iliev wrote:

> "Transfer rate" patch
> From:
> Ilko Iliev <office@ronetix.at>
>   Date:
> Tuesday 05 September 2006 11:24:48
>   Groups:
> gmane.comp.gdb.patches
> Hi,
> 
> I found a small bug by the calculating of the transfer rate at the
> "load" command - if the download image size or the download speed is
> higher then occurs overflow and the printed information is wrong.
> 
> I attached a patch for this problem.
> 
> 
> regards,
> Ilko Iliev
> www.ronetix.at
> gdb_download_speed.patch
>   --- symfile.c.orig      2006-08-31 15:29:12.000000000 +0200
> +++ symfile.c   2006-08-31 15:47:28.000000000 +0200
> @@ -1769,14 +1769,14 @@
> ui_out_text (uiout, "Transfer rate: ");
> if (time_count > 0)
> {
> -      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
> -                       1000 * (data_count * 8) / time_count);
> -      ui_out_text (uiout, " bits/sec");
> +      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
> +       (unsigned long)((((unsigned long
> long)data_count)*1000)/time_count)/1024); +      ui_out_text (uiout, "
> Kbytes/sec"); }

I am not sure about motivation to switch to "Kbytes/sec" -- I've seen a
target that does 700 *bytes* per second, so your code will just print '0'
in that case -- is that intended? 

> else
> {
> -      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count *
> 8)); -      ui_out_text (uiout, " bits in <1 sec");
> +      ui_out_field_fmt (uiout, "transferred-bits", "%lu", data_count);
> +      ui_out_text (uiout, " bytes in <1 sec");

You've changed the code to output the number of bytes and changes the text
to read "bytes" the the name of output field is still 'transferred-bits'.
I'm not sure this matters (maybe in MI mode), but in any way, you should
adjust that.


- Volodya

"Transfer rate" patch

[Ilko Iliev]

[-- Attachment #1: Type: text/plain, Size: 2153 bytes --]

Hi,

The corrected patch again - the output is in "KBytes/sec" or "bytes/sec".
The ChangeLog file is updated too.

I changed to "KBytes/sec" because of our and other fast JTAG Emulators.

regards,
Ilko Iliev
Ronetix - JTAG Emulators and Flash Programmers
www.ronetix.at



Vladimir Prus wrote:
> Ilko Iliev wrote:
>
>   
>> "Transfer rate" patch
>> From:
>> Ilko Iliev <office@ronetix.at>
>>   Date:
>> Tuesday 05 September 2006 11:24:48
>>   Groups:
>> gmane.comp.gdb.patches
>> Hi,
>>
>> I found a small bug by the calculating of the transfer rate at the
>> "load" command - if the download image size or the download speed is
>> higher then occurs overflow and the printed information is wrong.
>>
>> I attached a patch for this problem.
>>
>>
>> regards,
>> Ilko Iliev
>> www.ronetix.at
>> gdb_download_speed.patch
>>   --- symfile.c.orig      2006-08-31 15:29:12.000000000 +0200
>> +++ symfile.c   2006-08-31 15:47:28.000000000 +0200
>> @@ -1769,14 +1769,14 @@
>> ui_out_text (uiout, "Transfer rate: ");
>> if (time_count > 0)
>> {
>> -      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
>> -                       1000 * (data_count * 8) / time_count);
>> -      ui_out_text (uiout, " bits/sec");
>> +      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
>> +       (unsigned long)((((unsigned long
>> long)data_count)*1000)/time_count)/1024); +      ui_out_text (uiout, "
>> Kbytes/sec"); }
>>     
>
> I am not sure about motivation to switch to "Kbytes/sec" -- I've seen a
> target that does 700 *bytes* per second, so your code will just print '0'
> in that case -- is that intended? 
>
>   
>> else
>> {
>> -      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count *
>> 8)); -      ui_out_text (uiout, " bits in <1 sec");
>> +      ui_out_field_fmt (uiout, "transferred-bits", "%lu", data_count);
>> +      ui_out_text (uiout, " bytes in <1 sec");
>>     
>
> You've changed the code to output the number of bytes and changes the text
> to read "bytes" the the name of output field is still 'transferred-bits'.
> I'm not sure this matters (maybe in MI mode), but in any way, you should
> adjust that.
>
>
> - Volodya
>
>   


[-- Attachment #2: gdb_download_speed.patch --]
[-- Type: text/plain, Size: 1523 bytes --]

--- ChangeLog.orig	2006-09-07 15:23:19.000000000 +0200
+++ ChangeLog	2006-09-07 15:27:46.000000000 +0200
@@ -1,3 +1,8 @@
+2006-09-07  Ilko Iliev <iliev@ronetix.at>
+
+	* symfile.c (print_transfer_performance): Fix overflow problem
+        and change bits/sec to KBytes/sec or bytes/sec
+
 2006-08-28  DJ Delorie  <dj@redhat.com>
 
 	* m32c-tdep.c (m32c_decode_srcdest4): Initialize fields in sd



--- symfile.c.orig	2006-08-31 15:29:12.000000000 +0200
+++ symfile.c	2006-09-07 15:19:45.000000000 +0200
@@ -1769,14 +1769,23 @@
   ui_out_text (uiout, "Transfer rate: ");
   if (time_count > 0)
     {
-      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-			1000 * (data_count * 8) / time_count);
-      ui_out_text (uiout, " bits/sec");
+      if ( data_count < 1024 )
+        {
+          ui_out_field_fmt (uiout, "transfer-rate", "%lu",
+            (unsigned long)((((unsigned long long)data_count) * 1000) / time_count));
+          ui_out_text (uiout, " bytes/sec");
+        }
+      else
+        {
+          ui_out_field_fmt (uiout, "transfer-rate", "%lu", 
+	    (unsigned long)((((unsigned long long)data_count) * 1000) / time_count) / 1024);
+          ui_out_text (uiout, " Kbytes/sec");
+        }
     }
   else
     {
-      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count * 8));
-      ui_out_text (uiout, " bits in <1 sec");
+      ui_out_field_fmt (uiout, "transferred-bytes", "%lu", data_count);
+      ui_out_text (uiout, " bytes in <1 sec");
     }
   if (write_count > 0)
     {

Re: "Transfer rate" patch

[Vladimir Prus]

Ilko Iliev wrote:

[snip quoted entire previous patch]

>   --- ChangeLog.orig      2006-09-07 15:23:19.000000000 +0200
> +++ ChangeLog   2006-09-07 15:27:46.000000000 +0200
> @@ -1,3 +1,8 @@
> +2006-09-07  Ilko Iliev <iliev@ronetix.at>
> +
> +       * symfile.c (print_transfer_performance): Fix overflow problem
> +        and change bits/sec to KBytes/sec or bytes/sec
> +
> 2006-08-28  DJ Delorie  <dj@redhat.com>
> 
> * m32c-tdep.c (m32c_decode_srcdest4): Initialize fields in sd
> 
> 
> 
> --- symfile.c.orig      2006-08-31 15:29:12.000000000 +0200
> +++ symfile.c   2006-09-07 15:19:45.000000000 +0200
> @@ -1769,14 +1769,23 @@
> ui_out_text (uiout, "Transfer rate: ");
> if (time_count > 0)
> {
> -      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
> -                       1000 * (data_count * 8) / time_count);
> -      ui_out_text (uiout, " bits/sec");
> +      if ( data_count < 1024 )
> +        {
> +          ui_out_field_fmt (uiout, "transfer-rate", "%lu",
> +            (unsigned long)((((unsigned long long)data_count) * 1000) /
> time_count)); 

I think the above line is longer than 80 characters, and that's probably not
OK.

Also, I think this branch should be taken if download speed is smaller that
1024 bytes *per second*, not when total download size is less than 1024
bytes.

- Volodya

Re: "Transfer rate" patch

[Mark Kettenis]

> Date: Thu, 07 Sep 2006 16:20:35 +0200
> From: Ilko Iliev <iliev@ronetix.at>
> 
> The corrected patch again - the output is in "KBytes/sec" or "bytes/sec".
> The ChangeLog file is updated too.
> 
> I changed to "KBytes/sec" because of our and other fast JTAG Emulators.

Well 9600 Bps is still pretty much standard for serial lines.

Mark

Re: "Transfer rate" patch

[Ilko Iliev]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

Mark Kettenis wrote:
>> Date: Thu, 07 Sep 2006 16:20:35 +0200
>> From: Ilko Iliev <iliev@ronetix.at>
>>
>> The corrected patch again - the output is in "KBytes/sec" or "bytes/sec".
>> The ChangeLog file is updated too.
>>
>> I changed to "KBytes/sec" because of our and other fast JTAG Emulators.
>>     
>
> Well 9600 Bps is still pretty much standard for serial lines.
>
> Mark
>   

Even of 9600 Bps it is more comfortable to see the download sped in 
KBytes/s.
If the download speed is less than 1 KByte/s, the print is in bytes/s.

In the attachment is the corrected version of the patch.


regards,
Ilko Iliev
Ronetix - JTAG Emulators and Flash Programmers
www.ronetix.at



[-- Attachment #2: gdb_download_speed.patch --]
[-- Type: text/plain, Size: 2004 bytes --]

--- ChangeLog.orig	2006-09-07 15:23:19.000000000 +0200
+++ ChangeLog	2006-09-07 15:27:46.000000000 +0200
@@ -1,3 +1,8 @@
+2006-09-07  Ilko Iliev <iliev@ronetix.at>
+
+	* symfile.c (print_transfer_performance): Fix overflow problem
+        and change bits/sec to KBytes/sec or bytes/sec
+
 2006-08-28  DJ Delorie  <dj@redhat.com>
 
 	* m32c-tdep.c (m32c_decode_srcdest4): Initialize fields in sd



--- symfile.c.orig	2006-08-31 15:29:12.000000000 +0200
+++ symfile.c	2006-09-07 17:23:48.000000000 +0200
@@ -1758,27 +1758,36 @@ print_transfer_performance (struct ui_fi
 			    unsigned long write_count,
 			    const struct timeval *start_time,
 			    const struct timeval *end_time)
 {
   unsigned long time_count;
+  unsigned long rr;
 
   /* Compute the elapsed time in milliseconds, as a tradeoff between
      accuracy and overflow.  */
   time_count = (end_time->tv_sec - start_time->tv_sec) * 1000;
   time_count += (end_time->tv_usec - start_time->tv_usec) / 1000;
 
   ui_out_text (uiout, "Transfer rate: ");
   if (time_count > 0)
     {
-      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-			1000 * (data_count * 8) / time_count);
-      ui_out_text (uiout, " bits/sec");
+      rr = (unsigned long)((unsigned long long)data_count * 1000 / time_count);
+      if ( rr < 1024 )
+        {
+          ui_out_field_fmt (uiout, "transfer-rate", "%lu", rr );
+          ui_out_text (uiout, " bytes/sec");
+        }
+      else
+        {
+          ui_out_field_fmt (uiout, "transfer-rate", "%lu", rr / 1024 );
+          ui_out_text (uiout, " Kbytes/sec");
+        }
     }
   else
     {
-      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count * 8));
-      ui_out_text (uiout, " bits in <1 sec");
+      ui_out_field_fmt (uiout, "transferred-bytes", "%lu", data_count);
+      ui_out_text (uiout, " bytes in <1 sec");
     }
   if (write_count > 0)
     {
       ui_out_text (uiout, ", ");
       ui_out_field_fmt (uiout, "write-rate", "%lu", data_count / write_count);

Re: "Transfer rate" patch

[Andrew STUBBS]

What happened to this patch? This is one I would like to see.

Ilko Iliev wrote:
> Mark Kettenis wrote:
>>> Date: Thu, 07 Sep 2006 16:20:35 +0200
>>> From: Ilko Iliev <iliev@ronetix.at>
>>>
>>> The corrected patch again - the output is in "KBytes/sec" or 
>>> "bytes/sec".
>>> The ChangeLog file is updated too.
>>>
>>> I changed to "KBytes/sec" because of our and other fast JTAG Emulators.
>>>     
>>
>> Well 9600 Bps is still pretty much standard for serial lines.
>>
>> Mark
>>   
> 
> Even of 9600 Bps it is more comfortable to see the download sped in 
> KBytes/s.
> If the download speed is less than 1 KByte/s, the print is in bytes/s.
> 
> In the attachment is the corrected version of the patch.
> 
> 
> regards,
> Ilko Iliev
> Ronetix - JTAG Emulators and Flash Programmers
> www.ronetix.at
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- ChangeLog.orig	2006-09-07 15:23:19.000000000 +0200
> +++ ChangeLog	2006-09-07 15:27:46.000000000 +0200
> @@ -1,3 +1,8 @@
> +2006-09-07  Ilko Iliev <iliev@ronetix.at>
> +
> +	* symfile.c (print_transfer_performance): Fix overflow problem
> +        and change bits/sec to KBytes/sec or bytes/sec
> +
>  2006-08-28  DJ Delorie  <dj@redhat.com>
>  
>  	* m32c-tdep.c (m32c_decode_srcdest4): Initialize fields in sd
> 
> 
> 
> --- symfile.c.orig	2006-08-31 15:29:12.000000000 +0200
> +++ symfile.c	2006-09-07 17:23:48.000000000 +0200
> @@ -1758,27 +1758,36 @@ print_transfer_performance (struct ui_fi
>  			    unsigned long write_count,
>  			    const struct timeval *start_time,
>  			    const struct timeval *end_time)
>  {
>    unsigned long time_count;
> +  unsigned long rr;
>  
>    /* Compute the elapsed time in milliseconds, as a tradeoff between
>       accuracy and overflow.  */
>    time_count = (end_time->tv_sec - start_time->tv_sec) * 1000;
>    time_count += (end_time->tv_usec - start_time->tv_usec) / 1000;
>  
>    ui_out_text (uiout, "Transfer rate: ");
>    if (time_count > 0)
>      {
> -      ui_out_field_fmt (uiout, "transfer-rate", "%lu",
> -			1000 * (data_count * 8) / time_count);
> -      ui_out_text (uiout, " bits/sec");
> +      rr = (unsigned long)((unsigned long long)data_count * 1000 / time_count);
> +      if ( rr < 1024 )
> +        {
> +          ui_out_field_fmt (uiout, "transfer-rate", "%lu", rr );
> +          ui_out_text (uiout, " bytes/sec");
> +        }
> +      else
> +        {
> +          ui_out_field_fmt (uiout, "transfer-rate", "%lu", rr / 1024 );
> +          ui_out_text (uiout, " Kbytes/sec");
> +        }
>      }
>    else
>      {
> -      ui_out_field_fmt (uiout, "transferred-bits", "%lu", (data_count * 8));
> -      ui_out_text (uiout, " bits in <1 sec");
> +      ui_out_field_fmt (uiout, "transferred-bytes", "%lu", data_count);
> +      ui_out_text (uiout, " bytes in <1 sec");
>      }
>    if (write_count > 0)
>      {
>        ui_out_text (uiout, ", ");
>        ui_out_field_fmt (uiout, "write-rate", "%lu", data_count / write_count);
> 

Re: "Transfer rate" patch

[Daniel Jacobowitz]

On Tue, Oct 17, 2006 at 05:04:38PM +0100, Andrew STUBBS wrote:
> What happened to this patch? This is one I would like to see.

It's definitely in the pile of things I still need to look at.  I
wanted to think about the change in units, which is exposed to GDB/MI;
I'm not sure that it matters, by any means, but it stuck up a flag that
we ought to think about it.

-- 
Daniel Jacobowitz
CodeSourcery

Re: "Transfer rate" patch

[Andrew STUBBS]

Vladimir Prus wrote:
> You've changed the code to output the number of bytes and changes the text
> to read "bytes" the the name of output field is still 'transferred-bits'.
> I'm not sure this matters (maybe in MI mode), but in any way, you should
> adjust that.

I'm not sure what the policy is on these things, but I think that 
arbitrarily changing field names (and values) unnecessarily breaks 
compatibility with tools that use MI mode.