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

[Pedro Alves <pedro@codesourcery.com>]

On Friday 01 July 2011 17:51:09, Gary Benson wrote:
> Hi all,
> 
> While working on a new linker-debugger interface I took some time out
> to do a bit of profiling to see exactly where gdb is spending its time
> with inferiors that load a lot of shared libraries, and it turned out
> that the top 30% of the profile was update_section_map and the things
> it calls.
> 
> update_section_map is called in exactly one place, by find_pc_section,
> which calls update_section_map if the list of loaded object files has
> changed.
> 
> There are two calls in handle_inferior_event that (indirectly) call
> find_pc_section: find_pc_partial_function, and skip_inline_frames.
> The first of these to be called will end up calling update_section_map
> every time the solib event breakpoint is hit, because the list of
> loaded objects has been changed by the last time the breakpoint was
> hit.
> 
> I walked through handle_inferior_event and it turns out that when
> stop_on_solib_events is unset both the call to
> find_pc_partial_function and the call to skip_inline_frames can be
> omitted, the first because its result is never used, and the second
> because the solib event breakpoint is defined to be the address of
> a function--ie not inlined.

I'd rather a split in two different chunks/concepts, as per
your description of the problem:

1 - find_pc_partial_function is expensive, and as such we should
    only call it when necessary.  Can we somehow only do this:

> /* Don't care about return value; stop_func_start and stop_func_name
>     will both be 0 if it doesn't work.  */
>  find_pc_partial_function (stop_pc, &ecs->stop_func_name,
>			    &ecs->stop_func_start, &ecs->stop_func_end);
>  ecs->stop_func_start
>    += gdbarch_deprecated_function_start_offset (gdbarch);

   bit when necessary, rather than listing some special
   cases when it is _not_ necessary?  That is, make that bit
   a bit more lazy.  E.g, it looks like stops for
   BPSTAT_WHAT_STOP* never need that info.  (beware of places
   that pass the ecs down as argument to some function that
   ends up using those fields).

2 - A way to identify that we're stopped at a place defined to
    be the address of a function, i.e., not inlined.  This is sort
    of what you already have.  Can we move the skip_inline_frames
    call until after bpstat_stop_status is called, so we can
    look if the bpstat contains a bp_shlib_event instead?

> 
> This patch alters handle_inferior_event to check whether a stop is due
> to the solib event breakpoint, and omit the two calls if it is.  I'm
> not 100% convinced there aren't odd corner cases I've missed (the
> surrounding code is pretty dense!) but it passes the tests, and with a
> small benchmark I put together (a little program that loads 1000
> shared libraries) gdb ran the application in 36s with the patch
> against 47s without, a 23% improvement.
> 
> I'd really appreciate feedback from people who know this part of gdb
> well, as well as feedback from those users who are using gdb on
> many-solibs applications as to whether this patch helps.
> 
> Cheers,
> Gary
> 
> 

-- 
Pedro Alves