[PATCH] Optimize memory_xfer_partial for remote

[Don Breazeal <donb at codesourcery dot com>]

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  <donb@codesourcery.com>

	* 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
-- 
2.8.1