Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Gary Benson <gbenson@redhat.com>
To: gdb-patches@sourceware.org
Subject: [RFA] Improve performance with lots of shared libraries
Date: Fri, 01 Jul 2011 16:51:00 -0000	[thread overview]
Message-ID: <20110701165109.GA3399@redhat.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

Hi all,

While working on a new linker-debugger interface I took some time out
to do a bit of profiling to see exactly where gdb is spending its time
with inferiors that load a lot of shared libraries, and it turned out
that the top 30% of the profile was update_section_map and the things
it calls.

update_section_map is called in exactly one place, by find_pc_section,
which calls update_section_map if the list of loaded object files has
changed.

There are two calls in handle_inferior_event that (indirectly) call
find_pc_section: find_pc_partial_function, and skip_inline_frames.
The first of these to be called will end up calling update_section_map
every time the solib event breakpoint is hit, because the list of
loaded objects has been changed by the last time the breakpoint was
hit.

I walked through handle_inferior_event and it turns out that when
stop_on_solib_events is unset both the call to
find_pc_partial_function and the call to skip_inline_frames can be
omitted, the first because its result is never used, and the second
because the solib event breakpoint is defined to be the address of
a function--ie not inlined.

This patch alters handle_inferior_event to check whether a stop is due
to the solib event breakpoint, and omit the two calls if it is.  I'm
not 100% convinced there aren't odd corner cases I've missed (the
surrounding code is pretty dense!) but it passes the tests, and with a
small benchmark I put together (a little program that loads 1000
shared libraries) gdb ran the application in 36s with the patch
against 47s without, a 23% improvement.

I'd really appreciate feedback from people who know this part of gdb
well, as well as feedback from those users who are using gdb on
many-solibs applications as to whether this patch helps.

Cheers,
Gary

-- 
http://gbenson.net/

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4281 bytes --]

2011-07-01  Gary Benson  <gbenson@redhat.com>

	* infrun.c (solib_event_breakpoint_helper_arg): New structure.
	(at_solib_event_breakpoint_helper): New function.
	(at_solib_event_breakpoint): Likewise.
	(handle_inferior_event): Avoid calling find_pc_partial_function
	and skip_inline_frames when stopping at the solib event breakpoint.

diff --git a/gdb/infrun.c b/gdb/infrun.c
index a656cbf..6e7a062 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3080,6 +3080,56 @@ handle_syscall_event (struct execution_control_state *ecs)
   return 1;
 }
 
+/* Argument for at_solib_event_breakpoint_helper.  */
+
+struct solib_event_breakpoint_helper_arg
+{
+  CORE_ADDR prev_pc;
+  int shlib_bp_count;
+  int other_bp_count;
+};
+
+/* Helper for at_solib_event_breakpoint.  */
+
+static int
+at_solib_event_breakpoint_helper (struct breakpoint *b, void *argp)
+{
+  struct solib_event_breakpoint_helper_arg *arg
+    = (struct solib_event_breakpoint_helper_arg *) argp; 
+  struct bp_location *loc;
+
+  for (loc = b->loc; loc; loc = loc->next)
+    {
+      if (loc->pspace == current_program_space
+	  && (loc->address == stop_pc || loc->address == arg->prev_pc))
+	{
+	  if (b->type == bp_shlib_event)
+	    arg->shlib_bp_count++;
+	  else
+	    {
+	      arg->other_bp_count++;
+	      return 1; /* quick exit */
+	    }
+	}
+    }
+
+  return 0; /* carry on looking */
+}
+
+/* Nonzero if the location stopoed at is the shlib event breakpoint.  */
+
+static int
+at_solib_event_breakpoint (struct execution_control_state *ecs)
+{
+  struct solib_event_breakpoint_helper_arg arg;
+  arg.prev_pc = ecs->event_thread->prev_pc;
+  arg.shlib_bp_count = arg.other_bp_count = 0;
+
+  iterate_over_breakpoints (at_solib_event_breakpoint_helper, &arg);
+
+  return arg.shlib_bp_count && !arg.other_bp_count;
+}
+
 /* Given an execution control state that has been freshly filled in
    by an event from the inferior, figure out what it means and take
    appropriate action.  */
