Re: [PATCH] Prelimit number of bytes to read in "vFile:pread:"

[Gary Benson <gbenson@redhat.com>]

Sandra, could you please try this patch on your Altera 3c120 and
on your PandaBoard?  I'm interested to know what the times are
now.

Cheers,
Gary

Gary Benson wrote:
> Pedro Alves wrote:
> > The fact that Gary's chunk size limiting patch made things much
> > faster on the nios2 board is still mysterious to me.  I'd expect
> > the slowness to be latency bound, given the request/response
> > nature of the RSP, but then I'd expect that more chunking would
> > slow things down, not speed it up.
> 
> I think I figured this out...
> 
> While handling "vFile:pread:" packets, gdbserver would read the
> number of bytes requested regardless of whether this would fit
> into the reply packet.  gdbserver would then return a packet's
> worth of data and discard the remainder.  When accessing large
> binaries GDB (via BFD) routinely makes large "vFile:pread:"
> requests, resulting in gdbserver allocating large unnecessary
> buffers and reading some portions of the file many times over.
> 
> This commit causes gdbserver to limit the number of bytes to be
> read to a sensible maximum prior to allocating buffers and reading
> data.
> 
> Built and regtested on RHEL 6.6 x86_64.
> 
> May I push this to HEAD and to the branch?
> 
> Thanks,
> Gary
> 
> ---
> gdb/gdbserver/ChangeLog:
> 
> 	* hostio.c (handle_pread): Do not attempt to read more data
> 	than hostio_reply_with_data can fit in a packet.
> ---
>  gdb/gdbserver/ChangeLog |    5 +++++
>  gdb/gdbserver/hostio.c  |   12 ++++++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/gdbserver/hostio.c b/gdb/gdbserver/hostio.c
> index b38a6bd..8788f07 100644
> --- a/gdb/gdbserver/hostio.c
> +++ b/gdb/gdbserver/hostio.c
> @@ -344,6 +344,7 @@ handle_pread (char *own_buf, int *new_packet_len)
>  {
>    int fd, ret, len, offset, bytes_sent;
>    char *p, *data;
> +  static int max_reply_size = -1;
>  
>    p = own_buf + strlen ("vFile:pread:");
>  
> @@ -359,6 +360,17 @@ handle_pread (char *own_buf, int *new_packet_len)
>        return;
>      }
>  
> +  /* Do not attempt to read more than the maximum number of bytes
> +     hostio_reply_with_data can fit in a packet.  We may still read
> +     too much because of escaping, but this is handled below.  */
> +  if (max_reply_size == -1)
> +    {
> +      sprintf (own_buf, "F%x;", PBUFSIZ);
> +      max_reply_size = PBUFSIZ - strlen (own_buf);
> +    }
> +  if (len > max_reply_size)
> +    len = max_reply_size;
> +
>    data = xmalloc (len);
>  #ifdef HAVE_PREAD
>    ret = pread (fd, data, len, offset);
> -- 
> 1.7.1
> 

-- 
http://gbenson.net/