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

Michael Snyder
2002-11-22T18:57:07Z

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.