@@ -3928,12 +3978,6 @@ handle_inferior_event (struct execution_control_state *ecs)
   ecs->stop_func_start = 0;
   ecs->stop_func_end = 0;
   ecs->stop_func_name = 0;
-  /* Don't care about return value; stop_func_start and stop_func_name
-     will both be 0 if it doesn't work.  */
-  find_pc_partial_function (stop_pc, &ecs->stop_func_name,
-			    &ecs->stop_func_start, &ecs->stop_func_end);
-  ecs->stop_func_start
-    += gdbarch_deprecated_function_start_offset (gdbarch);
   ecs->event_thread->stepping_over_breakpoint = 0;
   bpstat_clear (&ecs->event_thread->control.stop_bpstat);
   ecs->event_thread->control.stop_step = 0;
@@ -3941,11 +3985,30 @@ handle_inferior_event (struct execution_control_state *ecs)
   ecs->random_signal = 0;
   stopped_by_random_signal = 0;
 
-  /* Hide inlined functions starting here, unless we just performed stepi or
-     nexti.  After stepi and nexti, always show the innermost frame (not any
-     inline function call sites).  */
-  if (ecs->event_thread->control.step_range_end != 1)
-    skip_inline_frames (ecs->ptid);
+  /* If we have stopped at the solib event breakpoint and
+     stop_on_solib_events is not set then we can avoid calling
+     anything that calls find_pc_section.  This saves a lot
+     of time when the inferior loads a lot of shared libraries,
+     because otherwise the section map gets regenerated every
+     time we stop.  */
+  if (stop_on_solib_events
+      || ecs->event_thread->suspend.stop_signal != TARGET_SIGNAL_TRAP
+      || stop_after_trap
+      || !at_solib_event_breakpoint (ecs))
+    {
+      /* Don't care about return value; stop_func_start and stop_func_name
+	 will both be 0 if it doesn't work.  */
+      find_pc_partial_function (stop_pc, &ecs->stop_func_name,
+				&ecs->stop_func_start, &ecs->stop_func_end);
+      ecs->stop_func_start
+	+= gdbarch_deprecated_function_start_offset (gdbarch);
+
+      /* Hide inlined functions starting here, unless we just
+	 performed stepi or nexti.  After stepi and nexti, always show
+	 the innermost frame (not any inline function call sites).  */
+      if (ecs->event_thread->control.step_range_end != 1)
+	skip_inline_frames (ecs->ptid);
+    }
 
   if (ecs->event_thread->suspend.stop_signal == TARGET_SIGNAL_TRAP
       && ecs->event_thread->control.trap_expected

             reply	other threads:[~2011-07-01 16:51 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-01 16:51 Gary Benson [this message]
2011-07-01 17:19 ` Tom Tromey
2011-07-04 14:10   ` Gary Benson
2011-07-01 17:32 ` Joel Brobecker
2011-07-04 14:21   ` Gary Benson
2011-07-01 17:45 ` Pedro Alves
2011-07-01 17:57   ` Pedro Alves
2011-09-09 14:25 Gary Benson
2011-09-09 14:35 ` Daniel Jacobowitz
2011-09-09 14:51   ` Jan Kratochvil
2011-09-09 15:04     ` Pedro Alves
2011-09-09 19:41       ` Jan Kratochvil
2011-09-12 12:44         ` Pedro Alves
2011-09-12 16:44           ` Jan Kratochvil
2011-09-14  9:28             ` Gary Benson
2011-10-04 19:46               ` Tom Tromey
2011-09-09 15:11     ` Pedro Alves
2011-09-09 14:53 ` Jan Kratochvil
2011-10-04 20:03   ` Tom Tromey
2011-09-22 17:35 Gary Benson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110701165109.GA3399@redhat.com \
    --to=gbenson@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox