RE: [PATCH 2/2] btrace: avoid symbol lookup

["Metzger, Markus T" <markus dot t dot metzger at intel dot com>]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Jan Kratochvil
> Sent: Monday, March 10, 2014 10:43 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 2/2] btrace: avoid symbol lookup
> 
> On Fri, 07 Mar 2014 09:57:45 +0100, Markus Metzger wrote:
> > --- a/gdb/btrace.c
> > +++ b/gdb/btrace.c
> > @@ -451,13 +451,17 @@ ftrace_update_function (struct gdbarch
> *gdbarch,
> >    struct symbol *fun;
> >    struct btrace_insn *last;
> >
> > -  /* Try to determine the function we're in.  We use both types of symbols
> > -     to avoid surprises when we sometimes get a full symbol and sometimes
> > -     only a minimal symbol.  */
> > -  fun = find_pc_function (pc);
> > +  /* Try to determine the function we're in.  */
> >    bmfun = lookup_minimal_symbol_by_pc (pc);
> >    mfun = bmfun.minsym;
> >
> > +  /* We only lookup the symbol in the debug information if we have not
> found
> > +     a minimal symbol.  This avoids the expensive lookup for globally visible
> > +     symbols.  */
> > +  fun = NULL;
> > +  if (mfun == NULL)
> > +    fun = find_pc_function (pc);
> > +
> >    if (fun == NULL && mfun == NULL)
> >      DEBUG_FTRACE ("no symbol at %s", core_addr_to_string_nz (pc));
> >
> [...]
> 
> Behavior after the change is not the same.  DWARF functions symbols can
> span
> over discontiguous ranges with DW_AT_ranges but their corresponding ELF
> function symbols cannot, therefore those are just some approximation.
> 
> Testcase gdb.dwarf2/dw2-objfile-overlap.exp tests such a case, from:
> 	https://sourceware.org/ml/gdb-patches/2011-11/msg00166.html
> 
> For address of symbol "inner":
>  * find_pc_function finds DWARF symbol "inner"
>  * lookup_minimal_symbol_by_pc finds ELF symbol "outer_inner"
> 
> In few Fedora 20 x86_64 -O2 -g built libraries I have not found any
> DW_TAG_subprogram using DW_AT_ranges, only
> DW_TAG_inlined_subroutine use
> DW_AT_ranges.  But that is more a limitation of gcc, other compilers may
> produce DW_TAG_subprogram using DW_AT_ranges with overlapping
> function parts.

I have not seen DW_AT_ranges used for split functions.  OpenMP is a good
candidate for this so I checked what gcc 4.8.2 and a recent icc are doing today.

Both generate separate functions for parallel regions and describe them as
artificial functions in DWARF.  Whereas GCC generates a separate local function
in ELF, ICC puts the parallel region function after the function it was extracted
from and describes both as a single function in ELF.

For GCC we get a function with a funny name in both cases.
For ICC we get a function with a funny name in the DWARF case and the name
of the function from which the parallel region has been extracted in the ELF case.

So the result is different in some cases.  I'm not sure whether it matters, though.

Inline functions are a different topic.  I would like to support them one day.
As you point out below, the existing version does not support inline functions
either, so some work is necessary.
We might need some more work in symbol handling to allow a fast lookup
covering only inline functions.  We could combine this with the minimal symbol
lookup to get both inline functions support and reasonable performance.


> Inlined functions are found by neither find_pc_function nor
> lookup_minimal_symbol_by_pc (block_containing_function (block_for_pc
> ()) finds
> inlined function).  Not sure if it is not a bug of find_pc_function but that
> is off-topic here.
> 
> I do not think it will be hit in real world cases.  GDB would need some better
> abstraction of the symbol kinds to be more systematic in what it outputs.
> 
> It would be good to know how much it helps your usecase as it is not a fully
> clean/transparent change.

As benchmark, I traced "gdb a.out -ex quit" with a breakpoint on quit_force
for debug versions of both gdb and a.out.  The tracing GDB was compiled
without any additional options; the traced GDB was compiled with "-g -O0".
I used a relatively big buffer that contained ~1.5 million instructions.
I used "maintenance time 1" to measure the execution time of "info record".

The first patch improves performance by ~2x.
This patch improves performance by ~3x on top of the first patch.

I first tried to cache the returned symbol and check whether the next PC
belongs to the same function.  My cache hit in ~50% of the cases but showed
no benefit.  For the majority of the other ~50% no symbol was found.  I can't
cache those.

What's missing is a "fast fail", i.e. quickly determine that we won't find any
symbol for this PC.  I won't be able to do this in a reasonable amount of time,
though, so I thought this patch is a compromise between functionality and
performance.


Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052