gdb/symfile.c: Transfer rate calculation

gdb/symfile.c: Transfer rate calculation

[Shaun Jackman]

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

The transfer rate calculation in print_transfer_performance uses a
time unit of whole seconds. This gross time unit limits the accuracy
of the calculation, since the number of elapsed whole seconds usually
has only one significant digit. This patch adds
print_transfer_performance_ms which uses a time unit of milliseconds.
It leaves the semantics of print_transfer_performance unchanged
because gdb/m32r-rom.c and gdb/remote-m32r-sdi.c use it.

Cheers,
Shaun

2005-07-08  Shaun Jackman  <sjackman@gmail.com>

	* gdb/symfile.c (print_transfer_performance_ms): New function.
	Use gettimeofday instead of date to obtain microsecond precision. 
	(print_transfer_performance): Call print_transfer_performance_ms.

--- ./gdb/symfile.c-	2004-06-24 15:09:34.000000000 -0700
+++ ./gdb/symfile.c	2005-07-08 16:25:05.000000000 -0700
@@ -54,6 +54,7 @@
 #include "gdb_string.h"
 #include "gdb_stat.h"
 #include <ctype.h>
+#include <sys/time.h>
 #include <time.h>
 
  #ifndef O_BINARY
@@ -92,6 +93,9 @@
 
  static void symbol_file_add_main_1 (char *args, int from_tty, int flags);
 
+static void print_transfer_performance_ms (struct ui_file *,
+    unsigned long, unsigned long, unsigned long);
+
 static void add_symbol_file_command (char *, int);
 
 static void add_shared_symbol_files_command (char *, int);
@@ -1460,7 +1464,7 @@
 {
   asection *s;
   bfd *loadfile_bfd;
-  time_t start_time, end_time;	/* Start and end times of download */
+  struct timeval start_time, end_time;	/* Start and end times of download */
   char *filename;
   struct cleanup *old_cleanups;
   char *offptr;
@@ -1512,11 +1516,11 @@
   bfd_map_over_sections (loadfile_bfd, add_section_size_callback,
  			 (void *) &cbdata.total_size);
 
-  start_time = time (NULL);
+  gettimeofday(&start_time, NULL);
 
   bfd_map_over_sections (loadfile_bfd, load_section_callback, &cbdata);
 
-  end_time = time (NULL);
+  gettimeofday(&end_time, NULL);
 
   entry = bfd_get_start_address (loadfile_bfd);
   ui_out_text (uiout, "Start address ");
@@ -1534,8 +1538,10 @@
      file is loaded in.  Some targets do (e.g., remote-vx.c) but
      others don't (or didn't - perhaphs they have all been deleted).  */
 
-  print_transfer_performance (gdb_stdout, cbdata.data_count,
-			      cbdata.write_count, end_time - start_time);
+  print_transfer_performance_ms (gdb_stdout, cbdata.data_count,
+			      cbdata.write_count,
+				  (end_time.tv_sec - start_time.tv_sec)*1000 +
+				  (end_time.tv_usec - start_time.tv_usec)/1000);
 
   do_cleanups (old_cleanups);
 }
@@ -1554,8 +1560,8 @@
 			      end_time - start_time, 0);
 }
 
-void
-print_transfer_performance (struct ui_file *stream,
+static void
+print_transfer_performance_ms (struct ui_file *stream,
 			    unsigned long data_count,
 			    unsigned long write_count,
 			    unsigned long time_count)
@@ -1564,8 +1570,8 @@
   if (time_count > 0)
     {
       ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-			(data_count * 8) / time_count);
-      ui_out_text (uiout, " bits/sec");
+			1000 * data_count / time_count);
+      ui_out_text (uiout, " bytes/sec");
     }
   else
     {
@@ -1581,6 +1587,16 @@
   ui_out_text (uiout, ".\n");
 }
 
