[commit] Improve performance with lots of shared libraries

[Gary Benson <gbenson at redhat dot com>]

Pedro Alves wrote:
> Thanks for extending the comment.  The first check is quite
> definitive.  The second is heuristic.  There's nothing preventing
> the event breakpoint function to contain a bit of inlined code, and
> the single-step ending up there.  If the user had set a breakpoint
> on that inlined code, the missing skip_inline_frames call would
> break things.  Fortunately, that's an extremely unlikely scenario.
> But please do adjust the comment to mention that we assume the code
> the single-step takes us to after single-stepping is also not
> inlined.
> 
> Also, let's not trust prev_pc unless we did finish a single-step:
> 
>       if (!stopped_at_non_inline_function (aspace, stop_pc)
>           && !(ecs->event_thread->suspend.stop_signal == TARGET_SIGNAL_TRAP
>                && ecs->event_thread->control.trap_expected
>                && stopped_at_non_inline_function (aspace,
>                                                   ecs->event_thread->prev_pc))
> 
> Please drop the "stopped" from the function's name.  The prev_pc
> case clearly isn't about having stopped at prev_pc.  Something like
> pc_at_non_inline_function would be better.  It is just concerned
> with returning whether its passed in PC is known to point at a
> non-inlined location, whether the thread is currently stopped there
> or not.
> 
> Okay with these changes.

Thanks, I made them and committed it (patch attached).

> To make this generic for all breakpoints/stops, what I have in mind
> would be:
> 
>  - at breakpoint creation or re-set time, check if the locations
>    we've created point at inlined code, and set a flag in the
>    breakpoint's locations.  We know the location is inlined or not
>    from the debug info.  Breakpoint creation is the slow path, so
>    that's okay.
> 
>  - given that we need to single-step over those breakpoints, we also
>    need to know whether the PC after stepping over those breakpoints
>    points at inlined code.  I think we can still do that at
>    breakpoint creation or re-set time.  We'd need to reuse the
>    software single-step machinery to know where the single-step
>    would take us, and record somewhere that those locations point to
>    inline code or not.  We'd also check this list in
>    stopped_at_non_inline_function.  The software single-step
>    machinery would need some cleaning up to make this possible.
>    It's interface, gdbarch_software_single_step, isn't fit for this.
>    The gdbarch hook should return a list of locations where to put
>    the breakpoint, instead of implementations planting the
>    breakpoints themselves, which would be a nice cleanup for other
>    things too.  We'd also need to implement this hook for x86.  It's
>    not implemented currently because x86 can do hardware
>    single-stepping.

Ah, nice!  Would it be appropriate to file a bug containing this
information?  So it doesn't get lost before I have a chance to work
on it?

Thanks,
Gary

-- 
http://gbenson.net/

Attachment: patch
Description: Text document