From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80497 invoked by alias); 20 Jun 2016 15:31:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 80486 invoked by uid 89); 20 Jun 2016 15:31:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Request, encountered, Limit, continually X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 20 Jun 2016 15:31:19 +0000 Received: from svr-orw-fem-03.mgc.mentorg.com ([147.34.97.39]) by relay1.mentorg.com with esmtp id 1bF1AT-0006qU-8B from Don_Breazeal@mentor.com for gdb-patches@sourceware.org; Mon, 20 Jun 2016 08:31:17 -0700 Received: from [172.30.1.26] (147.34.91.1) by SVR-ORW-FEM-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 20 Jun 2016 08:31:16 -0700 Subject: [PING] [PATCH] Optimize memory_xfer_partial for remote To: References: <1464980562-24184-1-git-send-email-donb@codesourcery.com> From: Don Breazeal Message-ID: <57680C3D.5090903@codesourcery.com> Date: Mon, 20 Jun 2016 15:31:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1464980562-24184-1-git-send-email-donb@codesourcery.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00330.txt.bz2 Ping On 6/3/2016 12:02 PM, Don Breazeal wrote: > Some analysis we did here showed that increasing the cap on the > transfer size in target.c:memory_xfer_partial could give 20% or more > improvement in remote load across JTAG. Transfer sizes are capped > to 4K bytes because of performance problems encountered with the > restore command, documented here: > > https://sourceware.org/ml/gdb-patches/2013-07/msg00611.html > > and with commit hash: 67c059c29e1fb0cdeacdd2005f955514d8d1fb34 > > The 4K cap was introduced because in a case where the restore command > requested a 100MB transfer, memory_xfer_partial would allocate and copy > an entire 100MB buffer in order to properly handle breakpoint shadow > instructions, even though memory_xfer_partial would actually only > write a small portion of the buffer contents. > > A couple of alternative solutions were suggested: > * change the algorithm for handling the breakpoint shadow instructions > * throttle the transfer size up or down based on the previous actual > transfer size > > I tried implementing the throttling approach, and my implementation > reduced the performance in some cases. > > This patch takes a simple approach: instead of hard-coding the cap on > transfer requests to 4096, use a variable and allow the target to set it. > This allows the remote target to set the cap to its packetsize. > > Here is the performance for a 100MB restore command using an srec file > (minutes:seconds), where the remote has a packetsize of 16K bytes: > * existing implementation: 7:50 > * proposed implementation: 5:34 > > We could make a similar change in target_read_alloc_1 and > target_fileio_read_alloc_1, but I left that alone for now. > > I considered making target_set_memory_xfer_limit a function in the target > vector, but concluded that was overkill. In this patch it is an external > function in target.c. > > Tested on x86_64 Linux with native and native-gdbserver, and manually > tested 'restore' on a Windows 7 host with a bare-metal ARM board. > > Thanks, > --Don > > gdb/ChangeLog: > 2016-06-03 Don Breazeal > > * remote.c (remote_start_remote): Call > target_set_memory_xfer_limit. > * target.c (memory_xfer_limit): New static variable. > (target_set_memory_xfer_limit): New function. > (memory_xfer_partial): Use memory_xfer_limit in place of > constant. > * target.h (target_set_memory_xfer_limit): Declare new function. > > --- > gdb/remote.c | 3 +++ > gdb/target.c | 19 +++++++++++++++++-- > gdb/target.h | 6 ++++++ > 3 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 1f0d67c..028d555 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -4079,6 +4079,9 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p) > getpkt (&rs->buf, &rs->buf_size, 0); > } > > + /* Set the cap on memory transfer requests to our packet size. */ > + target_set_memory_xfer_limit (get_remote_packet_size ()); > + > /* Let the target know which signals it is allowed to pass down to > the program. */ > update_signals_program_target (); > diff --git a/gdb/target.c b/gdb/target.c > index c0ce46d..dde71c2 100644 > --- a/gdb/target.c > +++ b/gdb/target.c > @@ -162,6 +162,10 @@ int may_insert_fast_tracepoints = 1; > > int may_stop = 1; > > +/* Limit on size of memory transfers, see memory_xfer_partial. */ > + > +static ULONGEST memory_xfer_limit = 4096; > + > /* Non-zero if we want to see trace of target level stuff. */ > > static unsigned int targetdebug = 0; > @@ -1235,6 +1239,16 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, > return res; > } > > +/* Set the cap on actual memory transfer requests. This prevents > + repeated requests to transfer much more than the transport > + mechanism can accommodate. See memory_xfer_partial. */ > + > +void > +target_set_memory_xfer_limit (ULONGEST new_limit) > +{ > + memory_xfer_limit = new_limit; > +} > + > /* Perform a partial memory transfer. For docs see target.h, > to_xfer_partial. */ > > @@ -1269,8 +1283,9 @@ memory_xfer_partial (struct target_ops *ops, enum target_object object, > by memory_xfer_partial_1. We will continually malloc > and free a copy of the entire write request for breakpoint > shadow handling even though we only end up writing a small > - subset of it. Cap writes to 4KB to mitigate this. */ > - len = min (4096, len); > + subset of it. Cap writes to the value of memory_xfer_limit > + to mitigate this. */ > + len = min (memory_xfer_limit, len); > > buf = (gdb_byte *) xmalloc (len); > old_chain = make_cleanup (xfree, buf); > diff --git a/gdb/target.h b/gdb/target.h > index 6b5b6e0..b511746 100644 > --- a/gdb/target.h > +++ b/gdb/target.h > @@ -266,6 +266,12 @@ enum target_xfer_status > const gdb_byte *writebuf, ULONGEST memaddr, > LONGEST len, ULONGEST *xfered_len); > > +/* Set the cap on actual memory transfer requests. This prevents > + repeated requests to transfer much more than the transport > + mechanism can accommodate. See memory_xfer_partial. */ > + > +extern void target_set_memory_xfer_limit (ULONGEST new_limit); > + > /* Request that OPS transfer up to LEN addressable units of the target's > OBJECT. When reading from a memory object, the size of an addressable unit > is architecture dependent and can be found using >