From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26131 invoked by alias); 22 Nov 2002 18:57:07 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 26123 invoked from network); 22 Nov 2002 18:57:06 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 22 Nov 2002 18:57:06 -0000 Received: from int-mx2.corp.redhat.com (nat-pool-rdu-dmz.redhat.com [172.16.52.200]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id gAMIX1P29405 for ; Fri, 22 Nov 2002 13:33:01 -0500 Received: from potter.sfbay.redhat.com (potter.sfbay.redhat.com [172.16.27.15]) by int-mx2.corp.redhat.com (8.11.6/8.11.6) with ESMTP id gAMIuxs12158; Fri, 22 Nov 2002 13:56:59 -0500 Received: from redhat.com (reddwarf.sfbay.redhat.com [172.16.24.50]) by potter.sfbay.redhat.com (8.11.6/8.11.6) with ESMTP id gAMIuwM24383; Fri, 22 Nov 2002 10:56:58 -0800 Message-ID: <3DDE7DFA.1F4F77F2@redhat.com> Date: Fri, 22 Nov 2002 10:57:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Accept-Language: en MIME-Version: 1.0 To: Daniel Jacobowitz CC: gdb-patches@sources.redhat.com Subject: Re: PATCH/RFC: Bring lin-lwp performance back to the real world References: <20021122041123.GA21389@nevyn.them.org> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-11/txt/msg00550.txt.bz2 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 > > * 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 > +#include > + > +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.