Re: [rfc][patch] Eliminate quadratic slow-down on number of solibs.

[Paul Pluzhnikov <ppluzhnikov@google.com>]

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

On Tue, May 12, 2009 at 1:48 AM, Joel Brobecker <brobecker@adacore.com> wrote:

> Perhaps we could get rid of create_overlay_event_breakpoint entirely
> by inlining it at the only location where it remains in use,

There were two locations ...

> and then
> we can rename create_overlay_event_breakpoint_1 back to
> create_overlay_event_breakpoint. Just thinking out loud, not that
> it matters very much; it's just that, if it is potentially expensive
> to call create_overlay_event_breakpoint because it iterates over
> all objfiles, it might be beneficial to make this apparent by forcing
> the caller to do the iteration (one line of code).

Sounds like a good idea.

Revised patch attached (there is a bit of code movement to eliminate
the need for forward prototypes).

Tested on Linux/x86_64 with no regressions.

Thanks,
-- 
Paul Pluzhnikov

2009-05-12  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* breakpoint.h: Add breakpoint_re_set_objfile prototype.

	* breakpoint.c (create_overlay_event_breakpoint): Renamed
	from create_overlay_event_breakpoint_1, old
	create_overlay_event_breakpoint deleted.
	(breakpoint_re_set_objfile): Don't rescan all objfiles
	unnecessarily.
	(breakpoint_re_set): New function.

	* symfile.c (new_symfile_objfile): Call breakpoint_re_set_objfile
	instead of breakpoint_re_set.

	* objfiles.c (objfile_relocate): Likewise.

[-- Attachment #2: gdb-breakpoint-20090512.txt --]
[-- Type: text/plain, Size: 6092 bytes --]

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d403f36..3461824 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -150,8 +150,6 @@ static int watchpoint_check (void *);
 
 static void maintenance_info_breakpoints (char *, int);
 
-static void create_overlay_event_breakpoint (char *);
-
 static int hw_breakpoint_used_count (void);
 
 static int hw_watchpoint_used_count (enum bptype, int *);
@@ -1459,12 +1457,58 @@ reattach_breakpoints (int pid)
   return 0;
 }
 
+static struct breakpoint *
+create_internal_breakpoint (CORE_ADDR address, enum bptype type)
+{
+  static int internal_breakpoint_number = -1;
+  struct symtab_and_line sal;
+  struct breakpoint *b;
+
+  init_sal (&sal);		/* initialize to zeroes */
+
+  sal.pc = address;
+  sal.section = find_pc_overlay (sal.pc);
+
+  b = set_raw_breakpoint (sal, type);
+  b->number = internal_breakpoint_number--;
+  b->disposition = disp_donttouch;
+
+  return b;
+}
+
+static void
+create_overlay_event_breakpoint (char *func_name, struct objfile *objfile)
+{
+  struct breakpoint *b;
+  struct minimal_symbol *m;
+
+  if ((m = lookup_minimal_symbol_text (func_name, objfile)) == NULL)
+    return;
+
+  b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m),
+				  bp_overlay_event);
+  b->addr_string = xstrdup (func_name);
+
+  if (overlay_debugging == ovly_auto)
+    {
+      b->enable_state = bp_enabled;
+      overlay_events_enabled = 1;
+    }
+  else
+    {
+      b->enable_state = bp_disabled;
+      overlay_events_enabled = 0;
+    }
+  update_global_location_list (1);
+}
+
 void
 update_breakpoints_after_exec (void)
 {
   struct breakpoint *b;
   struct breakpoint *temp;
   struct bp_location *bploc;
+  struct objfile *objfile;
 
   /* We're about to delete breakpoints from GDB's lists.  If the
      INSERTED flag is true, GDB will try to lift the breakpoints by
@@ -1559,7 +1603,8 @@ update_breakpoints_after_exec (void)
       }
   }
   /* FIXME what about longjmp breakpoints?  Re-create them here?  */
-  create_overlay_event_breakpoint ("_ovly_debug_event");
+  ALL_OBJFILES (objfile)
+    create_overlay_event_breakpoint ("_ovly_debug_event", objfile);
 }
 
 int
@@ -4380,26 +4425,6 @@ make_breakpoint_permanent (struct breakpoint *b)
     bl->inserted = 1;
 }
 
