Re: [RFA] Improve performance with lots of shared libraries

[Jan Kratochvil <jan.kratochvil@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