+void
+print_transfer_performance (struct ui_file *stream,
+			    unsigned long data_count,
+			    unsigned long write_count,
+			    unsigned long time_count)
+{
+  print_transfer_performance_ms(stream, data_count, write_count,
+				1000*time_count);
+}
+
 /* This function allows the addition of incrementally linked object files.
    It does not modify any state in the target, only in the debugger.  */
  /* Note: ezannoni 2000-04-13 This function/command used to have a

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: transfer-rate.diff --]
[-- Type: text/x-patch; name="transfer-rate.diff", Size: 3174 bytes --]

2005-07-08  Shaun Jackman  <sjackman@gmail.com>

	* gdb/symfile.c (print_transfer_performance_ms): New function.
	Use gettimeofday instead of date to obtain microsecond precision. 
	(print_transfer_performance): Call print_transfer_performance_ms.

--- ./gdb/symfile.c-	2004-06-24 15:09:34.000000000 -0700
+++ ./gdb/symfile.c	2005-07-08 16:25:05.000000000 -0700
@@ -54,6 +54,7 @@
 #include "gdb_string.h"
 #include "gdb_stat.h"
 #include <ctype.h>
+#include <sys/time.h>
 #include <time.h>
 
 #ifndef O_BINARY
@@ -92,6 +93,9 @@
 
 static void symbol_file_add_main_1 (char *args, int from_tty, int flags);
 
+static void print_transfer_performance_ms (struct ui_file *,
+    unsigned long, unsigned long, unsigned long);
+
 static void add_symbol_file_command (char *, int);
 
 static void add_shared_symbol_files_command (char *, int);
@@ -1460,7 +1464,7 @@
 {
   asection *s;
   bfd *loadfile_bfd;
-  time_t start_time, end_time;	/* Start and end times of download */
+  struct timeval start_time, end_time;	/* Start and end times of download */
   char *filename;
   struct cleanup *old_cleanups;
   char *offptr;
@@ -1512,11 +1516,11 @@
   bfd_map_over_sections (loadfile_bfd, add_section_size_callback,
 			 (void *) &cbdata.total_size);
 
-  start_time = time (NULL);
+  gettimeofday(&start_time, NULL);
 
   bfd_map_over_sections (loadfile_bfd, load_section_callback, &cbdata);
 
-  end_time = time (NULL);
+  gettimeofday(&end_time, NULL);
 
   entry = bfd_get_start_address (loadfile_bfd);
   ui_out_text (uiout, "Start address ");
@@ -1534,8 +1538,10 @@
      file is loaded in.  Some targets do (e.g., remote-vx.c) but
      others don't (or didn't - perhaphs they have all been deleted).  */
 
-  print_transfer_performance (gdb_stdout, cbdata.data_count,
-			      cbdata.write_count, end_time - start_time);
+  print_transfer_performance_ms (gdb_stdout, cbdata.data_count,
+			      cbdata.write_count,
+				  (end_time.tv_sec - start_time.tv_sec)*1000 +
+				  (end_time.tv_usec - start_time.tv_usec)/1000);
 
   do_cleanups (old_cleanups);
 }
@@ -1554,8 +1560,8 @@
 			      end_time - start_time, 0);
 }
 
-void
-print_transfer_performance (struct ui_file *stream,
+static void
+print_transfer_performance_ms (struct ui_file *stream,
 			    unsigned long data_count,
 			    unsigned long write_count,
 			    unsigned long time_count)
@@ -1564,8 +1570,8 @@
   if (time_count > 0)
     {
       ui_out_field_fmt (uiout, "transfer-rate", "%lu",
-			(data_count * 8) / time_count);
-      ui_out_text (uiout, " bits/sec");
+			1000 * data_count / time_count);
+      ui_out_text (uiout, " bytes/sec");
     }
   else
     {
@@ -1581,6 +1587,16 @@
   ui_out_text (uiout, ".\n");
 }
 