-static struct breakpoint *
-create_internal_breakpoint (CORE_ADDR address, enum bptype type)
-{
-  static int internal_breakpoint_number = -1;
-  struct symtab_and_line sal;
-  struct breakpoint *b;
-
-  init_sal (&sal);		/* initialize to zeroes */
-
-  sal.pc = address;
-  sal.section = find_pc_overlay (sal.pc);
-
-  b = set_raw_breakpoint (sal, type);
-  b->number = internal_breakpoint_number--;
-  b->disposition = disp_donttouch;
-
-  return b;
-}
-
-
 static void
 create_longjmp_breakpoint (char *func_name)
 {
@@ -4441,40 +4466,6 @@ delete_longjmp_breakpoint (int thread)
       }
 }
 
-static void
-create_overlay_event_breakpoint_1 (char *func_name, struct objfile *objfile)
-{
-  struct breakpoint *b;
-  struct minimal_symbol *m;
-
-  if ((m = lookup_minimal_symbol_text (func_name, objfile)) == NULL)
-    return;
- 
-  b = create_internal_breakpoint (SYMBOL_VALUE_ADDRESS (m), 
-				  bp_overlay_event);
-  b->addr_string = xstrdup (func_name);
-
-  if (overlay_debugging == ovly_auto)
-    {
-      b->enable_state = bp_enabled;
-      overlay_events_enabled = 1;
-    }
-  else 
-    {
-      b->enable_state = bp_disabled;
-      overlay_events_enabled = 0;
-    }
-  update_global_location_list (1);
-}
-
-static void
-create_overlay_event_breakpoint (char *func_name)
-{
-  struct objfile *objfile;
-  ALL_OBJFILES (objfile)
-    create_overlay_event_breakpoint_1 (func_name, objfile);
-}
-
 void
 enable_overlay_breakpoints (void)
 {
@@ -7723,9 +7714,13 @@ breakpoint_re_set_one (void *bint)
   return 0;
 }
 
-/* Re-set all breakpoints after symbols have been re-loaded.  */
+/* Re-set all breakpoints after symbols have been re-loaded.
+
+   If OBJFILE is non-null, create overlay break point only in OBJFILE
+   (speed optimization).  Otherwise rescan all loaded objfiles.  */
+
 void
-breakpoint_re_set (void)
+breakpoint_re_set_objfile (struct objfile *objfile)
 {
   struct breakpoint *b, *temp;
   enum language save_language;
@@ -7744,8 +7739,20 @@ breakpoint_re_set (void)
   }
   set_language (save_language);
   input_radix = save_input_radix;
-  
-  create_overlay_event_breakpoint ("_ovly_debug_event");
+
+  if (objfile == NULL)
+    ALL_OBJFILES (objfile)
+      create_overlay_event_breakpoint ("_ovly_debug_event", objfile);
+  else
+    create_overlay_event_breakpoint ("_ovly_debug_event", objfile);
+}
+
+/* Re-set all breakpoints after symbols have been re-loaded.  */
+
+void
+breakpoint_re_set (void)
+{
+  breakpoint_re_set_objfile (NULL);
 }
 \f
 /* Reset the thread number of this breakpoint:
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 17b2761..43c2f3f 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -687,7 +687,7 @@ extern int breakpoint_thread_match (CORE_ADDR, ptid_t);
 extern void until_break_command (char *, int, int);
 
 extern void breakpoint_re_set (void);
-
+extern void breakpoint_re_set_objfile (struct objfile *);
 extern void breakpoint_re_set_thread (struct breakpoint *);
 
 extern struct breakpoint *set_momentary_breakpoint
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 795d53b..30a4313 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -674,7 +674,7 @@ objfile_relocate (struct objfile *objfile, struct section_offsets *new_offsets)
     }
 
   /* Relocate breakpoints as necessary, after things are relocated. */
-  breakpoint_re_set ();
+  breakpoint_re_set_objfile (objfile);
 }
 \f
 /* Many places in gdb want to test just to see if we have any partial
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 527b0d5..774101d 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -923,7 +923,7 @@ new_symfile_objfile (struct objfile *objfile, int mainline, int verbo)
     }
   else
     {
-      breakpoint_re_set ();
+      breakpoint_re_set_objfile (objfile);
     }
 
   /* We're done reading the symbol file; finish off complaints.  */