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

[Pedro Alves <pedro@codesourcery.com>]

Hi Gary,

On Tuesday 04 October 2011 11:25:18, Gary Benson wrote:

>   2011-09-22  Gary Benson  <gbenson@redhat.com>
> 
>         * infrun.c (stopped_at_solib_event_breakpoint): New function.
>         (stopped_at_solib_event_breakpoint_helper): Likewise.
>         (handle_inferior_event): Avoid calling skip_inline_frames
>         when at the solib event breakpoint.
> 
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 225034c..2e49470 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -3102,6 +3102,39 @@ fill_in_stop_func (struct gdbarch *gdbarch,
>      }
>  }
>  
> +/* Helper for stopped_at_solib_event_breakpoint.  */
> +
> +static int
> +stopped_at_solib_event_breakpoint_helper (struct breakpoint *b, void *arg)

As I mentioned before, better would for this to be a property/function of
the breakpoint itself (and its locations) -- meaning "I am / am not an
inlined location".  There's no reason to limit this just to the solib
event breakpoint.  An obvious example without even that generalization
is the thread event breakpoint.

Even if you don't do that, please add this new function (that checks
whether we stopped at an address that can't be inlined) to breakpoint.c
instead, and call it `stopped_at_non_inline_function' or something
along those lines.  We can later rework its internals keeping
its interface.

> +{
> +  struct execution_control_state *ecs
> +    = (struct execution_control_state *) arg;
> +

You need to check that the breakpoint is enabled and installed.

> +  if (b->type == bp_shlib_event)
> +    {
> +      CORE_ADDR prev_pc = ecs->event_thread->prev_pc;
> +      struct bp_location *loc;
> +
> +      for (loc = b->loc; loc; loc = loc->next)
> +       {
> +         if (loc->pspace == current_program_space
> +             && (loc->address == stop_pc || loc->address == prev_pc))

I don't follow the prev_pc check.  What does that intend to do?

When working in the direction of matching from an event back to our tables
and symbolic info, you start from the address_space instead.  Get at the
address space of the current PC
using `get_regcache_aspace (get_thread_regcache (ecs->ptid))'.  But better
yet, since you'd end up rewritting bkpt_breakpoint_hit, it's better instead
to reuse bpstat_check_location (despite the name, it doesn't work with
a bpstat).  See bpstop_status->bpstat_check_location->bkpt_breakpoint_hit,
and factor out the necessary bits.

-- 
Pedro Alves