+void
+print_transfer_performance (struct ui_file *stream,
+			    unsigned long data_count,
+			    unsigned long write_count,
+			    unsigned long time_count)
+{
+  print_transfer_performance_ms(stream, data_count, write_count,
+				1000*time_count);
+}
+
 /* This function allows the addition of incrementally linked object files.
    It does not modify any state in the target, only in the debugger.  */
 /* Note: ezannoni 2000-04-13 This function/command used to have a

Re: gdb/symfile.c: Transfer rate calculation

[Shaun Jackman]

Ping?

Cheers,
Shaun

On 2005-07-08, Shaun Jackman <sjackman@gmail.com> wrote:
> The transfer rate calculation in print_transfer_performance uses a
> time unit of whole seconds. This gross time unit limits the accuracy
> of the calculation, since the number of elapsed whole seconds usually
> has only one significant digit. This patch adds
> print_transfer_performance_ms which uses a time unit of milliseconds.
> It leaves the semantics of print_transfer_performance unchanged
> because gdb/m32r-rom.c and gdb/remote-m32r-sdi.c use it.
> 
> Cheers,
> Shaun
> 
> 2005-07-08  Shaun Jackman  <sjackman@gmail.com>
> 
>         * gdb/symfile.c (print_transfer_performance_ms): New function.
>         Use gettimeofday instead of date to obtain microsecond precision.
>         (print_transfer_performance): Call print_transfer_performance_ms.
[patch clipped]

Re: gdb/symfile.c: Transfer rate calculation

[Daniel Jacobowitz]

On Fri, Jul 08, 2005 at 04:34:51PM -0700, Shaun Jackman wrote:
> The transfer rate calculation in print_transfer_performance uses a
> time unit of whole seconds. This gross time unit limits the accuracy
> of the calculation, since the number of elapsed whole seconds usually
> has only one significant digit. This patch adds
> print_transfer_performance_ms which uses a time unit of milliseconds.
> It leaves the semantics of print_transfer_performance unchanged
> because gdb/m32r-rom.c and gdb/remote-m32r-sdi.c use it.

I'd been planning to whine about the unconditional use of gettimeofday,
but we already do, so I figure it must be portable enough.  Ditto
<sys/time.h>.  There's no point in the extra wrappers, though.

Could you test this patch for me, please?  I don't have any affected
targets easily accessible.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-07-31  Daniel Jacobowitz  <dan@codesourcery.com>

	Suggested by Shaun Jackman <sjackman@gmail.com>:
	* defs.h (print_transfer_performance): Update prototype.
	* m32r-rom.c (m32r_load, m32r_upload_command): Use gettimeofday
	for print_transfer_performance.
	* remote-m32r-sdi.c (m32r_load): Likewise.
	* symfile.c (generic_load): Likewise.
	(report_transfer_performance): Create a dummy struct timeval.
	(print_transfer_performance): Use a more accurate measure
	of performance.

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.187
diff -u -p -r1.187 defs.h
--- defs.h	31 Jul 2005 20:56:25 -0000	1.187
+++ defs.h	1 Aug 2005 02:52:54 -0000
@@ -552,10 +552,12 @@ extern void symbol_file_command (char *,
 extern void generic_load (char *name, int from_tty);
 
 /* Summarise a download */
+struct timeval;
 extern void print_transfer_performance (struct ui_file *stream,
 					unsigned long data_count,
 					unsigned long write_count,
-					unsigned long time_count);
+					const struct timeval *start_time,
+					const struct timeval *end_time);
 
 /* From top.c */
 
Index: m32r-rom.c
===================================================================
RCS file: /cvs/src/src/gdb/m32r-rom.c,v
retrieving revision 1.24
diff -u -p -r1.24 m32r-rom.c
--- m32r-rom.c	17 Feb 2005 13:49:53 -0000	1.24
+++ m32r-rom.c	1 Aug 2005 02:52:54 -0000
@@ -1,8 +1,8 @@
 /* Remote debugging interface to m32r and mon2000 ROM monitors for GDB, 
    the GNU debugger.
 
-   Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2004 Free Software
-   Foundation, Inc.
+   Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2004, 2005
+   Free Software Foundation, Inc.
 
    Adapted by Michael Snyder of Cygnus Support.
 
@@ -35,6 +35,7 @@
 #include "command.h"
 #include "gdbcmd.h"
 #include "symfile.h"		/* for generic load */
+#include <sys/time.h>
 #include <time.h>		/* for time_t */
 #include "gdb_string.h"
 #include "objfiles.h"		/* for ALL_OBJFILES etc. */
@@ -119,7 +120,7 @@ m32r_load (char *filename, int from_tty)
   bfd *abfd;
   asection *s;
   unsigned int i, data_count = 0;
-  time_t start_time, end_time;	/* for timing of download */
+  struct timeval start_time, end_time;
 
   if (filename == NULL || filename[0] == 0)
     filename = get_exec_file (1);
@@ -129,7 +130,7 @@ m32r_load (char *filename, int from_tty)
     error (_("Unable to open file %s."), filename);
   if (bfd_check_format (abfd, bfd_object) == 0)
     error (_("File is not an object file."));
