From patchwork Fri Jun 3 19:02:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 12755 Received: (qmail 49391 invoked by alias); 3 Jun 2016 19:02:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 49377 invoked by uid 89); 3 Jun 2016 19:02:55 -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=cap, Limit, Cap, non-zero 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; Fri, 03 Jun 2016 19:02:45 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1b8uMk-0005eQ-TW from Don_Breazeal@mentor.com for gdb-patches@sourceware.org; Fri, 03 Jun 2016 12:02:42 -0700 Received: from build5-trusty-cs (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.3.224.2; Fri, 3 Jun 2016 12:02:42 -0700 Received: by build5-trusty-cs (Postfix, from userid 1905) id 701882A7C10; Fri, 3 Jun 2016 12:02:42 -0700 (PDT) From: Don Breazeal To: Subject: [PATCH] Optimize memory_xfer_partial for remote Date: Fri, 3 Jun 2016 12:02:42 -0700 Message-ID: <1464980562-24184-1-git-send-email-donb@codesourcery.com> MIME-Version: 1.0 X-IsSubscribed: yes 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