[PING][PATCH][gdb/symtab] Fix symbol loading performance regression

[Tom de Vries]

On 13-05-19 11:27, Tom de Vries wrote:
> Hi,
> 
> The commit "[gdb/symtab] Fix language of duplicate static minimal symbol"
> introduces a performance regression, when loading a cc1 executable build with
> -O0 -g and gcc 7.4.0.  The performance regression, measured in 'real' time is
> about 175%.
> 
> The slower execution comes from the fact that the fix in symbol_set_names
> makes the call to symbol_find_demangled_name unconditional.
> 
> Fix this by reverting the commit, and redoing the fix as follows.
> 
> Recapturing the original problem, the first time symbol_set_names is called
> with gsymbol.language == lang_auto and linkage_name == "_ZL3foov", the name is
> not present in the per_bfd->demangled_names_hash hash table, so
> symbol_find_demangled_name is called to demangle the name, after which the
> mangled/demangled pair is added to the hashtable.  The call to
> symbol_find_demangled_name also sets gsymbol.language to lang_cplus.
> The second time symbol_set_names is called with gsymbol.language == lang_auto
> and linkage_name == "_ZL3foov", the name is present in the hash table, so the
> demangled name from the hash table is used.  However, the language of the
> symbol remains lang_auto.
> 
> Fix this by adding a field language in struct demangled_name_entry, and using
> the field in symbol_set_names to set the language of gsymbol, if necessary.
> 
> Tested on x86_64-linux.
> 
> OK for trunk?
> 

Ping.

Thanks,
- Tom

> [gdb/symtab] Fix symbol loading performance regression
> 
> gdb/ChangeLog:
> 
> 2019-05-11  Tom de Vries  <tdevries@suse.de>
> 
> 	PR symtab/24545
> 	* symtab.c (struct demangled_name_entry): Add language field.
> 	(symbol_set_names):  Revert "[gdb/symtab] Fix language of duplicate
> 	static minimal symbol".  Set and use language field.
> 
> ---
>  gdb/symtab.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 130d5cd48f..44964533ee 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -713,6 +713,7 @@ symbol_set_language (struct general_symbol_info *gsymbol,
>  struct demangled_name_entry
>  {
>    const char *mangled;
> +  ENUM_BITFIELD(language) language : LANGUAGE_BITS;
>    char demangled[1];
>  };
>  
> @@ -853,11 +854,6 @@ symbol_set_names (struct general_symbol_info *gsymbol,
>    else
>      linkage_name_copy = linkage_name;
>  
> -  /* Set the symbol language.  */
> -  char *demangled_name_ptr
> -    = symbol_find_demangled_name (gsymbol, linkage_name_copy);
> -  gdb::unique_xmalloc_ptr<char> demangled_name (demangled_name_ptr);
> -
>    entry.mangled = linkage_name_copy;
>    slot = ((struct demangled_name_entry **)
>  	  htab_find_slot (per_bfd->demangled_names_hash.get (),
> @@ -870,7 +866,9 @@ symbol_set_names (struct general_symbol_info *gsymbol,
>        || (gsymbol->language == language_go
>  	  && (*slot)->demangled[0] == '\0'))
>      {
> -      int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0;
> +      char *demangled_name = symbol_find_demangled_name (gsymbol,
> +							 linkage_name_copy);
> +      int demangled_len = demangled_name ? strlen (demangled_name) : 0;
>  
>        /* Suppose we have demangled_name==NULL, copy_name==0, and
>  	 linkage_name_copy==linkage_name.  In this case, we already have the
> @@ -906,12 +904,19 @@ symbol_set_names (struct general_symbol_info *gsymbol,
>  	  strcpy (mangled_ptr, linkage_name_copy);
>  	  (*slot)->mangled = mangled_ptr;
>  	}
> +      (*slot)->language = gsymbol->language;
>  
>        if (demangled_name != NULL)
> -	strcpy ((*slot)->demangled, demangled_name.get());
> +	{
> +	  strcpy ((*slot)->demangled, demangled_name);
> +	  xfree (demangled_name);
> +	}
>        else
>  	(*slot)->demangled[0] = '\0';
>      }
> +  else if (gsymbol->language == language_unknown
> +	   || gsymbol->language == language_auto)
> +    gsymbol->language = (*slot)->language;
>  
>    gsymbol->name = (*slot)->mangled;
>    if ((*slot)->demangled[0] != '\0')
>