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

Daniel Jacobowitz
2002-11-22T21:49:57Z

On Fri, Nov 22, 2002 at 03:34:13PM -0500, Daniel Jacobowitz wrote:
> OK.
>
> I'm going to sit on this for a couple of hours while I pound on it; I
> was testing the patch in another environment and saw very strange
> performance numbers.

More precisely, I ran this test in two environments under strace:
gdb linux-dp
break print_philosopher
run
kill
quit

- My desktop runs glibc 2.3.1, stripped. Stracing a gdb without the
pread64 patch doing this shows 1698755 PEEKTEXT's.

- My test board runs glibc 2.2.5, unstripped. Stracing a gdb without
the pread64 patch doing this shows 299861 PEEKTEXT's.

No wonder the timing results were different!

Turns out there were two problems with my numbers. One of them is the
glibc versions. We read in the entire PTHREAD_THREADS_MAX pthreads
handles in libthread_db.so:

1999-11-02 Ulrich Drepper <drepper@cygnus.com>

* td_ta_thr_iter.c (td_ta_thr_iter): Optimize a bit. Read all
handles at once.

That's gone from 1024 * 16 to 16384 * 16, courtesy of the new threads
support which supports more than 1K threads. So the slow has changed
to glacial. I'd ask them to reconsider that change but now that we use
pread64 instead of ptrace/ptrace/ptrace/ptrace/... it doesn't really
matter if we read 16K or 256K; it's not a substantial amount of our
time. It does serve to make my patch more dramatic though.

The other was just a typo on my part - I was calling child_xfer_memory
even if linux_proc_xfer_memory succeeded. It wasn't in the patch I
posted, I added it later. Clever me.

Once fixed, I am again happy with it. This version uses pread64, which
requires adding -D_LARGEFILE64_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=500
on glibc... I'll commit it to the trunk and 5.3 branch tomorrow unless
someone sees a problem, and we'll be much faster. Should be safe
everywhere. Only enables pread64 if libc has pread64, and libc will
emulate it if the kernel doesn't really support it.

--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer

2002-11-22 Daniel Jacobowitz <drow@mvista.com>

* acconfig.h (HAVE_PREAD64): Add.
* configure.in: Check for pread64.
* config.in: Regenerated.
* configure: Regenerated.
* lin-lwp.c (lin_lwp_xfer_memory): Call linux_proc_xfer_memory.
* linux-proc.c (linux_proc_xfer_memory): New function.
* config/nm-linux.h (linux_proc_xfer_memory): Add prototype.

Index: acconfig.h
===================================================================
RCS file: /cvs/src/src/gdb/acconfig.h,v
retrieving revision 1.18
diff -u -p -r1.18 acconfig.h
--- acconfig.h 11 Apr 2002 18:32:51 -0000 1.18
+++ acconfig.h 22 Nov 2002 21:41:51 -0000
@@ -49,6 +49,9 @@
/* Define if <sys/procfs.h> has pr_siginfo64_t */
#undef HAVE_PR_SIGINFO64_T

+/* Define if the pread64 function is available. */
+#undef HAVE_PREAD64
+
/* Define if <link.h> exists and defines struct link_map which has
members with an ``l_'' prefix. (For Solaris, SVR4, and
SVR4-like systems.) */
Index: configure.in
===================================================================
RCS file: /cvs/src/src/gdb/configure.in,v
retrieving revision 1.93
diff -u -p -r1.93 configure.in
--- configure.in 12 Nov 2002 02:39:41 -0000 1.93
+++ configure.in 22 Nov 2002 21:41:52 -0000
@@ -617,6 +617,23 @@ if test "x$gdb_cv_thread_db_h_has_td_not
[Define if <thread_db.h> has the TD_NOTALLOC error code.])
fi

+dnl linux-proc.c wants to use pread64, which may require special CFLAGS
+dnl -D_BSD_SOURCE is normally assumed but we have to specify it because of
+dnl -D_XOPEN_SOURCE=500.
+if test $host = $target; then
+ case $target in
+ *-linux*)
+ save_CFLAGS=$CFLAGS
+ CFLAGS="$CFLAGS -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_LARGEFILE64_SOURCE"
+ AC_TRY_LINK([#include <unistd.h>],
+ [pread64 (0, NULL, 0, 0);],
+ [ENABLE_CFLAGS="$ENABLE_CFLAGS -D_BSD_SOURCE -D_XOPEN_SOURCE=500 -D_LARGEFILE64_SOURCE"
+ AC_DEFINE(HAVE_PREAD64)], [])
+ CFLAGS=$save_CFLAGS
+ ;;
+ esac
+fi
+
dnl The CLI cannot be disabled yet, but may be in the future

dnl Handle CLI sub-directory configury.
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 21:41:53 -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-proc.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-proc.c,v
retrieving revision 1.11
diff -u -p -r1.11 linux-proc.c
--- linux-proc.c 23 Aug 2002 19:06:05 -0000 1.11
+++ linux-proc.c 22 Nov 2002 21:41:53 -0000
@@ -25,6 +25,8 @@
#include <sys/procfs.h> /* for elf_gregset etc. */
#include <sys/stat.h> /* for struct stat */
#include <ctype.h> /* for isdigit */
+#include <unistd.h> /* for open, pread64 */
+#include <fcntl.h> /* for O_RDONLY */
#include "regcache.h" /* for registers_changed */
#include "gregset.h" /* for gregset */
#include "gdbcore.h" /* for get_exec_file */
@@ -33,6 +35,10 @@
#include "cli/cli-decode.h" /* for add_info */
#include "gdb_string.h"

+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+
/* Function: child_pid_to_exec_file
*
* Accepts an integer pid
@@ -576,4 +582,37 @@ Specify any of the following keywords fo
stat -- list a bunch of random process info.\n\
status -- list a different bunch of random process info.\n\
all -- list all available /proc info.");
+}
+
+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 < 3 * sizeof (long))
+ return 0;
+
+ sprintf (filename, "/proc/%d/mem", PIDGET (inferior_ptid));
+ fd = open (filename, O_RDONLY | O_LARGEFILE);
+ if (fd == -1)
+ return 0;
+
+#ifdef HAVE_PREAD64
+ if (pread64 (fd, myaddr, len, addr) != len)
+#else
+ if (lseek (fd, addr, SEEK_SET) == -1
+ || read (fd, myaddr, len) != len)
+#endif
+ 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 21:41:53 -0000
@@ -71,4 +71,7 @@ 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);