Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Anton Blanchard <anton@samba.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Improve performance of large restore commands
Date: Thu, 25 Jul 2013 17:59:00 -0000	[thread overview]
Message-ID: <51F16780.70408@redhat.com> (raw)
In-Reply-To: <20130725220858.58184193@kryten>

On 07/25/2013 01:08 PM, Anton Blanchard wrote:
> 
> I noticed a large (100MB) restore took hours to complete. The problem
> is target_xfer_partial repeatedly mallocs and memcpys the entire
> 100MB buffer only to find a small portion of it is actually written.

I think you meant memory_xfer_partial, in the breakpoint shadow
handling, right?  I'd prefer pushing the capping close to the offending
malloc/memcpy.

We could conceivably change that shadowing algorithm to not malloc
at all.  E.g., say, with a memory block like

  |------B------|
start          end

with B being the address where a breakpoint is supposed to be planted,
write block [start,B), then a write for the breakpoint instruction
at B, then another block write for (B,e).

Or, we could throttle the requested window width up/down depending
on the buffer size returned at each partial transfer.

I'm not actually suggesting doing this, only explaining why I'd
rather put the cap close to the problem it solves.  target_write_partial
is used for other targets objects too, not just memory.

> We already cap reads to 4K

Where exactly?  In the target backend, perhaps?  I'm not finding a
cap at the target.c level.

> --
> 
> 2013-07-25  Anton Blanchard  <anton@samba.org>
> 
> 	* target.c (target_write_with_progress): Cap write to 4K

Period at end of sentence.

> 
> Index: b/gdb/target.c
> ===================================================================
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2287,9 +2287,11 @@ target_write_with_progress (struct targe
>  
>    while (xfered < len)
>      {
> +      /* Cap the write to 4K */
> +      int to_transfer = min(4096, len - xfered);
>        LONGEST xfer = target_write_partial (ops, object, annex,

Empty line after last declaration.  Missing space before parens.

>  					   (gdb_byte *) buf + xfered,
> -					   offset + xfered, len - xfered);
> +					   offset + xfered, to_transfer);
>  
>        if (xfer == 0)
>  	return xfered;
> 

-- 
Pedro Alves


  parent reply	other threads:[~2013-07-25 17:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-25 12:09 Anton Blanchard
2013-07-25 17:30 ` Luis Machado
2013-07-25 17:59 ` Pedro Alves [this message]
2013-07-29  5:45   ` Anton Blanchard
2013-07-29 14:14     ` Pedro Alves
2013-07-29 23:25       ` Anton Blanchard
2013-07-30 11:33         ` Pedro Alves
2013-07-31 12:35           ` Anton Blanchard
2013-07-31 14:47             ` Pedro Alves

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=51F16780.70408@redhat.com \
    --to=palves@redhat.com \
    --cc=anton@samba.org \
    --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