This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Improve performance with lots of shared libraries
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 9 Sep 2011 16:51:10 +0200
- Subject: Re: [RFA] Improve performance with lots of shared libraries
- References: <20110909123156.GA1503@redhat.com>
On Fri, 09 Sep 2011 14:31:56 +0200, Gary Benson wrote:
> There are two things I'm not sure about with it as it stands. One is
> to do with program spaces: I noticed that breakpoints have a program
> space, but breakpoint locations also have a program space. Is the way
> I have used these correct?
Tom is working on removing the program space from the breakpoint itself. Or
at least Tom was discussing its removal.
> --- /dev/null
> +++ b/gdb/solib-bp-disable.c
[...]
> +/* Nonzero if multi-threaded inferior support is present. */
> +
> +static int
> +multi_thread_support_availabie (void)
This should be in target.c (and probably renamed).
[...]
> +/* Enable or disable a single solib event breakpoint as appropriate. */
> +
> +static int
> +update_solib_breakpoint (struct breakpoint *b, void *arg)
> +{
> + int enable = *(int *) arg;
> + struct bp_location *loc;
> +
> + if (b->pspace != current_program_space)
> + return 0;
> +
> + if (b->type != bp_shlib_event)
> + return 0;
> +
> + for (loc = b->loc; loc; loc = loc->next)
> + if (loc->pspace == current_program_space)
> + {
> + if (enable && b->enable_state == bp_disabled)
> + b->enable_state = bp_enabled;
> + else if (!enable && b->enable_state == bp_enabled)
> + b->enable_state = bp_disabled;
> + }
After modifying any ENABLE_STATE you have to always call
update_global_location_list. For some events it is already done, it should be
probably per-event (sometimes still duplicating the call but that is probably
OK).
> +
> + return 0;
> +}
> +
> +/* Enable or disable solib event breakpoints as appropriate. */
> +
> +void
> +update_solib_breakpoints ()
> +{
> + int enable = stop_on_solib_events
> + || !multi_thread_support_availabie ()
> + || pending_breakpoints_exist ();
This formatting is not compliant with Emacs-driven GNU Coding Style, it should
be AFAIK:
int enable = (stop_on_solib_events || !multi_thread_support_availabie ()
|| pending_breakpoints_exist ());
[...]
> +void
> +_initialize_solib_bp_disable (void)
> +{
> + /* Observe breakpoint operations so we can enable the shared
> + library event breakpoint if there are breakpoints pending
> + on shared library loads. */
> + observer_attach_breakpoint_created (breakpoint_event);
> + observer_attach_breakpoint_modified (breakpoint_event);
> + observer_attach_breakpoint_deleted (breakpoint_event);
> +
> + /* We also need to watch for inferior creation, because the
> + creation of the shared library event breakpoint does not
> + cause a breakpoint_created notification so inferior_created
> + is the next best thing. */
Isn't more bug the missing notification?
> + observer_attach_inferior_created (inferior_event);
> +
> + /* Observe shared libraries being loaded and unloaded so we
> + can disable the shared library event breakpoint once a
> + thread debugging library has been loaded. */
> + observer_attach_solib_loaded (solib_event);
> + observer_attach_solib_unloaded (solib_event);
> +}
There is open the problem of ambiguous breakpoints but otherwise it looks
great to me, thanks.
Jan