-  start_time = time (NULL);
+  gettimeofday (&start_time, NULL);
 #if 0
   for (s = abfd->sections; s; s = s->next)
     if (s->flags & SEC_LOAD)
@@ -163,10 +164,10 @@ m32r_load (char *filename, int from_tty)
       return;
     }
 #endif
-  end_time = time (NULL);
+  gettimeofday (&end_time, NULL);
   printf_filtered ("Start address 0x%lx\n", bfd_get_start_address (abfd));
-  print_transfer_performance (gdb_stdout, data_count, 0,
-			      end_time - start_time);
+  print_transfer_performance (gdb_stdout, data_count, 0, &start_time,
+			      &end_time);
 
   /* Finally, make the PC point at the start address */
   if (exec_bfd)
@@ -405,7 +406,7 @@ m32r_upload_command (char *args, int fro
 {
   bfd *abfd;
   asection *s;
-  time_t start_time, end_time;	/* for timing of download */
+  struct timeval start_time, end_time;
   int resp_len, data_count = 0;
   char buf[1024];
   struct hostent *hostent;
@@ -467,7 +468,7 @@ m32r_upload_command (char *args, int fro
 	  ("Need to know default download path (use 'set download-path')");
     }
 
-  start_time = time (NULL);
+  gettimeofday (&start_time, NULL);
   monitor_printf ("uhip %s\r", server_addr);
   resp_len = monitor_expect_prompt (buf, sizeof (buf));	/* parse result? */
   monitor_printf ("ulip %s\r", board_addr);
@@ -491,7 +492,7 @@ m32r_upload_command (char *args, int fro
   else
     printf_filtered (" -- Ethernet load complete.\n");
 
-  end_time = time (NULL);
+  gettimeofday (&end_time, NULL);
   abfd = bfd_openr (args, 0);
   if (abfd != NULL)
     {				/* Download is done -- print section statistics */
@@ -517,8 +518,8 @@ m32r_upload_command (char *args, int fro
       /* Finally, make the PC point at the start address */
       write_pc (bfd_get_start_address (abfd));
       printf_filtered ("Start address 0x%lx\n", bfd_get_start_address (abfd));
-      print_transfer_performance (gdb_stdout, data_count, 0,
-				  end_time - start_time);
+      print_transfer_performance (gdb_stdout, data_count, 0, &start_time,
+				  &end_time);
     }
   inferior_ptid = null_ptid;	/* No process now */
 
Index: remote-m32r-sdi.c
===================================================================
RCS file: /cvs/src/src/gdb/remote-m32r-sdi.c,v
retrieving revision 1.12
diff -u -p -r1.12 remote-m32r-sdi.c
--- remote-m32r-sdi.c	1 May 2005 19:58:54 -0000	1.12
+++ remote-m32r-sdi.c	1 Aug 2005 02:52:54 -0000
@@ -1213,7 +1213,7 @@ m32r_load (char *args, int from_tty)
   char *filename;
   int quiet;
   int nostart;
-  time_t start_time, end_time;	/* Start and end times of download */
+  struct timeval start_time, end_time;
   unsigned long data_count;	/* Number of bytes transferred to memory */
   int ret;
   static RETSIGTYPE (*prev_sigint) ();
@@ -1263,7 +1263,7 @@ m32r_load (char *args, int from_tty)
     error (_("\"%s\" is not an object file: %s"), filename,
 	   bfd_errmsg (bfd_get_error ()));
 
-  start_time = time (NULL);
+  gettimeofday (&start_time, NULL);
   data_count = 0;
 
   interrupted = 0;
@@ -1349,7 +1349,7 @@ m32r_load (char *args, int from_tty)
   interrupted = 0;
   signal (SIGINT, prev_sigint);
 
-  end_time = time (NULL);
+  gettimeofday (&end_time, NULL);
 
   /* Make the PC point at the start address */
   if (exec_bfd)
@@ -1373,8 +1373,8 @@ m32r_load (char *args, int from_tty)
 	printf_unfiltered ("[Starting %s at 0x%lx]\n", filename, entry);
     }
 
-  print_transfer_performance (gdb_stdout, data_count, 0,
-			      end_time - start_time);
+  print_transfer_performance (gdb_stdout, data_count, 0, &start_time,
+			      &end_time);
 
   do_cleanups (old_chain);
 }
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.160
diff -u -p -r1.160 symfile.c
--- symfile.c	13 Jun 2005 18:39:11 -0000	1.160
+++ symfile.c	1 Aug 2005 02:52:54 -0000
@@ -1,7 +1,7 @@
 /* Generic symbol file reading for the GNU debugger, GDB.
 
    Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998,
-   1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
+   1999, 2000, 2001, 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
 
    Contributed by Cygnus Support, using pieces from other GDB modules.
 
@@ -57,6 +57,7 @@
 #include "gdb_stat.h"
 #include <ctype.h>
 #include <time.h>
+#include <sys/time.h>
 
 #ifndef O_BINARY
 #define O_BINARY 0
@@ -1600,7 +1601,7 @@ generic_load (char *args, int from_tty)
 {
   asection *s;
   bfd *loadfile_bfd;
-  time_t start_time, end_time;	/* Start and end times of download */
+  struct timeval start_time, end_time;
   char *filename;
   struct cleanup *old_cleanups;
   char *offptr;
@@ -1652,11 +1653,11 @@ generic_load (char *args, int from_tty)
   bfd_map_over_sections (loadfile_bfd, add_section_size_callback,
 			 (void *) &cbdata.total_size);
 
-  start_time = time (NULL);
+  gettimeofday (&start_time, NULL);
 
   bfd_map_over_sections (loadfile_bfd, load_section_callback, &cbdata);
 
-  end_time = time (NULL);
+  gettimeofday (&end_time, NULL);
 
   entry = bfd_get_start_address (loadfile_bfd);
   ui_out_text (uiout, "Start address ");
@@ -1675,7 +1676,7 @@ generic_load (char *args, int from_tty)
      others don't (or didn't - perhaps they have all been deleted).  */
 
   print_transfer_performance (gdb_stdout, cbdata.data_count,
-			      cbdata.write_count, end_time - start_time);
+			      cbdata.write_count, &start_time, &end_time);
 
   do_cleanups (old_cleanups);
 }
