[PATCH] Speed up target_read_string

[PATCH] Speed up target_read_string

[Luis Machado]

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

Hi,

The following change is aimed at increasing the performance of 
target_read_string for remote debugging. To accomplish that, the buffer 
has been increased to 64 bytes and we now use target_read_partial 
instead of target_read_memory.

The increase in the buffer size reduces the impact of the remote 
transfer overhead.

Luis

[-- Attachment #2: target_read_string.diff --]
[-- Type: text/x-patch, Size: 2708 bytes --]

2011-08-19  Pedro Alves  <pedro@codesourcery.com>
	    Luis Machado  <lgustavo@codesourcery.com>

	* target.c (target_read_partial): Declare prototype.
	(target_read_string): Use 64-bytes blocks for reads and use
	target_read_partial instead of target_read_memory.

Index: gdb/target.c
===================================================================
--- gdb/target.c.orig	2011-08-18 12:13:58.773749146 -0300
+++ gdb/target.c	2011-08-19 11:13:00.297749146 -0300
@@ -88,6 +88,11 @@
 				    void *readbuf, const void *writebuf,
 				    ULONGEST offset, LONGEST len);
 
+static LONGEST target_read_partial (struct target_ops *ops,
+				    enum target_object object,
+				    const char *annex, gdb_byte *buf,
+				    ULONGEST offset, LONGEST len);
+
 static struct gdbarch *default_thread_architecture (struct target_ops *ops,
 						    ptid_t ptid);
 
@@ -1185,55 +1190,51 @@
 int
 target_read_string (CORE_ADDR memaddr, char **string, int len, int *errnop)
 {
-  int tlen, origlen, offset, i;
-  gdb_byte buf[4];
-  int errcode = 0;
-  char *buffer;
-  int buffer_allocated;
-  char *bufptr;
-  unsigned int nbytes_read = 0;
+  int tlen, i, errcode = 0, buffer_allocated;
+  gdb_byte buf[64];
+  char *buffer, *bufptr;
+  unsigned int nbytes_read = 0, raw_read = 0;
+  LONGEST xfered;
 
   gdb_assert (string);
 
   /* Small for testing.  */
-  buffer_allocated = 4;
+  buffer_allocated = 64;
   buffer = xmalloc (buffer_allocated);
   bufptr = buffer;
 
-  origlen = len;
-
   while (len > 0)
     {
-      tlen = MIN (len, 4 - (memaddr & 3));
-      offset = memaddr & 3;
+      tlen = MIN (len, 64);
 
-      errcode = target_read_memory (memaddr & ~3, buf, sizeof buf);
-      if (errcode != 0)
+      xfered = target_read_partial (current_target.beneath,
+				    TARGET_OBJECT_MEMORY,
+				    NULL, buf,
+				    memaddr, tlen);
+      if (xfered <= 0)
 	{
 	  /* The transfer request might have crossed the boundary to an
 	     unallocated region of memory.  Retry the transfer, requesting
 	     a single byte.  */
 	  tlen = 1;
-	  offset = 0;
 	  errcode = target_read_memory (memaddr, buf, 1);
 	  if (errcode != 0)
 	    goto done;
 	}
+      else
+	raw_read += tlen;
 
-      if (bufptr - buffer + tlen > buffer_allocated)
+      if (raw_read > buffer_allocated)
 	{
-	  unsigned int bytes;
-
-	  bytes = bufptr - buffer;
 	  buffer_allocated *= 2;
 	  buffer = xrealloc (buffer, buffer_allocated);
-	  bufptr = buffer + bytes;
+	  bufptr = buffer + raw_read - tlen;
 	}
 
       for (i = 0; i < tlen; i++)
 	{
-	  *bufptr++ = buf[i + offset];
-	  if (buf[i + offset] == '\000')
+	  *bufptr++ = buf[i];
+	  if (buf[i] == '\000')
 	    {
 	      nbytes_read += i + 1;
 	      goto done;

Re: [PATCH] Speed up target_read_string

[Jan Kratochvil]

On Fri, 19 Aug 2011 19:11:02 +0200, Luis Machado wrote:
> The following change is aimed at increasing the performance of
> target_read_string for remote debugging. To accomplish that, the
> buffer has been increased to 64 bytes and we now use
> target_read_partial instead of target_read_memory.

As this is implementation of
	[RFC] Make target_read_string faster over high-latency links.
	http://sourceware.org/ml/gdb-patches/2011-07/msg00391.html

where Daniel had concerns about some embedded targets without a reply I guess
you are aware of these issues (I am not).


Thanks,
Jan

Re: [PATCH] Speed up target_read_string

[Daniel Jacobowitz]

On Sat, Aug 20, 2011 at 2:37 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Fri, 19 Aug 2011 19:11:02 +0200, Luis Machado wrote:
>> The following change is aimed at increasing the performance of
>> target_read_string for remote debugging. To accomplish that, the
>> buffer has been increased to 64 bytes and we now use
>> target_read_partial instead of target_read_memory.
>
> As this is implementation of
>        [RFC] Make target_read_string faster over high-latency links.
>        http://sourceware.org/ml/gdb-patches/2011-07/msg00391.html
>
> where Daniel had concerns about some embedded targets without a reply I guess
> you are aware of these issues (I am not).
>

For avoidance of doubt, I'll defer to Luis and Pedro on this topic.

-- 
Thanks,
Daniel