Re: [patch] Improve symbol lookup performance noted in PR 15519

[Doug Evans <dje at google dot com>]

On Mon, Jun 3, 2013 at 10:27 AM, Pedro Alves <palves@redhat.com> wrote:
> What I guess I'm still missing to understand it,
> is a short blurb describing what is being skipped and why is
> it safe to be skipped.  :-)
>
> Just to make sure I understand the change -- I see
> cp_lookup_symbol_namespace does:
>
> ...
>   /* Search for name in namespaces imported to this and parent
>      blocks.  */
>   while (block != NULL)
>     {
>       sym = cp_lookup_symbol_imports (scope, name, block,
>                                       domain, 0, 1);
>
>       if (sym)
>         return sym;
>
>       block = BLOCK_SUPERBLOCK (block);
>     }
>
> and a chunk of the speedup comes from skipping that, correct?
> That is, it is supposedly unnecessary to look symbol imports
> when looking up a symbol in a baseclass, right?

Right.
There are two kinds of "using"s: directives and declarations.
"using directives" cannot appear in class scope, and that is what
cp_lookup_symbol_namespace handles.
And, AFAICT, gdb doesn't support "using declarations" yet,
which does affect base class lookup here.

> The patch then also replaces a lookup_symbol_static with a specific
> block call followed by a fallback lookup_static_symbol_aux search over all
> objfiles, by always doing the lookup_static_symbol_aux search over
> all objfiles.  It makes me wonder if it was there for a reason things were
> done that way before, like for something like the same named class/methods
> being implemented/hidden/private in different sos/dlls (therefore not
> really subject to ODR), therefore making it desirable to lookup in the
> same context first.  I have no idea, probably not.  :-)  I guess I'm just
> after getting the analysis/conclusion that led to the change
> recorded for posterity.  :-)

It's kind of a toss up.
Even if someone played games with visibility the previous lookup only
checks the current symtab; there's no guarantee the needed debug info
isn't in another symtab in that objfile (e.g., due to gcc's aggressive
debug info pruning).
Ultimately, I think what we want is to search the current objfile, and
then search the remaining objfiles, but I left that for another day.
Still, searching the current symtab isn't expensive enough that
re-searching it is a problem and it sometimes will win.
So I've modified the patch to keep that behaviour.
Still have to search the world if block != NULL though.  Improving
that's also left for another day.

Attachment: gdb-130605-cp-namespace-3.patch.txt
Description: Text document