This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] Optimize memory_xfer_partial for remote
- From: Pedro Alves <palves at redhat dot com>
- To: Don Breazeal <donb at codesourcery dot com>, gdb-patches at sourceware dot org, qiyaoltc at gmail dot com
- Date: Fri, 24 Jun 2016 23:23:16 +0100
- Subject: Re: [PATCH v2] Optimize memory_xfer_partial for remote
- Authentication-results: sourceware.org; auth=none
- References: <1466803274-62026-1-git-send-email-donb at codesourcery dot com>
On 06/24/2016 10:21 PM, Don Breazeal wrote:
>
> and with commit hash: 67c059c29e1fb0cdeacdd2005f955514d8d1fb34
>
Write:
... with commit 67c059c29e1f ("Improve performance of large restore
commands") ...
so the reader has a clue what the commit is about without having
to check.
> gdb/ChangeLog:
> 2016-06-24 Don Breazeal <donb@codesourcery.com>
>
> * remote.c (remote_get_memory_xfer_limit): New function.
> * target-delegates.c (delegate_get_memory_xfer_limit,
> debug_get_memory_xfer_limit, install_delegators,
> install_dummy_methods, init_debug_target): New functions
> and target_ops initialization from regenerating the file.
The standard practice is to just say:
* target-delegates.c: Regenerate.
> * target.c (default_get_memory_xfer_limit): New function and
> forward declaration.
> (memory_xfer_partial): Call target_ops.to_get_memory_xfer_limit.
> * target.h (struct target_ops)<to_get_memory_xfer_limit>: New
> member.
Space between ")<".
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 501f3c6..03c7ab7 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -10160,6 +10160,12 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
> return TARGET_XFER_OK;
> }
>
> +static ULONGEST
> +remote_get_memory_xfer_limit (struct target_ops *ops)
Intro comment, something like "Implementation of ... method.".
>
> +/* The default implementation for the to_get_memory_xfer_limit method.
> + The hard-coded limit here was determined to be a reasonable default
> + that eliminated exponential slowdown on very large transfers without
> + unduly compromising performance on smaller transfers. */
Where's this coming from? Is this new experimentation you did,
or are you talking about Anton's patch?
> @@ -1301,8 +1314,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);
>
Does this still work if remote is not the top-most target?
E.g., what happens if you do "record" to push a record_statum
target on top? Do we get the 4KB default limit, or the
remote limit?
Thanks,
Pedro Alves