Re: [3/10] introduce psymtab users

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 25 Apr 2012 20:21:23 +0200, Tom Tromey wrote:
[...]
> @@ -142,6 +167,10 @@ struct partial_symtab
>  
>    unsigned char psymtabs_addrmap_supported;
>  
> +  /* A flag that is temporarily used when searching psymtabs.  */
> +
> +  unsigned char searched_flag;

Please make it the enum.


> @@ -760,6 +769,11 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name,
>  static struct symtab *
>  psymtab_to_symtab (struct partial_symtab *pst)
>  {
> +  /* If it is a shared psymtab, find an unshared psymtab that includes
> +     it.  Any such psymtab will do.  */
> +  while (pst->users != NULL)
> +    pst = pst->users[0];

Currently GDB already has a problem that if it needs to expand symtab A to get
symbol X it already expands also symtabs B, C and D, all of them pretty big
for C++.

With such random choosing of a symtab for expansion this multi-expand
performance hit may get even worse for big C++ programs.

(Not sure if you do not already plan some more general fix for that.)


> @@ -1236,6 +1260,102 @@ map_matching_symbols_psymtab (const char *name, domain_enum namespace,
>      }
>  }	    
>  
> +/* A convenience enum to give names to some constants used in
> +   recursively_search_psymtabs.  */
> +
> +enum
> +  {
> +    PST_NOT_SEARCHED,
> +    PST_SEARCHED_AND_FOUND,
> +    PST_SEARCHED_AND_NOT_FOUND
> +  };
> +
> +/* A helper for expand_symtabs_matching_via_partial that handles
> +   searching included psymtabs.  */

Describe return value, please.

> +
> +static int
> +recursively_search_psymtabs (struct partial_symtab *ps,
> +			     struct objfile *objfile,
> +			     enum search_domain kind,
> +			     int (*name_matcher) (const char *, void *),
> +			     void *data)
> +{
> +  struct partial_symbol **psym;
> +  struct partial_symbol **bound, **gbound, **sbound;
> +  int keep_going = 1;
> +  int result = PST_SEARCHED_AND_NOT_FOUND;
> +  int i;
> +
> +  if (ps->searched_flag != PST_NOT_SEARCHED)
> +    return ps->searched_flag == PST_SEARCHED_AND_FOUND;
> +
> +  /* Recurse into shared psymtabs first, because they may have already
> +     been searched, and this could save some time.  */
> +  for (i = 0; i < ps->number_of_dependencies; ++i)
> +    {
> +      int r;
> +
> +      /* Skip non-shared dependencies, these are handled elsewhere.  */
> +      if (ps->dependencies[i]->users == NULL)
> +	continue;
> +
> +      r = recursively_search_psymtabs (ps->dependencies[i],
> +				       objfile, kind, name_matcher, data);
> +      if (r != 0)
> +	{
> +	  ps->searched_flag = PST_SEARCHED_AND_FOUND;
> +	  return 1;
> +	}
> +    }
> +
> +  gbound = (objfile->global_psymbols.list
> +	    + ps->globals_offset + ps->n_global_syms);

nitpick: While I do not like it IMO GNU Coding Standard says:
	gbound = &objfile->global_psymbols.list[ps->globals_offset + ps->n_global_syms];

But it applies also to the current code.

> +  sbound = (objfile->static_psymbols.list
> +	    + ps->statics_offset + ps->n_static_syms);
> +  bound = gbound;
> +
> +  /* Go through all of the symbols stored in a partial
> +     symtab in one loop.  */
> +  psym = objfile->global_psymbols.list + ps->globals_offset;
> +  while (keep_going)

I do not see why to use the 'keep_going' variable here, just use 'break;'.

But it applies also to the current code.

> +    {
> +      if (psym >= bound)
> +	{
> +	  if (bound == gbound && ps->n_static_syms != 0)
> +	    {
> +	      psym = objfile->static_psymbols.list + ps->statics_offset;
> +	      bound = sbound;
> +	    }
> +	  else
> +	    keep_going = 0;
> +	  continue;
> +	}
> +      else
> +	{
> +	  QUIT;
> +
> +	  if ((kind == ALL_DOMAIN
> +	       || (kind == VARIABLES_DOMAIN
> +		   && SYMBOL_CLASS (*psym) != LOC_TYPEDEF
> +		   && SYMBOL_CLASS (*psym) != LOC_BLOCK)
> +	       || (kind == FUNCTIONS_DOMAIN
> +		   && SYMBOL_CLASS (*psym) == LOC_BLOCK)
> +	       || (kind == TYPES_DOMAIN
> +		   && SYMBOL_CLASS (*psym) == LOC_TYPEDEF))
> +	      && (*name_matcher) (SYMBOL_SEARCH_NAME (*psym), data))
> +	    {
> +	      /* Found a match, so notify our caller.  */
> +	      result = PST_SEARCHED_AND_FOUND;
> +	      keep_going = 0;
> +	    }
> +	}
> +      psym++;
> +    }
> +
> +  ps->searched_flag = result;
> +  return result == PST_SEARCHED_AND_FOUND;
> +}
> +
>  static void
>  expand_symtabs_matching_via_partial
>    (struct objfile *objfile,


Thanks,
Jan