[RFA take 2, 1 of 2] Improve many-shared-libraries performance

[RFA take 2, 1 of 2] Improve many-shared-libraries performance

[Gary Benson]

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

Hi all,

A week or so ago I proposed a patch to improve GDB's performance with
inferiors that load a lot of shared libraries.  My patch hinged around
avoiding calls to find_pc_section for solib event breakpoints.  There
were two separate places in handle_inferior_event that (indirectly)
call find_pc_section, and Pedro suggested I make them both lazy rather
than looking specifically for solib event breakpoints and avoiding the
calls only there.

This patch handles part of this, by calling find_pc_partial_function
lazily.  Since the two cases are orthogonal I wanted to get this out
there for review/approval while I work on the second case, which is
proving to be more difficult.  If nothing else this keeps the patches
simpler, but this patch has the potential to improve performance in
other areas in any event.

Is it ok to commit?

Cheers,
Gary

-- 
http://gbenson.net/

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

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3b779de..c4b737e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2299,6 +2299,7 @@ struct execution_control_state
 
   struct target_waitstatus ws;
   int random_signal;
+  int stop_func_filled_in;
   CORE_ADDR stop_func_start;
   CORE_ADDR stop_func_end;
   char *stop_func_name;
@@ -3049,6 +3050,35 @@ handle_syscall_event (struct execution_control_state *ecs)
   return 1;
 }
 
+/* Clear the supplied execution_control_state's stop_func_* fields.  */
+
+static void
+clear_stop_func (struct execution_control_state *ecs)
+{
+  ecs->stop_func_filled_in = 0;
+  ecs->stop_func_start = 0;
+  ecs->stop_func_end = 0;
+  ecs->stop_func_name = NULL;
+}
+
+/* Lazily fill in the execution_control_state's stop_func_* fields.  */
+
+static void
+fill_in_stop_func (struct gdbarch *gdbarch, struct execution_control_state *ecs)
+{
+  if (!ecs->stop_func_filled_in)
+    {
+      /* 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->stop_func_filled_in = 1;
+    }
+}
+
 /* 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.  */
@@ -3889,15 +3919,7 @@ handle_inferior_event (struct execution_control_state *ecs)
       return;
     }
 
-  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);
+  clear_stop_func (ecs);
   ecs->event_thread->stepping_over_breakpoint = 0;
   bpstat_clear (&ecs->event_thread->control.stop_bpstat);
   ecs->event_thread->control.stop_step = 0;
@@ -4359,6 +4381,7 @@ process_event_stop_test:
 	    keep_going (ecs);
 	    return;
 	  }
