From patchwork Thu Aug 20 14:46:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 8322 Received: (qmail 5971 invoked by alias); 20 Aug 2015 14:46:46 -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 5962 invoked by uid 89); 20 Aug 2015 14:46:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 20 Aug 2015 14:46:43 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id 5B7168C1B8; Thu, 20 Aug 2015 14:46:42 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7KEkdDY001269; Thu, 20 Aug 2015 10:46:40 -0400 Message-ID: <55D5E84E.5060303@redhat.com> Date: Thu, 20 Aug 2015 15:46:38 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Gary Benson CC: Sandra Loosemore , Joel Brobecker , Doug Evans , Jan Kratochvil , gdb-patches , =?windows-1252?Q?Andr=E9_P=F6nitz?= , Paul Koning Subject: Re: [PATCH 0/2] Better handling of slow remote transfers References: <20150811195943.GC22245@adacore.com> <20150812094831.GD11096@blade.nx> <20150814182648.GO22245@adacore.com> <55CE6AA3.8000300@codesourcery.com> <20150816184913.GA2998@adacore.com> <20150817085310.GC25320@blade.nx> <55D1EE96.9060202@codesourcery.com> <20150818095858.GB9815@blade.nx> <55D3625B.40101@codesourcery.com> <55D3DB83.4050204@redhat.com> <20150819134242.GA18586@blade.nx> In-Reply-To: <20150819134242.GA18586@blade.nx> On 08/19/2015 02:42 PM, Gary Benson wrote: > Pedro Alves wrote: >> BTW, the transfers seem to be always interruptible for me, even without >> Gary's patch, and even the slow ones. > > Ok, I'm glad I'm not the only one :) > >> From d426ce0a36830378a8ec2e2cbdd94d9fcb47b891 Mon Sep 17 00:00:00 2001 >> From: Pedro Alves >> Date: Tue, 18 Aug 2015 23:27:20 +0100 >> Subject: [PATCH 3/3] add readahead cache to gdb's vFile:pread > > I tried this out on its own, and got similar hit/miss ratios, so it > looks like a good addition. Comments below. > >> diff --git a/gdb/remote.c b/gdb/remote.c >> index a839adf..8a739c8 100644 >> --- a/gdb/remote.c >> +++ b/gdb/remote.c >> @@ -10311,6 +10311,26 @@ remote_hostio_send_command (int command_bytes, int which_packet, >> return ret; >> } >> >> +struct readahead_cache >> +{ >> + int fd; >> + gdb_byte *buf; >> + ULONGEST offset; >> + size_t bufsize; >> +}; >> + >> +static struct readahead_cache *readahead_cache; > > Would this be better in struct remote_state (and maybe not allocated, > just inlined in the main remote_state--it's only 16 or 32 bytes)? Definitely. (At first I made the cache invalidation free the cache object (and set it NULL). Then I when I saw that the CPU usage went up, I made the invalidation just clear fd to -1. The CPU did not go down. Then I lowered back the max packet size and the CPU usage went back down, but I kept the cache invalidation as it was.) > >> @@ -10325,6 +10345,8 @@ remote_hostio_set_filesystem >> char arg[9]; >> int ret; >> >> + readahead_cache_invalidate (); >> + >> if (packet_support (PACKET_vFile_setfs) == PACKET_DISABLE) >> return 0; >> > > This isn't necessary, file descriptors persist across setns calls. OK. > >> + if (remote_debug) >> + fprintf_unfiltered (gdb_stdlog, >> + "readahead cache hit %d\n", >> + ++readahead_cache_hit_count); > and >> + if (remote_debug) >> + fprintf_unfiltered (gdb_stdlog, "readahead cache miss %d\n", >> + ++readahead_cache_miss_count); > > These only update the counters when debug printing is switched on. > Is this what you intended? Hey, that was a quick and dirty patch. :-) Here's a cleaned up version. WDYT? --- From d853f786962906127da99c0ac42745447a9bbd05 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Thu, 20 Aug 2015 14:15:48 +0100 Subject: [PATCH] Add readahead cache to gdb's vFile:pread This patch almost halves the time it takes to "target remote + run to main" on a higher-latency connection. E.g., I've got a ping time of ~85ms to an x86-64 machine on the gcc compile farm (almost 2000km away from me), and I'm behind a ~16Mbit ADSL. When I connect to a gdbserver debugging itself on that machine and run to main, it takes almost 55 seconds: [palves@gcc76] $ ./gdbserver :9999 ./gdbserver [palves@home] $ ssh -L 9999:localhost:9999 gcc76.fsffrance.org [palves@home] time ./gdb -data-directory=data-directory -ex "tar rem :9999" -ex "b main" -ex "c" -ex "set confirm off" -ex "quit" Pristine gdb 7.10.50.20150820-cvs gets us: ... Remote debugging using :9999 Reading symbols from target:/home/palves/gdb/build/gdb/gdbserver/gdbserver...done. Reading symbols from target:/lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done. 0x00007ffff7ddd190 in ?? () from target:/lib64/ld-linux-x86-64.so.2 Breakpoint 1 at 0x41200c: file ../../../src/gdb/gdbserver/server.c, line 3635. Continuing. Breakpoint 1, main (argc=1, argv=0x7fffffffe3d8) at ../../../src/gdb/gdbserver/server.c:3635 3635 ../../../src/gdb/gdbserver/server.c: No such file or directory. /home/palves/gdb/build/gdb/gdbserver/gdbserver: No such file or directory. real 0m54.803s user 0m0.329s sys 0m0.064s While the readahead cache added by this patch, it drops to: real 0m29.462s user 0m0.454s sys 0m0.054s I added a few counters to show cache hit/miss, and got: readahead cache miss 142 readahead cache hit 310 Tested on x86_64 Fedora 20. gdb/ChangeLog: 2015-08-20 Pedro Alves * remote.c (struct readahead_cache): New. (struct remote_state) : New field. (remote_open_1): Invalidate the cache. (readahead_cache_invalidate, readahead_cache_invalidate_fd): New functions. (remote_hostio_pwrite): Invalidate the readahead cache. (remote_hostio_pread): Rename to ... (remote_hostio_pread_vFile): ... this. (remote_hostio_pread_from_cache): New function. (remote_hostio_pread): Reimplement. (remote_hostio_close): Invalidate the readahead cache. --- gdb/remote.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 135 insertions(+), 4 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 4e483fd..bfa5469 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -226,6 +226,8 @@ static int remote_can_run_breakpoint_commands (struct target_ops *self); static void remote_btrace_reset (void); +static void readahead_cache_invalidate (void); + /* For "remote". */ static struct cmd_list_element *remote_cmdlist; @@ -262,6 +264,29 @@ typedef unsigned char threadref[OPAQUETHREADBYTES]; #define MAXTHREADLISTRESULTS 32 +/* Data for the vFile:pread readahead cache. */ + +struct readahead_cache +{ + /* The file descriptor for the file that is being cached. -1 if the + cache is invalid. */ + int fd; + + /* The offset into the file that the cache buffer corresponds + to. */ + ULONGEST offset; + + /* The buffer holding the cache contents. */ + gdb_byte *buf; + /* The buffer's size. We try to read as much as fits into a packet + at a time. */ + size_t bufsize; + + /* Cache hit and miss counters. */ + ULONGEST hit_count; + ULONGEST miss_count; +}; + /* Description of the remote protocol state for the currently connected target. This is per-target state, and independent of the selected architecture. */ @@ -381,6 +406,14 @@ struct remote_state Initialized to -1 to indicate that no "vFile:setfs:" packet has yet been sent. */ int fs_pid; + + /* A readahead cache for vFile:pread. Often, reading a binary + involves a sequence of small reads. E.g., when parsing an ELF + file. A readahead cache helps mostly the case of remote + debugging on a connection with higher latency, due to the + request/reply nature of the RSP. We only cache data for a single + file descriptor at a time. */ + struct readahead_cache readahead_cache; }; /* Private data that we'll store in (struct thread_info)->private. */ @@ -4498,6 +4531,8 @@ remote_open_1 (const char *name, int from_tty, rs->use_threadinfo_query = 1; rs->use_threadextra_query = 1; + readahead_cache_invalidate (); + if (target_async_permitted) { /* With this target we start out by owning the terminal. */ @@ -10329,6 +10364,27 @@ remote_hostio_send_command (int command_bytes, int which_packet, return ret; } +/* Invalidate the readahead cache. */ + +static void +readahead_cache_invalidate (void) +{ + struct remote_state *rs = get_remote_state (); + + rs->readahead_cache.fd = -1; +} + +/* Invalidate the readahead cache if it is holding data for FD. */ + +static void +readahead_cache_invalidate_fd (int fd) +{ + struct remote_state *rs = get_remote_state (); + + if (rs->readahead_cache.fd == fd) + rs->readahead_cache.fd = -1; +} + /* Set the filesystem remote_hostio functions that take FILENAME arguments will use. Return 0 on success, or -1 if an error occurs (and set *REMOTE_ERRNO). */ @@ -10407,6 +10463,8 @@ remote_hostio_pwrite (struct target_ops *self, int left = get_remote_packet_size (); int out_len; + readahead_cache_invalidate_fd (fd); + remote_buffer_add_string (&p, &left, "vFile:pwrite:"); remote_buffer_add_int (&p, &left, fd); @@ -10422,12 +10480,13 @@ remote_hostio_pwrite (struct target_ops *self, remote_errno, NULL, NULL); } -/* Implementation of to_fileio_pread. */ +/* Helper for the implementation of to_fileio_pread. Read the file + from the remote side with vFile: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_vFile (struct target_ops *self, + int fd, gdb_byte *read_buf, int len, + ULONGEST offset, int *remote_errno) { struct remote_state *rs = get_remote_state (); char *p = rs->buf; @@ -10461,6 +10520,76 @@ remote_hostio_pread (struct target_ops *self, return ret; } +/* Serve pread from the readahead cache. Returns number of bytes + read, or 0 if the request can't be served from the cache. */ + +static int +remote_hostio_pread_from_cache (struct remote_state *rs, + int fd, gdb_byte *read_buf, size_t len, + ULONGEST offset) +{ + struct readahead_cache *cache = &rs->readahead_cache; + + if (cache->fd == fd + && cache->offset <= offset + && offset < cache->offset + cache->bufsize) + { + ULONGEST max = cache->offset + cache->bufsize; + + if (offset + len > max) + len = max - offset; + + memcpy (read_buf, cache->buf + offset - cache->offset, len); + return len; + } + + return 0; +} + +/* 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) +{ + int ret; + struct remote_state *rs = get_remote_state (); + struct readahead_cache *cache = &rs->readahead_cache; + + ret = remote_hostio_pread_from_cache (rs, fd, read_buf, len, offset); + if (ret > 0) + { + cache->hit_count++; + + if (remote_debug) + fprintf_unfiltered (gdb_stdlog, "readahead cache hit %s\n", + pulongest (cache->hit_count)); + return ret; + } + + cache->miss_count++; + if (remote_debug) + fprintf_unfiltered (gdb_stdlog, "readahead cache miss %s\n", + pulongest (cache->miss_count)); + + cache->fd = fd; + cache->offset = offset; + cache->bufsize = get_remote_packet_size (); + cache->buf = xrealloc (cache->buf, cache->bufsize); + + ret = remote_hostio_pread_vFile (self, cache->fd, cache->buf, cache->bufsize, + cache->offset, remote_errno); + if (ret <= 0) + { + readahead_cache_invalidate_fd (fd); + return ret; + } + + cache->bufsize = ret; + return remote_hostio_pread_from_cache (rs, fd, read_buf, len, offset); +} + /* Implementation of to_fileio_close. */ static int @@ -10470,6 +10599,8 @@ remote_hostio_close (struct target_ops *self, int fd, int *remote_errno) char *p = rs->buf; int left = get_remote_packet_size () - 1; + readahead_cache_invalidate_fd (fd); + remote_buffer_add_string (&p, &left, "vFile:close:"); remote_buffer_add_int (&p, &left, fd);