[1/2] Avoid some copying in psymtab.c

Message ID 20200320204935.19509-2-tromey@adacore.com
State New
Headers show
Series
  • Avoid copying in name lookup
Related show

Commit Message

Tom Tromey March 20, 2020, 8:49 p.m.
I noticed that psymtab.c was always copying the search string in
psymtab_search_name, even when it wasn't necessary.  This patch
replaces this function with a class which holds the allocated search
name, if necessary.

Once I had done that, I noticed that lookup_partial_symbol was
creating a lookup_name_info.  However, this function called in loops,
causing even more excess allocation.  This patch further fixes this by
hosting the creation of the lookup_name_info into the callers.

gdb/ChangeLog
2020-03-20  Tom Tromey  <tromey@adacore.com>

	* psymtab.c (class psymtab_search_name): New class.
	(psymtab_search_name): Remove function.
	(psym_lookup_symbol): Create search name and lookup name here.
	(lookup_partial_symbol): Remove "name" parameter; add
	lookup_name.
	(psym_expand_symtabs_for_function): Update.
---
 gdb/ChangeLog |  9 +++++
 gdb/psymtab.c | 94 ++++++++++++++++++++++++++++-----------------------
 2 files changed, 61 insertions(+), 42 deletions(-)

-- 
2.21.1

Comments

Victor Collod via Gdb-patches March 31, 2020, 5:46 p.m. | #1
On 3/20/20 8:49 PM, Tom Tromey wrote:
> +  psymtab_search_name search_name (name);

> +  lookup_name_info psym_lookup_name (search_name.get (),

> +				     symbol_name_match_type::FULL);

> +


lookup_name_info has built-in support for psymtab-style symbol
names, with no parameter info.  See psym_expand_symtabs_matching:

  lookup_name_info lookup_name = lookup_name_in.make_ignore_params ();

I think doing the same in psym_lookup_symbol is likely to work
just the same, and avoid this new class.

Thanks,
Pedro Alves
Tom Tromey March 31, 2020, 7:35 p.m. | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> lookup_name_info has built-in support for psymtab-style symbol
Pedro> names, with no parameter info.  See psym_expand_symtabs_matching:

Pedro>   lookup_name_info lookup_name = lookup_name_in.make_ignore_params ();

Pedro> I think doing the same in psym_lookup_symbol is likely to work
Pedro> just the same, and avoid this new class.

I knew about this but somehow I thought it only worked "one way".
But, I've tried it and as far as I can tell it works fine and it
simplifies the code, so I've made this change.

Tom

Patch

diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index f77f6d5108f..34b9741e966 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -39,7 +39,8 @@ 
 
 static struct partial_symbol *lookup_partial_symbol (struct objfile *,
 						     struct partial_symtab *,
-						     const char *, int,
+						     const lookup_name_info &,
+						     int,
 						     domain_enum);
 
 static const char *psymtab_to_fullname (struct partial_symtab *ps);
@@ -469,6 +470,38 @@  find_pc_sect_psymbol (struct objfile *objfile,
   return best;
 }
 
+/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
+   not contain any method/function instance information (since this would
+   force reading type information while reading psymtabs).  Therefore,
+   if NAME contains overload information, it must be stripped before searching
+   psymtabs.  */
+
+class psymtab_search_name
+{
+public:
+  explicit psymtab_search_name (const char *name)
+    : m_name (name)
+  {
+    if (current_language->la_language == language_cplus && strchr (name, '('))
+      m_search_name = cp_remove_params (name);
+  }
+
+  /* Return the search name.  */
+  const char *get () const
+  {
+    if (m_search_name == nullptr)
+      return m_name;
+    return m_search_name.get ();
+  }
+
+private:
+
+  /* The original name.  */
+  const char *m_name;
+  /* The search name, if one was needed.  */
+  gdb::unique_xmalloc_ptr<char> m_search_name;
+};
+
 /* Psymtab version of lookup_symbol.  See its definition in
    the definition of quick_symbol_functions in symfile.h.  */
 
