[RFA take 2] Improve performance with lots of shared libraries

[Gary Benson <gbenson@redhat.com>]

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

Hi Pedro,

Pedro Alves wrote:
> On Tuesday 04 October 2011 11:25:18, Gary Benson wrote:
> >   2011-09-22  Gary Benson  <gbenson@redhat.com>
> > 
> >         * infrun.c (stopped_at_solib_event_breakpoint): New function.
> >         (stopped_at_solib_event_breakpoint_helper): Likewise.
> >         (handle_inferior_event): Avoid calling skip_inline_frames
> >         when at the solib event breakpoint.
> 
> As I mentioned before, better would for this to be a
> property/function of the breakpoint itself (and its locations) --
> meaning "I am / am not an inlined location".  There's no reason to
> limit this just to the solib event breakpoint.  An obvious example
> without even that generalization is the thread event breakpoint.
> 
> Even if you don't do that, please add this new function (that checks
> whether we stopped at an address that can't be inlined) to
> breakpoint.c instead, and call it `stopped_at_non_inline_function'
> or something along those lines.  We can later rework its internals
> keeping its interface.
...
> You need to check that the breakpoint is enabled and installed.
...
> When working in the direction of matching from an event back to our
> tables and symbolic info, you start from the address_space instead.
> Get at the address space of the current PC using
> `get_regcache_aspace (get_thread_regcache (ecs->ptid))'.  But better
> yet, since you'd end up rewritting bkpt_breakpoint_hit, it's better
> instead to reuse bpstat_check_location (despite the name, it doesn't
> work with a bpstat).  See
> bpstop_status->bpstat_check_location->bkpt_breakpoint_hit, and
> factor out the necessary bits.

Further to my mail yesterday with questions/explanations, attached is
a patch which does what I think you are asking.  Is this what you had
in mind?

Note that I haven't extended it to recognise anything other than the
shared library event breakpoint as a non-inline location.  If possible
I'd like to handle other cases in a separate patch as it would require
further discussion.

Thanks,
Gary

-- 
http://gbenson.net/

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

2011-10-11  Gary Benson  <gbenson@redhat.com>

	* breakpoint.h (stopped_at_non_inline_function): Declare.
	* breakpoint.c (is_non_inline_function,
	stopped_at_non_inline_function): New functions.
	* infrun.c (handle_inferior_event): Don't call skip_inline_frames
	if the stop is at a location where functions cannot be inlined.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 08ff69b..5782d3d 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13325,6 +13325,45 @@ iterate_over_breakpoints (int (*callback) (struct breakpoint *, void *),
   return NULL;
 }
 
+/* Zero if any of the breakpoint's locations could be a location where
+   functions have been inlined, nonzero otherwise.  */
+
+static int
+is_non_inline_function (struct breakpoint *b)
+{
+  /* The shared library event breakpoint is set on the address of a
+     non-inline function.  */
+  if (b->type == bp_shlib_event)
+    return 1;
+
+  return 0;
+}
+
+/* Nonzero if the specified PC cannot be a location where functions
+   have been inlined.  */
+
+int
+stopped_at_non_inline_function (struct address_space *aspace, CORE_ADDR pc)
+{
+  struct breakpoint *b;
+  struct bp_location *bl;
+
+  ALL_BREAKPOINTS (b)
+    {
+      if (!is_non_inline_function (b))
+	continue;
+
+      for (bl = b->loc; bl != NULL; bl = bl->next)
+	{
+	  if (!bl->shlib_disabled
+	      && bpstat_check_location (bl, aspace, pc))
+	    return 1;
+	}
+    }
+
+  return 0;
+}
+
 void
 initialize_breakpoint_ops (void)
 {
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 5e5d1b9..a4e5a4f 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1357,6 +1357,12 @@ extern void end_rbreak_breakpoints (void);
 extern struct breakpoint *iterate_over_breakpoints (int (*) (struct breakpoint *,
 							     void *), void *);
 
+/* Nonzero if the specified PC cannot be a location where functions
+   have been inlined.  */
+
+extern int stopped_at_non_inline_function (struct address_space *aspace,
+					   CORE_ADDR pc);
+			 
 extern int user_breakpoint_p (struct breakpoint *);
 
 #endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cc2e29b..11ba700 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4044,7 +4044,23 @@ handle_inferior_event (struct execution_control_state *ecs)
      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);
+    {
+      struct address_space *aspace = 
+	get_regcache_aspace (get_thread_regcache (ecs->ptid));
+
+      /* skip_inline_frames is expensive, so we avoid it if we can
+	 determine that the address is one where functions cannot have
+	 been inlined.  This improves performance with inferiors that
+	 load a lot of shared libraries, because the solib event
+	 breakpoint is defined as the address of a function (i.e. not
+	 inline).  Note that we have to check the previous PC as well
+	 as the current one to catch cases when we have just
+	 single-stepped over a breakpoint prior to reinstating it.  */
+      if (!stopped_at_non_inline_function (aspace, stop_pc)
+	  && !stopped_at_non_inline_function (aspace,
+					      ecs->event_thread->prev_pc))
+	skip_inline_frames (ecs->ptid);
+    }
 
   if (ecs->event_thread->suspend.stop_signal == TARGET_SIGNAL_TRAP
       && ecs->event_thread->control.trap_expected