@@ -1690,21 +1691,35 @@ void
 report_transfer_performance (unsigned long data_count, time_t start_time,
 			     time_t end_time)
 {
-  print_transfer_performance (gdb_stdout, data_count,
-			      end_time - start_time, 0);
+  struct timeval start, end;
+
+  start.tv_sec = start_time;
+  start.tv_usec = 0;
+  end.tv_sec = end_time;
+  end.tv_usec = 0;
+
+  print_transfer_performance (gdb_stdout, data_count, 0, &start, &end);
 }
 
 void
 print_transfer_performance (struct ui_file *stream,
 			    unsigned long data_count,
 			    unsigned long write_count,
-			    unsigned long time_count)
+			    const struct timeval *start_time,
+			    const struct timeval *end_time)
 {
+  unsigned long time_count;
+
+  /* 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",
-			(data_count * 8) / time_count);
+			1000 * (data_count * 8) / time_count);
       ui_out_text (uiout, " bits/sec");
     }
   else

Re: gdb/symfile.c: Transfer rate calculation

[Shaun Jackman]

On 7/31/05, Daniel Jacobowitz <drow@false.org> wrote:
> I'd been planning to whine about the unconditional use of gettimeofday,
> but we already do, so I figure it must be portable enough.  Ditto
> <sys/time.h>.  There's no point in the extra wrappers, though.
> 
> Could you test this patch for me, please?  I don't have any affected
> targets easily accessible.

This patch worked for me. Thanks, Daniel!

Cheers,
Shaun

Re: gdb/symfile.c: Transfer rate calculation

[Daniel Jacobowitz]

On Mon, Aug 01, 2005 at 01:40:53PM -0600, Shaun Jackman wrote:
> On 7/31/05, Daniel Jacobowitz <drow@false.org> wrote:
> > I'd been planning to whine about the unconditional use of gettimeofday,
> > but we already do, so I figure it must be portable enough.  Ditto
> > <sys/time.h>.  There's no point in the extra wrappers, though.
> > 
> > Could you test this patch for me, please?  I don't have any affected
> > targets easily accessible.
> 
> This patch worked for me. Thanks, Daniel!

No problem.  I've committed the patch.

-- 
Daniel Jacobowitz
CodeSourcery, LLC