[2/2] Accelerate lookup_symbol_aux_objfile 8x

  Hi,

lookup_symbol_aux_objfile() processing is very ineffective.  For each primary
symtab it searches it and also all its secondary symtabs.  But that means that
secondary symtabs included in many primary symtabs get needlessly searched
many times during one lookup_symbol_aux_objfile() run.

lookup_symbol_aux_objfile does not care in which primary/secondary symtab the
symbol is found.


Jan
gdb/
2014-10-20  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symtab.c (lookup_symbol_aux_objfile): Use ALL_OBJFILE_SYMTABS, inline
	lookup_block_symbol.
  

On Mon, Oct 20, 2014 at 2:45 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> lookup_symbol_aux_objfile() processing is very ineffective.  For each primary
> symtab it searches it and also all its secondary symtabs.  But that means that
> secondary symtabs included in many primary symtabs get needlessly searched
> many times during one lookup_symbol_aux_objfile() run.
>
> lookup_symbol_aux_objfile does not care in which primary/secondary symtab the
> symbol is found.
>
>
> Jan
>
> gdb/
> 2014-10-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * symtab.c (lookup_symbol_aux_objfile): Use ALL_OBJFILE_SYMTABS, inline
>         lookup_block_symbol.
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index c530d50..bc800ef 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -1657,15 +1657,25 @@ lookup_symbol_aux_objfile (struct objfile *objfile, int block_index,
>    const struct block *block;
>    struct symtab *s;
>
> -  ALL_OBJFILE_PRIMARY_SYMTABS (objfile, s)
> +  gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK);
> +
> +  ALL_OBJFILE_SYMTABS (objfile, s)
>      {
> +      struct dict_iterator dict_iter;
> +
>        bv = BLOCKVECTOR (s);
>        block = BLOCKVECTOR_BLOCK (bv, block_index);
> -      sym = lookup_block_symbol (block, name, domain);
> -      if (sym)
> +
> +      for (sym = dict_iter_name_first (block->dict, name, &dict_iter);
> +          sym != NULL;
> +          sym = dict_iter_name_next (name, &dict_iter))
>         {
> -         block_found = block;
> -         return fixup_symbol_section (sym, objfile);
> +         if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
> +                                    SYMBOL_DOMAIN (sym), domain))
> +           {
> +             block_found = block;
> +             return fixup_symbol_section (sym, objfile);
> +           }
>         }
>      }
>
>

Hi.

I think we're going to need some new terminology. :-)
When I think of primary and non-primary symtabs I think of .c/.cc
(primary) and .h (non-primary).
Now we've got "secondary" dwz-created symtabs.
Primary, secondary, and header symtabs?

At any rate, I understand what this patch is trying to achieve, but
there's more that needs to be considered here.
Non-primary (as in s->primary == 0) symtabs share their block vector
with their primary symtabs.
Therefore while this patch avoids searching dwz secondary symtabs
multiple times, we will now search symtabs for headers whereas before
we wouldn't (and thus search symtabs for .c/.cc files an extra time
for each included header).

[Our symbol table data structures and code sucks in so many major ways.
What'd I'd like to see happen is some deeper changes, and have some
planned when I get the time, but I'm not going to impose that on this
patch set. :-)]
  

On Wed, 22 Oct 2014 11:40:18 +0200, Doug Evans wrote:
> When I think of primary and non-primary symtabs I think of .c/.cc
> (primary) and .h (non-primary).
> Now we've got "secondary" dwz-created symtabs.

dwz-created symtabs are primary, they can contain symbols (although that
happens rarely).


> Therefore while this patch avoids searching dwz secondary symtabs
> multiple times, we will now search symtabs for headers whereas before
> we wouldn't (and thus search symtabs for .c/.cc files an extra time
> for each included header).

Addressed in the new patch sent in the other mail, thanks.


> [Our symbol table data structures and code sucks in so many major ways.
> What'd I'd like to see happen is some deeper changes, and have some
> planned when I get the time, but I'm not going to impose that on this
> patch set. :-)]

There are many possible improvements of GDB as usual.  Just I find this
patchset already done and providing in fact good-enough-for-me acceleration.


Jan