diff mbox

Make remote transfers interruptible

Message ID 1439389814-29211-1-git-send-email-gbenson@redhat.com
State New, archived
Headers

Commit Message

Gary Benson Aug. 12, 2015, 2:30 p.m. UTC 
  Hi Sandra,

Sandra Loosemore wrote:
> On 08/05/2015 09:28 AM, Gary Benson wrote:
> > This commit makes it possible to interrupt slow remote file transfers.
> >
> > gdb/ChangeLog:
> >
> > 	* gdb_bfd.c (gdb_bfd_iovec_fileio_pread): Add QUIT call.
> 
> It still does not work for me.  :-(

Could you please try this newer version and see if it allows you to
interrupt the remote transfers?

Thanks,
Gary

---

Comments

Sandra Loosemore Aug. 12, 2015, 5:31 p.m. UTC | #1 
On 08/12/2015 08:30 AM, Gary Benson wrote:
> Hi Sandra,
>
> Sandra Loosemore wrote:
>> On 08/05/2015 09:28 AM, Gary Benson wrote:
>>> This commit makes it possible to interrupt slow remote file transfers.
>>>
>>> gdb/ChangeLog:
>>>
>>> 	* gdb_bfd.c (gdb_bfd_iovec_fileio_pread): Add QUIT call.
>>
>> It still does not work for me.  :-(
>
> Could you please try this newer version and see if it allows you to
> interrupt the remote transfers?

This version still doesn't make the transfer interruptable with ^C. 
*But*, with this patch, the startup time is reduced from 4 minutes to 19 
seconds.  Huh?  Is it really transferring the entire file contents, or 
was the time being used for some GDB-side operation that is quadratic or 
exponential in the size of the read requested rather than the actual 
byte transfer?  Independently of the ^C issue, I think we need to better 
understand what is going on here and better tune the code on both sides 
of the RSP for large file transfers.  Even if a user asks for 
target-side libraries explicitly, 4 minutes to transfer one library 
doesn't provide a good user experience, and 19 seconds isn't so great 
either when you consider that some interactive applications link with 
dozens of GUI or multimedia libraries and not just glibc.

-Sandra
Doug Evans Aug. 12, 2015, 5:39 p.m. UTC | #2 
On Wed, Aug 12, 2015 at 10:31 AM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> On 08/12/2015 08:30 AM, Gary Benson wrote:
>>
>> Hi Sandra,
>>
>> Sandra Loosemore wrote:
>>>
>>> On 08/05/2015 09:28 AM, Gary Benson wrote:
>>>>
>>>> This commit makes it possible to interrupt slow remote file transfers.
>>>>
>>>> gdb/ChangeLog:
>>>>
>>>>         * gdb_bfd.c (gdb_bfd_iovec_fileio_pread): Add QUIT call.
>>>
>>>
>>> It still does not work for me.  :-(
>>
>>
>> Could you please try this newer version and see if it allows you to
>> interrupt the remote transfers?
>
>
> This version still doesn't make the transfer interruptable with ^C. *But*,
> with this patch, the startup time is reduced from 4 minutes to 19 seconds.
> Huh?  Is it really transferring the entire file contents, or was the time
> being used for some GDB-side operation that is quadratic or exponential in
> the size of the read requested rather than the actual byte transfer?
> Independently of the ^C issue, I think we need to better understand what is
> going on here and better tune the code on both sides of the RSP for large
> file transfers.  Even if a user asks for target-side libraries explicitly, 4
> minutes to transfer one library doesn't provide a good user experience, and
> 19 seconds isn't so great either when you consider that some interactive
> applications link with dozens of GUI or multimedia libraries and not just
> glibc.

Dozens?  How about thousands.  1/2 :-)

[just want to make sure such cases are in the mindset of the community]
Gary Benson Aug. 13, 2015, 9:10 a.m. UTC | #3 
Sandra Loosemore wrote:
> On 08/12/2015 08:30 AM, Gary Benson wrote:
> >Sandra Loosemore wrote:
> > >On 08/05/2015 09:28 AM, Gary Benson wrote:
> > > > This commit makes it possible to interrupt slow remote file
> > > > transfers.
> > > >
> > > > gdb/ChangeLog:
> > > >
> > > >	* gdb_bfd.c (gdb_bfd_iovec_fileio_pread): Add QUIT call.
> > >
> > > It still does not work for me.  :-(
> >
> > Could you please try this newer version and see if it allows you
> > to interrupt the remote transfers?
> 
> This version still doesn't make the transfer interruptable with ^C.

Then I am out of ideas :( I can interrupt on my setup with either
patch (I tested it by adding a 100ms usleep in gdbserver/hostio.c
handle_pread).

I will push a warning-printing patch to HEAD, so that users in your
situation at least have some feedback on what is happening.  Could
you please look at making the transfers interruptible?

Joel, would you like me to also push it to the 7.10 branch, or will
you do that?

> *But*, with this patch, the startup time is reduced from 4 minutes
> to 19 seconds.  Huh?  Is it really transferring the entire file
> contents, or was the time being used for some GDB-side operation
> that is quadratic or exponential in the size of the read requested
> rather than the actual byte transfer?  Independently of the ^C
> issue, I think we need to better understand what is going on here
> and better tune the code on both sides of the RSP for large file
> transfers.

That's pretty weird.  It sounds like there's real scope for you
to improve performance here.

Thanks,
Gary
Joel Brobecker Aug. 14, 2015, 6:37 p.m. UTC | #4 
> Joel, would you like me to also push it to the 7.10 branch, or will
> you do that?

Just answering in a general way: Any Global Maintainer has authority
to approve a patch for a release branch; and once approve, you can
cherry-pick yourself and apply to the branch. Not a problem.
Pedro Alves Aug. 17, 2015, 4 p.m. UTC | #5 
On 08/12/2015 06:31 PM, Sandra Loosemore wrote:
> On 08/12/2015 08:30 AM, Gary Benson wrote:
>> Hi Sandra,
>>
>> Sandra Loosemore wrote:
>>> On 08/05/2015 09:28 AM, Gary Benson wrote:
>>>> This commit makes it possible to interrupt slow remote file transfers.
>>>>
>>>> gdb/ChangeLog:
>>>>
>>>> 	* gdb_bfd.c (gdb_bfd_iovec_fileio_pread): Add QUIT call.
>>>
>>> It still does not work for me.  :-(
>>
>> Could you please try this newer version and see if it allows you to
>> interrupt the remote transfers?
> 
> This version still doesn't make the transfer interruptable with ^C. 
> *But*, with this patch, the startup time is reduced from 4 minutes to 19 
> seconds.  Huh?  Is it really transferring the entire file contents, or 
> was the time being used for some GDB-side operation that is quadratic or 
> exponential in the size of the read requested rather than the actual 
> byte transfer?  Independently of the ^C issue, I think we need to better 
> understand what is going on here and better tune the code on both sides 
> of the RSP for large file transfers.  Even if a user asks for 
> target-side libraries explicitly, 4 minutes to transfer one library 
> doesn't provide a good user experience, and 19 seconds isn't so great 
> either when you consider that some interactive applications link with 
> dozens of GUI or multimedia libraries and not just glibc.
> 

Now that's quite surprising.  It seems to make no difference to me.

Granted, I'm testing on the local machine, but, attaching to Firefox,
with:

 $ ./gdbserver/gdbserver --multi :9999

and then:

 $ ./gdb -data-directory=data-directory -ex "tar extended-rem :9999" -ex "set pagination off" -ex "attach 31613" -ex "info shared" -ex "set confirm off" -ex "detach" -ex "quit"

takes around 3-5 seconds with or without patch.  Around one second less if I do (add "set sysroot /"):

 $ ./gdb -data-directory=data-directory -ex "set sysroot /" -ex "tar extended-rem :9999" -ex "set pagination off" -ex "attach 31613" -ex "info shared" -ex "set confirm off" -ex "detach" -ex "quit"

I was previously assuming you were seeing this on multiple machines,
but looking back, I only find mention of nios2.

Could it be the slowdown you see is caused by some other local patches
you might have in the tree you're using?  Do you see it with pristine FSF?

If you try your example test with gdb 7.9, with "set sysroot remote:",
does it also take the 4 minutes to reach main?

Also, can you reproduce this with other machines?  E.g., what about
x86_64 GNU/Linux?  Wondering whether it's a kernel/libc/etc issue...

Does anyone else confirm the unexpected slowness Sandra is seeing?

Thanks,
Pedro Alves
Sandra Loosemore Aug. 17, 2015, 6:52 p.m. UTC | #6 
On 08/17/2015 10:00 AM, Pedro Alves wrote:
>
> I was previously assuming you were seeing this on multiple machines,
> but looking back, I only find mention of nios2.
>
> Could it be the slowdown you see is caused by some other local patches
> you might have in the tree you're using?  Do you see it with pristine FSF?

I initially saw this in a local branch, but I have been very careful to 
reproduce and test things with an unmodified FSF checkout.

> If you try your example test with gdb 7.9, with "set sysroot remote:",
> does it also take the 4 minutes to reach main?
>
> Also, can you reproduce this with other machines?  E.g., what about
> x86_64 GNU/Linux?  Wondering whether it's a kernel/libc/etc issue...

The other Linux-target build I have handy for testing right now is 
arm-none-linux-gnueabi with unmodified FSF head, testing on a 
PandaBoard.  There I found it takes about 8 seconds to start up the same 
hello-world program that takes 4 minutes on Nios II with the new sysroot 
default, and < 1 second if I do "set sysroot" first.  Aside from 
possible kernel issues or whatever, the Nios II board is theoretically a 
factor of 20 slower than the PandaBoard (processor supposedly clocked at 
10Mhz, and only one core versus 1Ghz and 2 cores for the PandaBoard), 
and the libc.so it's transferring is larger.

-Sandra
diff mbox

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 69da508..7db1e25 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10378,12 +10378,11 @@  remote_hostio_pwrite (struct target_ops *self,
 				     remote_errno, NULL, NULL);
 }
 
-/* Implementation of to_fileio_pread.  */
+/* Helper for remote_hostio_pread.  */
 
 static int
-remote_hostio_pread (struct target_ops *self,
-		     int fd, gdb_byte *read_buf, int len,
-		     ULONGEST offset, int *remote_errno)
+remote_hostio_pread_1 (int fd, gdb_byte *read_buf, int len,
+		       ULONGEST offset, int *remote_errno)
 {
   struct remote_state *rs = get_remote_state ();
   char *p = rs->buf;
@@ -10417,6 +10416,37 @@  remote_hostio_pread (struct target_ops *self,
   return ret;
 }
 
+/* Implementation of to_fileio_pread.  */
+
+static int
+remote_hostio_pread (struct target_ops *self,
+		     int fd, gdb_byte *read_buf, int len,
+		     ULONGEST offset, int *remote_errno)
+{
+  gdb_byte *buf = read_buf;
+
+  while (len > 0)
+    {
+      int ret;
+
+      QUIT;
+
+      ret = remote_hostio_pread_1 (fd, buf, min (len, 4096), offset,
+				   remote_errno);
+      if (ret < 0)
+	return ret;
+
+      if (ret == 0)
+	break;
+
+      buf += ret;
+      offset += ret;
+      len -= ret;
+    }
+
+  return buf - read_buf;
+}
+
 /* Implementation of to_fileio_close.  */
 
 static int