PATCH/RFC: Bring lin-lwp performance back to the real world

Michael Snyder msnyder@redhat.com
Fri Nov 22 10:57:00 GMT 2002


Appologies for top posting -- long message.
Daniel, this is excellent.  Can think of several places to go with it, 
but as you say it's a good start.  

I just have one suggestion.  There is already a "linux-proc.c" module.
How about putting linux_proc_xfer_memory in there?  Keep all uses of
this silly pseudo-proc system in one place.

Michael

Daniel Jacobowitz wrote:
> 
> First, let me quote some numbers from an earlier message on gdb@:
> 
> currently:
> runtest linux-dp.exp print-threads.exp  17.21s user 48.22s system 82% cpu 1:19.56 total
> With a patch to reduce redundant register fetches (a little):
> runtest linux-dp.exp print-threads.exp  16.67s user 45.35s system 82% cpu 1:15.27 total
> 
> Well, I have a new patch:
> runtest linux-dp.exp print-threads.exp  2.37s user 3.84s system 32% cpu 19.325 total
> 
> Note: cuts the real-world time by a factor of 4, cuts system (ptrace!) time
> by a factor of almost _twelve_.  I ran some larger tests and these ratios
> held.
> 
> I'm _almost_ reluctant to submit this patch:
>  - It's stupid.  Look at the open/close and you'll see what I mean.  This
>    patch took me literally about fifteen minutes.  Most of the rest of the
>    time was waiting for unpatched GDBs to finish testing.
>  - It's such a wonderful bandaid that a lot of the badly needed
>    cleanups may lose momentum.
> But we'll just have to endure those for now; I think it's worthwhile.  It's
> a quick, safe fix for a substantial and often-reported performance problem.
> 
> What it does: large (> 3 words) reads are done by opening /proc/pid/mem and
> reading from it instead of by ptrace.  The number was chosen to keep the
> number of syscalls roughly equivalent for short reads.  Failure is handled
> gracefully; this works at least back to kernel 2.2.x though.
> 
> What it doesn't do:
>  - implement it for non-threaded applications; this is an easy followup
>    but I'd rather do it separately, since it involves renaming
>    child_xfer_memory and adding a wrapper to it, so that I can call it.
>    No point in overriding the whole thing.
>  - Let us debug 64-bit apps from a 32-bit native GDB.  I'm pretty sure this
>    is broken right now anyway.  It can be fixed by adding autoconf magic
>    to define -D_LARGEFILE64_SOURCE and use lseek64, if said function
>    is available.  Also a good candidate to do separately.
>  - Speed up that damned huge.exp, which times out on a lot of my boards.
>    We don't do read-combining when reading from an array, but we should.
>    Another candidate for separately.
>  - Cache the open file descriptor.  I don't do this because I don't want to
>    track which LWP I'm reading from, esp. in the ideal future when not all
>    LWPs are necessarily stopped.  Also the fork code needs to write to an
>    arbitrary attached process.  It's of less importance now that we cut down
>    on the number of reads; but it should still be done if the hooks are
>    available to do it cleanly (i.e. closing the FD when we
>    detach/exit/etc.).  I'm postponing this also because it's minor and it
>    would conflict with some other outstanding patches.
> 
> This patch also has another very important consequence.  Right now, Debian's
> Apache package links to -lpthread, even though it only creates one thread.
> It loads lots of shared modules.  This makes it _slow_.  Apache2 will be
> even worse.  Right now starting apache -X under GDB, no debugging symbols or
> anything, running it until it realizes it can't open its error log or bind
> its port takes:
> Command execution time: 28.960000
> With this patch:
> Command execution time: 2.330000
> i.e. usably fast!
> 
> (0.363 seconds to do it without a debugger; six times slower for app startup
> is acceptable in my opinion.  Lots of shlib events here!)
> 
> Now that all that is said and done, I'll cut to the chase.  Here's the
> patch.  I'm looking to apply it to both trunk and 5.3 branch - I think it's
> safe enough; the kernel uses the exact same mechanism it uses for ptrace
> PEEKTEXT anyway - but I definitely want feedback on this.  What do y'all
> (ack!  I said y'all!  I'm sorry!) think of this?
> 
> --
> Daniel Jacobowitz
> MontaVista Software                         Debian GNU/Linux Developer
> 
> 2002-11-21  Daniel Jacobowitz  <drow@mvista.com>
> 
>         * Makefile.in (linux-nat.o): Add dependencies.
>         * lin-lwp.c (lin_lwp_xfer_memory): Call linux_proc_xfer_memory.
>         * linux-nat.c: New file.
>         * config/nm-linux.h (linux_proc_xfer_memory): Add prototype.
> 
>         * config/alpha/alpha-linux.mh (NATDEPFILES): Add linux-nat.o.
>         * config/arm/linux.mh (NATDEPFILES): Likewise.
>         * config/i386/linux.mh (NATDEPFILES): Likewise.
>         * config/i386/x86-64linux.mh (NATDEPFILES): Likewise.
>         * config/ia64/linux.mh (NATDEPFILES): Likewise.
>         * config/m68k/linux.mh (NATDEPFILES): Likewise.
>         * config/mips/linux.mh (NATDEPFILES): Likewise.
>         * config/powerpc/linux.mh (NATDEPFILES): Likewise.
>         * config/s390/linux.mh (NATDEPFILES): Likewise.
>         * config/sh/linux.mh (NATDEPFILES): Likewise.
> 
> Index: Makefile.in
> ===================================================================
> RCS file: /cvs/src/src/gdb/Makefile.in,v
> retrieving revision 1.283
> diff -u -p -r1.283 Makefile.in
> --- Makefile.in 19 Nov 2002 23:14:45 -0000      1.283
> +++ Makefile.in 22 Nov 2002 04:00:39 -0000
> @@ -1862,6 +1862,7 @@ lin-lwp.o: lin-lwp.c $(defs_h) $(gdb_ass
>  linespec.o: linespec.c $(defs_h) $(symtab_h) $(frame_h) $(command_h) \
>         $(symfile_h) $(objfiles_h) $(demangle_h) $(value_h) $(completer_h) \
>         $(cp_abi_h) $(source_h) $(parser_defs_h)
> +linux-nat.o: linux-nat.c $(defs_h) $(inferior_h)
>  linux-proc.o: linux-proc.c $(defs_h) $(inferior_h) $(regcache_h) \
>         $(gregset_h) $(gdbcore_h) $(gdbthread_h) $(elf_bfd_h) \
>         $(cli_decode_h) $(gdb_string_h)
> Index: lin-lwp.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/lin-lwp.c,v
> retrieving revision 1.36
> diff -u -p -r1.36 lin-lwp.c
> --- lin-lwp.c   31 Oct 2002 21:00:08 -0000      1.36
> +++ lin-lwp.c   22 Nov 2002 04:00:39 -0000
> @@ -1380,7 +1380,9 @@ lin_lwp_xfer_memory (CORE_ADDR memaddr,
>    if (is_lwp (inferior_ptid))
>      inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid));
> 
> -  xfer = child_xfer_memory (memaddr, myaddr, len, write, attrib, target);
> +  xfer = linux_proc_xfer_memory (memaddr, myaddr, len, write, attrib, target);
> +  if (xfer == 0)
> +    xfer = child_xfer_memory (memaddr, myaddr, len, write, attrib, target);
> 
>    do_cleanups (old_chain);
>    return xfer;
> Index: linux-nat.c
> ===================================================================
> RCS file: linux-nat.c
> diff -N linux-nat.c
> --- /dev/null   1 Jan 1970 00:00:00 -0000
> +++ linux-nat.c 22 Nov 2002 04:00:39 -0000
> @@ -0,0 +1,58 @@
> +/* GNU/Linux native-dependent code common to multiple platforms.
> +   Copyright (C) 2002 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 2 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software
> +   Foundation, Inc., 59 Temple Place - Suite 330,
> +   Boston, MA 02111-1307, USA.  */
> +
> +#include "defs.h"
> +#include "inferior.h"
> +
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +int linux_proc_xfer_memory (CORE_ADDR addr, char *myaddr, int len, int write,
> +                           struct mem_attrib *attrib,
> +                           struct target_ops *target)
> +{
> +  int fd, ret;
> +  char filename[64];
> +
> +  if (write)
> +    return 0;
> +
> +  /* Don't bother for one word.  */
> +  if (len < 4 * sizeof (long))
> +    return 0;
> +
> +  sprintf (filename, "/proc/%d/mem", PIDGET (inferior_ptid));
> +  fd = open (filename, O_RDONLY);
> +  if (fd == -1)
> +    return 0;
> +
> +  /* FIXME 64-bit vs 32-bit, i.e. Sparc debugging Sparc64?
> +     Autoconf for _LARGEFILE64_SOURCE?  */
> +  if (lseek (fd, addr, SEEK_SET) == -1
> +      || read (fd, myaddr, len) != len)
> +    ret = 0;
> +  else
> +    ret = len;
> +
> +  close (fd);
> +  return ret;
> +}
> +
> +
> Index: config/nm-linux.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/nm-linux.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 nm-linux.h
> --- config/nm-linux.h   24 Feb 2002 22:56:04 -0000      1.12
> +++ config/nm-linux.h   22 Nov 2002 04:00:39 -0000
> @@ -71,4 +71,8 @@ extern void lin_thread_get_thread_signal
>  /* Override child_pid_to_exec_file in 'inftarg.c'.  */
>  #define CHILD_PID_TO_EXEC_FILE
> 
> +struct mem_attrib;
> +extern int linux_proc_xfer_memory (CORE_ADDR addr, char *myaddr, int len,
> +                                  int write, struct mem_attrib *attrib,
> +                                  struct target_ops *target);
> 
> Index: config/alpha/alpha-linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/alpha/alpha-linux.mh,v
> retrieving revision 1.9
> diff -u -p -r1.9 alpha-linux.mh
> --- config/alpha/alpha-linux.mh 18 Jan 2002 04:50:57 -0000      1.9
> +++ config/alpha/alpha-linux.mh 22 Nov 2002 04:00:39 -0000
> @@ -2,7 +2,8 @@
>  XM_FILE= xm-alphalinux.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o corelow.o alpha-nat.o linux-proc.o \
> -       fork-child.o proc-service.o thread-db.o lin-lwp.o gcore.o
> +       fork-child.o proc-service.o thread-db.o lin-lwp.o gcore.o \
> +       linux-nat.o
> 
>  LOADLIBES = -ldl -rdynamic
> 
> Index: config/arm/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/arm/linux.mh,v
> retrieving revision 1.10
> diff -u -p -r1.10 linux.mh
> --- config/arm/linux.mh 18 Jan 2002 04:50:58 -0000      1.10
> +++ config/arm/linux.mh 22 Nov 2002 04:00:39 -0000
> @@ -5,7 +5,7 @@ XM_FILE= xm-linux.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o      \
>         core-regset.o arm-linux-nat.o linux-proc.o gcore.o      \
> -       proc-service.o thread-db.o lin-lwp.o
> +       proc-service.o thread-db.o lin-lwp.o linux-nat.o
> 
>  LOADLIBES= -ldl -rdynamic
> 
> Index: config/i386/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/i386/linux.mh,v
> retrieving revision 1.12
> diff -u -p -r1.12 linux.mh
> --- config/i386/linux.mh        11 May 2002 17:22:27 -0000      1.12
> +++ config/i386/linux.mh        22 Nov 2002 04:00:39 -0000
> @@ -5,7 +5,8 @@ XM_FILE= xm-i386.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o linux-proc.o \
>         core-aout.o i386-nat.o i386-linux-nat.o \
> -       proc-service.o thread-db.o lin-lwp.o linux-proc.o gcore.o
> +       proc-service.o thread-db.o lin-lwp.o linux-proc.o gcore.o \
> +       linux-nat.o
> 
>  # The dynamically loaded libthread_db needs access to symbols in the
>  # gdb executable.
> Index: config/i386/x86-64linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/i386/x86-64linux.mh,v
> retrieving revision 1.7
> diff -u -p -r1.7 x86-64linux.mh
> --- config/i386/x86-64linux.mh  1 Jul 2002 22:09:52 -0000       1.7
> +++ config/i386/x86-64linux.mh  22 Nov 2002 04:00:39 -0000
> @@ -6,6 +6,6 @@ NAT_FILE= nm-x86-64linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
>         core-aout.o i386-nat.o x86-64-linux-nat.o \
>         proc-service.o thread-db.o lin-lwp.o \
> -       linux-proc.o gcore.o
> +       linux-proc.o gcore.o linux-nat.o
> 
>  LOADLIBES = -ldl -rdynamic
> Index: config/ia64/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/ia64/linux.mh,v
> retrieving revision 1.13
> diff -u -p -r1.13 linux.mh
> --- config/ia64/linux.mh        4 Feb 2002 19:11:17 -0000       1.13
> +++ config/ia64/linux.mh        22 Nov 2002 04:00:39 -0000
> @@ -5,6 +5,6 @@ XM_FILE= xm-linux.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o gcore.o \
>         core-aout.o core-regset.o ia64-linux-nat.o linux-proc.o \
> -       proc-service.o thread-db.o lin-lwp.o
> +       proc-service.o thread-db.o lin-lwp.o linux-nat.o
> 
>  LOADLIBES = -ldl -rdynamic
> Index: config/m68k/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/m68k/linux.mh,v
> retrieving revision 1.10
> diff -u -p -r1.10 linux.mh
> --- config/m68k/linux.mh        14 Feb 2002 05:48:35 -0000      1.10
> +++ config/m68k/linux.mh        22 Nov 2002 04:00:39 -0000
> @@ -5,7 +5,7 @@ XM_FILE= xm-linux.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o \
>         corelow.o core-aout.o m68klinux-nat.o linux-proc.o gcore.o \
> -       proc-service.o thread-db.o lin-lwp.o
> +       proc-service.o thread-db.o lin-lwp.o linux-nat.o
> 
>  # The dynamically loaded libthread_db needs access to symbols in the
>  # gdb executable.
> Index: config/mips/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/mips/linux.mh,v
> retrieving revision 1.4
> diff -u -p -r1.4 linux.mh
> --- config/mips/linux.mh        18 Jan 2002 04:51:03 -0000      1.4
> +++ config/mips/linux.mh        22 Nov 2002 04:00:39 -0000
> @@ -2,6 +2,7 @@
>  XM_FILE= xm-linux.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o mips-linux-nat.o \
> -       thread-db.o lin-lwp.o proc-service.o linux-proc.o gcore.o
> +       thread-db.o lin-lwp.o proc-service.o linux-proc.o gcore.o \
> +       linux-nat.o
> 
>  LOADLIBES = -ldl -rdynamic
> Index: config/powerpc/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/powerpc/linux.mh,v
> retrieving revision 1.12
> diff -u -p -r1.12 linux.mh
> --- config/powerpc/linux.mh     30 Jul 2002 19:03:49 -0000      1.12
> +++ config/powerpc/linux.mh     22 Nov 2002 04:00:40 -0000
> @@ -6,7 +6,7 @@ XM_CLIBS=
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o linux-proc.o \
>         ppc-linux-nat.o proc-service.o thread-db.o lin-lwp.o \
> -       gcore.o
> +       gcore.o linux-nat.o
> 
>  LOADLIBES = -ldl -rdynamic
> 
> Index: config/s390/s390.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/s390/s390.mh,v
> retrieving revision 1.6
> diff -u -p -r1.6 s390.mh
> --- config/s390/s390.mh 28 Apr 2002 00:30:01 -0000      1.6
> +++ config/s390/s390.mh 22 Nov 2002 04:00:40 -0000
> @@ -6,7 +6,7 @@ XM_CLIBS=
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o s390-nat.o \
>         core-aout.o core-regset.o linux-proc.o gcore.o thread-db.o lin-lwp.o \
> -       proc-service.o
> +       proc-service.o linux-nat.o
>  LOADLIBES = -ldl -rdynamic
> 
> 
> Index: config/sparc/linux.mh
> ===================================================================
> RCS file: /cvs/src/src/gdb/config/sparc/linux.mh,v
> retrieving revision 1.9
> diff -u -p -r1.9 linux.mh
> --- config/sparc/linux.mh       4 May 2002 15:52:42 -0000       1.9
> +++ config/sparc/linux.mh       22 Nov 2002 04:00:40 -0000
> @@ -5,7 +5,7 @@ XM_FILE= xm-linux.h
>  NAT_FILE= nm-linux.h
>  NATDEPFILES= fork-child.o infptrace.o inftarg.o corelow.o sparc-nat.o \
>         proc-service.o thread-db.o lin-lwp.o sparc-linux-nat.o \
> -       linux-proc.o gcore.o
> +       linux-proc.o gcore.o linux-nat.o
> 
>  # The dynamically loaded libthread_db needs access to symbols in the
>  # gdb executable.



More information about the Gdb-patches mailing list