Optimize memory_xfer_partial for remote

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 were 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 in commit 67c059c29e1f ("Improve performance of large restore
commands").

The 4K cap was introduced because in a case where the restore command
requested a 100MB transfer, memory_xfer_partial would repeatedy
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 implements a new target function that returns that target's
limit on memory transfer size.  It defaults to ULONGEST_MAX bytes,
because for native targets there is no marshaling and thus no limit is
needed.  For remote targets it uses get_memory_write_packet_size.

gdb/ChangeLog:

	* remote.c (remote_get_memory_xfer_limit): New function.
	* target-delegates.c: Regenerate.
	* target.c (memory_xfer_partial): Call
	target_ops.to_get_memory_xfer_limit.
	* target.h (struct target_ops)
	<to_get_memory_xfer_limit>: New member.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 58c9c787aa011720032c3a8dd0b1be9fe1fa37b4..85a54445bbbcb43b61185847f4aa34105871ade2 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2016-07-01  Don Breazeal  <donb@codesourcery.com>
+
+       * remote.c (remote_get_memory_xfer_limit): New function.
+       * target-delegates.c: Regenerate.
+       * target.c (memory_xfer_partial): Call
+       target_ops.to_get_memory_xfer_limit.
+       * target.h (struct target_ops) <to_get_memory_xfer_limit>: New
+       member.
+
 2016-07-01  John Baldwin  <jhb@FreeBSD.org>
 
        * fbsd-nat.c (struct fbsd_fork_child_info): Rename to ...

diff --git a/gdb/remote.c b/gdb/remote.c
index e4b2095a40dced437613bdf22549ad58ffad6f23..5568346688d1e8c388b93fa7f2ea652136e08c42 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10152,6 +10152,14 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
   return TARGET_XFER_OK;
 }
 
+/* Implementation of to_get_memory_xfer_limit.  */
+
+static ULONGEST
+remote_get_memory_xfer_limit (struct target_ops *ops)
+{
+  return get_memory_write_packet_size ();
+}
+
 static int
 remote_search_memory (struct target_ops* ops,
                      CORE_ADDR start_addr, ULONGEST search_space_len,
@@ -13065,6 +13073,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_interrupt = remote_interrupt;
   remote_ops.to_pass_ctrlc = remote_pass_ctrlc;
   remote_ops.to_xfer_partial = remote_xfer_partial;
+  remote_ops.to_get_memory_xfer_limit = remote_get_memory_xfer_limit;
   remote_ops.to_rcmd = remote_rcmd;
   remote_ops.to_pid_to_exec_file = remote_pid_to_exec_file;
   remote_ops.to_log_command = serial_log_command;

diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 03aa2cc51002e5697f6fc5c963af998b59fe2325..288703373ea08dfe9a0e486b30f703ce5a30ee97 100644 (file)
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -2064,6 +2064,33 @@ debug_xfer_partial (struct target_ops *self, enum target_object arg1, const char
   return result;
 }
 
+static ULONGEST
+delegate_get_memory_xfer_limit (struct target_ops *self)
+{
+  self = self->beneath;
+  return self->to_get_memory_xfer_limit (self);
+}
+
+static ULONGEST
+tdefault_get_memory_xfer_limit (struct target_ops *self)
+{
+  return ULONGEST_MAX;
+}
+
+static ULONGEST
+debug_get_memory_xfer_limit (struct target_ops *self)
+{
+  ULONGEST result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_get_memory_xfer_limit (...)\n", debug_target.to_shortname);
+  result = debug_target.to_get_memory_xfer_limit (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_get_memory_xfer_limit (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_ULONGEST (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 static VEC(mem_region_s) *
 delegate_memory_map (struct target_ops *self)
 {
@@ -4223,6 +4250,8 @@ install_delegators (struct target_ops *ops)
     ops->to_get_thread_local_address = delegate_get_thread_local_address;
   if (ops->to_xfer_partial == NULL)
     ops->to_xfer_partial = delegate_xfer_partial;
+  if (ops->to_get_memory_xfer_limit == NULL)
+    ops->to_get_memory_xfer_limit = delegate_get_memory_xfer_limit;
   if (ops->to_memory_map == NULL)
     ops->to_memory_map = delegate_memory_map;
   if (ops->to_flash_erase == NULL)
@@ -4454,6 +4483,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_goto_bookmark = tdefault_goto_bookmark;
   ops->to_get_thread_local_address = tdefault_get_thread_local_address;
   ops->to_xfer_partial = tdefault_xfer_partial;
+  ops->to_get_memory_xfer_limit = tdefault_get_memory_xfer_limit;
   ops->to_memory_map = tdefault_memory_map;
   ops->to_flash_erase = tdefault_flash_erase;
   ops->to_flash_done = tdefault_flash_done;
@@ -4610,6 +4640,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_goto_bookmark = debug_goto_bookmark;
   ops->to_get_thread_local_address = debug_get_thread_local_address;
   ops->to_xfer_partial = debug_xfer_partial;
+  ops->to_get_memory_xfer_limit = debug_get_memory_xfer_limit;
   ops->to_memory_map = debug_memory_map;
   ops->to_flash_erase = debug_flash_erase;
   ops->to_flash_done = debug_flash_done;

diff --git a/gdb/target.c b/gdb/target.c
index d3fc35ba14571eccc0b8fb1d31d12a1db69f1e22..622836135f255c552a1d7761aa1a1c840b39e61e 100644 (file)
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1301,8 +1301,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 a limit specified by the target
+        to mitigate this.  */
+      len = min (ops->to_get_memory_xfer_limit (ops), len);
 
       buf = (gdb_byte *) xmalloc (len);
       old_chain = make_cleanup (xfree, buf);

diff --git a/gdb/target.h b/gdb/target.h
index fc317e37d51146880d2ba303f23c978c727752d2..15fd806ad94acbedcc67e0a04092964f94043b17 100644 (file)
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -745,6 +745,12 @@ struct target_ops
                                                ULONGEST *xfered_len)
       TARGET_DEFAULT_RETURN (TARGET_XFER_E_IO);
 
+    /* Return the limit on the size of any single memory transfer
+       for the target.  */
+
+    ULONGEST (*to_get_memory_xfer_limit) (struct target_ops *)
+      TARGET_DEFAULT_RETURN (ULONGEST_MAX);
+
     /* Returns the memory map for the target.  A return value of NULL
        means that no memory map is available.  If a memory address
        does not fall within any returned regions, it's assumed to be