+	fill_in_stop_func (gdbarch, ecs);
 	if (stop_pc == ecs->stop_func_start
 	    && execution_direction == EXEC_REVERSE)
 	  {
@@ -4511,6 +4534,7 @@ process_event_stop_test:
      a dangling pointer.  */
   frame = get_current_frame ();
   gdbarch = get_frame_arch (frame);
+  fill_in_stop_func (gdbarch, ecs);
 
   /* If stepping through a line, keep going if still within it.
 
@@ -5071,6 +5095,8 @@ handle_step_into_function (struct gdbarch *gdbarch,
   struct symtab *s;
   struct symtab_and_line stop_func_sal, sr_sal;
 
+  fill_in_stop_func (gdbarch, ecs);
+
   s = find_pc_symtab (stop_pc);
   if (s && s->language != language_asm)
     ecs->stop_func_start = gdbarch_skip_prologue (gdbarch,
@@ -5150,6 +5176,8 @@ handle_step_into_function_backward (struct gdbarch *gdbarch,
   struct symtab *s;
   struct symtab_and_line stop_func_sal;
 
+  fill_in_stop_func (gdbarch, ecs);
+
   s = find_pc_symtab (stop_pc);
   if (s && s->language != language_asm)
     ecs->stop_func_start = gdbarch_skip_prologue (gdbarch,

Re: [RFA take 2, 1 of 2] Improve many-shared-libraries performance

[Tom Tromey]

>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:

Gary> Is it ok to commit?

I defer again to Pedro, except in my dedication to nit-picking.

Gary> +fill_in_stop_func (struct gdbarch *gdbarch, struct execution_control_state *ecs)

This line is too long.

Tom

Re: [RFA take 2, 1 of 2] Improve many-shared-libraries performance

[Pedro Alves]

On Tuesday 12 July 2011 14:37:40, Gary Benson wrote:

> Is it ok to commit?

Yes, with Tom's nit fixed, and a ChangeLog entry.
Thank you for doing this.

-- 
Pedro Alves

Re: [committed][RFA take 2, 1 of 2] Improve many-shared-libraries performance

[Gary Benson]

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

Pedro Alves wrote:
> On Tuesday 12 July 2011 14:37:40, Gary Benson wrote:
> > Is it ok to commit?
> 
> Yes, with Tom's nit fixed, and a ChangeLog entry.
> Thank you for doing this.

Fixed and committed.
Thank you for the review.

Cheers,
Gary

-- 
http://gbenson.net/

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

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

	* infrun.c (struct execution_control_state): New member
	stop_func_filled_in.
	(clear_stop_func, fill_in_stop_func): New functions.
	(handle_inferior_event): Call clear_stop_func rather than
	manipulating the execution control state directly.
	Call fill_in_stop_func lazily as required rather than
	directly calling find_pc_partial_function in all cases.

Index: gdb/infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.495
diff -u -r1.495 infrun.c
--- gdb/infrun.c	14 Jul 2011 15:00:19 -0000	1.495
+++ gdb/infrun.c	19 Jul 2011 12:22:53 -0000
@@ -2326,6 +2326,7 @@
 
   struct target_waitstatus ws;
   int random_signal;
+  int stop_func_filled_in;
   CORE_ADDR stop_func_start;
   CORE_ADDR stop_func_end;
   char *stop_func_name;
@@ -3080,6 +3081,36 @@
   return 1;
 }
 
+/* Clear the supplied execution_control_state's stop_func_* fields.  */
+
+static void
+clear_stop_func (struct execution_control_state *ecs)
+{
+  ecs->stop_func_filled_in = 0;
+  ecs->stop_func_start = 0;
+  ecs->stop_func_end = 0;
+  ecs->stop_func_name = NULL;
+}
+
+/* Lazily fill in the execution_control_state's stop_func_* fields.  */
+
+static void
+fill_in_stop_func (struct gdbarch *gdbarch,
+		   struct execution_control_state *ecs)
+{
+  if (!ecs->stop_func_filled_in)
+    {
+      /* 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->stop_func_filled_in = 1;
+    }
+}
+
 /* 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.  */
@@ -3925,15 +3956,7 @@
       return;
     }
 
-  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);
+  clear_stop_func (ecs);
   ecs->event_thread->stepping_over_breakpoint = 0;
   bpstat_clear (&ecs->event_thread->control.stop_bpstat);
   ecs->event_thread->control.stop_step = 0;
@@ -4377,6 +4400,7 @@
 	    keep_going (ecs);
 	    return;
 	  }
+	fill_in_stop_func (gdbarch, ecs);
 	if (stop_pc == ecs->stop_func_start
 	    && execution_direction == EXEC_REVERSE)
 	  {
@@ -4568,6 +4592,7 @@
      a dangling pointer.  */
   frame = get_current_frame ();
   gdbarch = get_frame_arch (frame);
+  fill_in_stop_func (gdbarch, ecs);
 
   /* If stepping through a line, keep going if still within it.
 
@@ -5128,6 +5153,8 @@
   struct symtab *s;
   struct symtab_and_line stop_func_sal, sr_sal;
 
+  fill_in_stop_func (gdbarch, ecs);
+
   s = find_pc_symtab (stop_pc);
   if (s && s->language != language_asm)
     ecs->stop_func_start = gdbarch_skip_prologue (gdbarch,
@@ -5207,6 +5234,8 @@
   struct symtab *s;
   struct symtab_and_line stop_func_sal;
 
+  fill_in_stop_func (gdbarch, ecs);
+
   s = find_pc_symtab (stop_pc);
   if (s && s->language != language_asm)
     ecs->stop_func_start = gdbarch_skip_prologue (gdbarch,