@@ -482,9 +515,14 @@  psym_lookup_symbol (struct objfile *objfile,
 
   lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
 
+  psymtab_search_name search_name (name);
+  lookup_name_info psym_lookup_name (search_name.get (),
+				     symbol_name_match_type::FULL);
+
   for (partial_symtab *ps : require_partial_symbols (objfile, true))
     {
-      if (!ps->readin_p () && lookup_partial_symbol (objfile, ps, name,
+      if (!ps->readin_p () && lookup_partial_symbol (objfile, ps,
+						     psym_lookup_name,
 						     psymtab_index, domain))
 	{
 	  struct symbol *sym, *with_opaque = NULL;
@@ -612,42 +650,14 @@  match_partial_symbol (struct objfile *objfile,
   return NULL;
 }
 
-/* Returns the name used to search psymtabs.  Unlike symtabs, psymtabs do
-   not contain any method/function instance information (since this would
-   force reading type information while reading psymtabs).  Therefore,
-   if NAME contains overload information, it must be stripped before searching
-   psymtabs.  */
-
-static gdb::unique_xmalloc_ptr<char>
-psymtab_search_name (const char *name)
-{
-  switch (current_language->la_language)
-    {
-    case language_cplus:
-      {
-	if (strchr (name, '('))
-	  {
-	    gdb::unique_xmalloc_ptr<char> ret = cp_remove_params (name);
-
-	    if (ret)
-	      return ret;
-	  }
-      }
-      break;
-
-    default:
-      break;
-    }
-
-  return make_unique_xstrdup (name);
-}
-
-/* Look, in partial_symtab PST, for symbol whose natural name is NAME.
-   Check the global symbols if GLOBAL, the static symbols if not.  */
+/* Look, in partial_symtab PST, for symbol whose natural name is
+   LOOKUP_NAME.  Check the global symbols if GLOBAL, the static
+   symbols if not.  */
 
 static struct partial_symbol *
 lookup_partial_symbol (struct objfile *objfile,
-		       struct partial_symtab *pst, const char *name,
+		       struct partial_symtab *pst,
+		       const lookup_name_info &lookup_name,
 		       int global, domain_enum domain)
 {
   struct partial_symbol **start, **psym;
@@ -658,10 +668,6 @@  lookup_partial_symbol (struct objfile *objfile,
   if (length == 0)
     return NULL;
 
-  gdb::unique_xmalloc_ptr<char> search_name = psymtab_search_name (name);
-
-  lookup_name_info lookup_name (search_name.get (), symbol_name_match_type::FULL);
-
   start = (global ?
 	   &objfile->partial_symtabs->global_psymbols[pst->globals_offset] :
 	   &objfile->partial_symtabs->static_psymbols[pst->statics_offset]);
@@ -686,7 +692,7 @@  lookup_partial_symbol (struct objfile *objfile,
 	    internal_error (__FILE__, __LINE__,
 			    _("failed internal consistency check"));
 	  if (strcmp_iw_ordered ((*center)->ginfo.search_name (),
-				 search_name.get ()) >= 0)
+				 lookup_name.name ().c_str ()) >= 0)
 	    {
 	      top = center;
 	    }
@@ -1044,14 +1050,18 @@  static void
 psym_expand_symtabs_for_function (struct objfile *objfile,
 				  const char *func_name)
 {
+  psymtab_search_name search_name (func_name);
+  lookup_name_info lookup_name (search_name.get (),
+				symbol_name_match_type::FULL);
+
   for (partial_symtab *ps : require_partial_symbols (objfile, true))
     {
       if (ps->readin_p ())
 	continue;
 
-      if ((lookup_partial_symbol (objfile, ps, func_name, 1, VAR_DOMAIN)
+      if ((lookup_partial_symbol (objfile, ps, lookup_name, 1, VAR_DOMAIN)
 	   != NULL)
-	  || (lookup_partial_symbol (objfile, ps, func_name, 0, VAR_DOMAIN)
+	  || (lookup_partial_symbol (objfile, ps, lookup_name, 0, VAR_DOMAIN)
 	      != NULL))
 	psymtab_to_symtab (objfile, ps);
     }