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

Paul Pluzhnikov
2009-05-12T21:21:02Z

[-- 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. */