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: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Gary Benson <gbenson at redhat dot com>
- Date: Fri, 1 Jul 2011 18:45:01 +0100
- Subject: Re: [RFA] Improve performance with lots of shared libraries
- References: <20110701165109.GA3399@redhat.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