Speed up psymbol reading by removing a copy
authorTom Tromey <tom@tromey.com>
Fri, 8 May 2020 20:14:05 +0000 (14:14 -0600)
committerTom Tromey <tromey@adacore.com>
Fri, 8 May 2020 20:14:06 +0000 (14:14 -0600)
I noticed that cp_canonicalize_string and friends copy a
unique_xmalloc_ptr to a std::string.  However, this copy isn't
genuinely needed anywhere, and it serves to slow down DWARF psymbol
reading.

This patch removes the copy and updates the callers to adapt.

This speeds up the reader from 1.906 seconds (mean of 10 runs, of gdb
on a copy of itself) to 1.888 seconds (mean of 10 runs, on the same
copy as the first trial).

gdb/ChangeLog
2020-05-08  Tom Tromey  <tom@tromey.com>

* symtab.h (class demangle_result_storage) <set_malloc_ptr>: New
overload.
<swap_string, m_string>: Remove.
* symtab.c (demangle_for_lookup, completion_list_add_symbol):
Update.
* stabsread.c (define_symbol, read_type): Update.
* linespec.c (find_linespec_symbols): Update.
* gnu-v3-abi.c (gnuv3_get_typeid): Update.
* dwarf2/read.c (dwarf2_canonicalize_name): Update.
* dbxread.c (read_dbx_symtab): Update.
* cp-support.h (cp_canonicalize_string_full)
(cp_canonicalize_string, cp_canonicalize_string_no_typedefs):
Return unique_xmalloc_ptr.
* cp-support.c (inspect_type): Update.
(cp_canonicalize_string_full): Return unique_xmalloc_ptr.
(cp_canonicalize_string_no_typedefs, cp_canonicalize_string):
Likewise.
* c-typeprint.c (print_name_maybe_canonical): Update.
* break-catch-throw.c (check_status_exception_catchpoint):
Update.

13 files changed:
gdb/ChangeLog
gdb/break-catch-throw.c
gdb/c-exp.y
gdb/c-typeprint.c
gdb/cp-support.c
gdb/cp-support.h
gdb/dbxread.c
gdb/dwarf2/read.c
gdb/gnu-v3-abi.c
gdb/linespec.c
gdb/stabsread.c
gdb/symtab.c
gdb/symtab.h

index 5edf06faa557be3885d276438aa092513e0692ae..dbfc94ca052ef898edbbea9ffd50219732f14582 100644 (file)
@@ -1,3 +1,26 @@
+2020-05-08  Tom Tromey  <tom@tromey.com>
+
+       * symtab.h (class demangle_result_storage) <set_malloc_ptr>: New
+       overload.
+       <swap_string, m_string>: Remove.
+       * symtab.c (demangle_for_lookup, completion_list_add_symbol):
+       Update.
+       * stabsread.c (define_symbol, read_type): Update.
+       * linespec.c (find_linespec_symbols): Update.
+       * gnu-v3-abi.c (gnuv3_get_typeid): Update.
+       * dwarf2/read.c (dwarf2_canonicalize_name): Update.
+       * dbxread.c (read_dbx_symtab): Update.
+       * cp-support.h (cp_canonicalize_string_full)
+       (cp_canonicalize_string, cp_canonicalize_string_no_typedefs):
+       Return unique_xmalloc_ptr.
+       * cp-support.c (inspect_type): Update.
+       (cp_canonicalize_string_full): Return unique_xmalloc_ptr.
+       (cp_canonicalize_string_no_typedefs, cp_canonicalize_string):
+       Likewise.
+       * c-typeprint.c (print_name_maybe_canonical): Update.
+       * break-catch-throw.c (check_status_exception_catchpoint):
+       Update.
+
 2020-05-08  Tom de Vries  <tdevries@suse.de>
 
        * infrun.c (follow_fork): Copy current_line and current_symtab to
index 07dcc7dc0e7371a2df5255c575d83695287e495f..59293c4e57060399b1dbeb3d70f68276434ca110 100644 (file)
@@ -156,26 +156,28 @@ check_status_exception_catchpoint (struct bpstats *bs)
   if (self->pattern == NULL)
     return;
 
+  const char *name = nullptr;
+  gdb::unique_xmalloc_ptr<char> canon;
   try
     {
       struct value *typeinfo_arg;
-      std::string canon;
 
       fetch_probe_arguments (NULL, &typeinfo_arg);
       type_name = cplus_typename_from_type_info (typeinfo_arg);
 
       canon = cp_canonicalize_string (type_name.c_str ());
-      if (!canon.empty ())
-       std::swap (type_name, canon);
+      name = (canon == nullptr
+             ? canon.get ()
+             : type_name.c_str ());
     }
   catch (const gdb_exception_error &e)
     {
       exception_print (gdb_stderr, e);
     }
 
-  if (!type_name.empty ())
+  if (name != nullptr)
     {
-      if (self->pattern->exec (type_name.c_str (), 0, NULL, 0) != 0)
+      if (self->pattern->exec (name, 0, NULL, 0) != 0)
        bs->stop = 0;
     }
 }
index feab51a8e2cb356223cf9fea1043403973cabd38..f84691a62e7aff99838680cfa91263fe61a2b5b1 100644 (file)
@@ -1739,13 +1739,14 @@ oper:   OPERATOR NEW
 
                          c_print_type ($2, NULL, &buf, -1, 0,
                                        &type_print_raw_options);
+                         std::string name = std::move (buf.string ());
 
                          /* This also needs canonicalization.  */
-                         std::string canon
-                           = cp_canonicalize_string (buf.c_str ());
-                         if (canon.empty ())
-                           canon = std::move (buf.string ());
-                         $$ = operator_stoken ((" " + canon).c_str ());
+                         gdb::unique_xmalloc_ptr<char> canon
+                           = cp_canonicalize_string (name.c_str ());
+                         if (canon != nullptr)
+                           name = canon.get ();
+                         $$ = operator_stoken ((" " + name).c_str ());
                        }
        ;
 
index 50d0eaa2ddedb8c863f51cf0a703aef35e996576..aaf9e0dfe0a1a70a5dbae4edfb96c80f168688fa 100644 (file)
@@ -84,14 +84,14 @@ print_name_maybe_canonical (const char *name,
                            const struct type_print_options *flags,
                            struct ui_file *stream)
 {
-  std::string s;
+  gdb::unique_xmalloc_ptr<char> s;
 
   if (!flags->raw)
     s = cp_canonicalize_string_full (name,
                                     find_typedef_for_canonicalize,
                                     (void *) flags);
 
-  fputs_filtered (!s.empty () ? s.c_str () : name, stream);
+  fputs_filtered (s != nullptr ? s.get () : name, stream);
 }
 
 \f
index 6601272e717ccfd53269c945543ad60ab0044a14..92a2e3b4904111ae55541d0444524c679bcb83f0 100644 (file)
@@ -274,12 +274,13 @@ inspect_type (struct demangle_parse_info *info,
 
                 Canonicalize the name again, and store it in the
                 current node (RET_COMP).  */
-             std::string canon = cp_canonicalize_string_no_typedefs (name);
+             gdb::unique_xmalloc_ptr<char> canon
+               = cp_canonicalize_string_no_typedefs (name);
 
-             if (!canon.empty ())
+             if (canon != nullptr)
                {
                  /* Copy the canonicalization into the obstack.  */
-                 name = copy_string_to_obstack (&info->obstack, canon.c_str (), &len);
+                 name = copy_string_to_obstack (&info->obstack, canon.get (), &len);
                }
 
              ret_comp->u.s_name.s = name;
@@ -506,16 +507,15 @@ replace_typedefs (struct demangle_parse_info *info,
 
 /* Parse STRING and convert it to canonical form, resolving any
    typedefs.  If parsing fails, or if STRING is already canonical,
-   return the empty string.  Otherwise return the canonical form.  If
+   return nullptr.  Otherwise return the canonical form.  If
    FINDER is not NULL, then type components are passed to FINDER to be
    looked up.  DATA is passed verbatim to FINDER.  */
 
-std::string
+gdb::unique_xmalloc_ptr<char>
 cp_canonicalize_string_full (const char *string,
                             canonicalization_ftype *finder,
                             void *data)
 {
-  std::string ret;
   unsigned int estimated_len;
   std::unique_ptr<demangle_parse_info> info;
 
@@ -531,41 +531,42 @@ cp_canonicalize_string_full (const char *string,
                                                            estimated_len);
       gdb_assert (us);
 
-      ret = us.get ();
       /* Finally, compare the original string with the computed
         name, returning NULL if they are the same.  */
-      if (ret == string)
-       return std::string ();
+      if (strcmp (us.get (), string) == 0)
+       return nullptr;
+
+      return us;
     }
 
-  return ret;
+  return nullptr;
 }
 
 /* Like cp_canonicalize_string_full, but always passes NULL for
    FINDER.  */
 
-std::string
+gdb::unique_xmalloc_ptr<char>
 cp_canonicalize_string_no_typedefs (const char *string)
 {
   return cp_canonicalize_string_full (string, NULL, NULL);
 }
 
 /* Parse STRING and convert it to canonical form.  If parsing fails,
-   or if STRING is already canonical, return the empty string.
+   or if STRING is already canonical, return nullptr.
    Otherwise return the canonical form.  */
 
-std::string
+gdb::unique_xmalloc_ptr<char>
 cp_canonicalize_string (const char *string)
 {
   std::unique_ptr<demangle_parse_info> info;
   unsigned int estimated_len;
 
   if (cp_already_canonical (string))
-    return std::string ();
+    return nullptr;
 
   info = cp_demangled_name_to_comp (string, NULL);
   if (info == NULL)
-    return std::string ();
+    return nullptr;
 
   estimated_len = strlen (string) * 2;
   gdb::unique_xmalloc_ptr<char> us (cp_comp_to_string (info->tree,
@@ -575,15 +576,13 @@ cp_canonicalize_string (const char *string)
     {
       warning (_("internal error: string \"%s\" failed to be canonicalized"),
               string);
-      return std::string ();
+      return nullptr;
     }
 
-  std::string ret (us.get ());
-
-  if (ret == string)
-    return std::string ();
+  if (strcmp (us.get (), string) == 0)
+    return nullptr;
 
-  return ret;
+  return us;
 }
 
 /* Convert a mangled name to a demangle_component tree.  *MEMORY is
index 6f1483427147d0b47b58294d30efab9f7542c1c4..6ca898315bb1a8d4f9632e73680eab260167c622 100644 (file)
@@ -77,15 +77,16 @@ struct demangle_parse_info
 
 /* Functions from cp-support.c.  */
 
-extern std::string cp_canonicalize_string (const char *string);
+extern gdb::unique_xmalloc_ptr<char> cp_canonicalize_string
+  (const char *string);
 
-extern std::string cp_canonicalize_string_no_typedefs (const char *string);
+extern gdb::unique_xmalloc_ptr<char> cp_canonicalize_string_no_typedefs
+  (const char *string);
 
 typedef const char *(canonicalization_ftype) (struct type *, void *);
 
-extern std::string cp_canonicalize_string_full (const char *string,
-                                               canonicalization_ftype *finder,
-                                               void *data);
+extern gdb::unique_xmalloc_ptr<char> cp_canonicalize_string_full
+  (const char *string, canonicalization_ftype *finder, void *data);
 
 extern char *cp_class_name_from_physname (const char *physname);
 
index c0155593e3b4301021ac06d00cb540db6509c94e..1e1a5dc9b979e520c7859eb1b32046b6670efcff 100644 (file)
@@ -1434,12 +1434,13 @@ read_dbx_symtab (minimal_symbol_reader &reader, struct objfile *objfile)
          if (psymtab_language == language_cplus)
            {
              std::string name (namestring, p - namestring);
-             std::string new_name = cp_canonicalize_string (name.c_str ());
-             if (!new_name.empty ())
+             gdb::unique_xmalloc_ptr<char> new_name
+               = cp_canonicalize_string (name.c_str ());
+             if (new_name != nullptr)
                {
-                 sym_len = new_name.length ();
+                 sym_len = strlen (new_name.get ());
                  sym_name = obstack_strdup (&objfile->objfile_obstack,
-                                            new_name);
+                                            new_name.get ());
                }
            }
 
index ac208991ff775f00a8e6060961adcd89c1c3a55d..60b56b8ea81be49613c47e8205987b0cf730f6a1 100644 (file)
@@ -21736,13 +21736,11 @@ dwarf2_canonicalize_name (const char *name, struct dwarf2_cu *cu,
 {
   if (name && cu->language == language_cplus)
     {
-      std::string canon_name = cp_canonicalize_string (name);
+      gdb::unique_xmalloc_ptr<char> canon_name
+       = cp_canonicalize_string (name);
 
-      if (!canon_name.empty ())
-       {
-         if (canon_name != name)
-           name = objfile->intern (canon_name);
-       }
+      if (canon_name != nullptr)
+       name = objfile->intern (canon_name.get ());
     }
 
   return name;
index 70558437f9c6fdb728fe4516dd9b902ff250b0cc..83deed5965792fa3ef5a7568007f0bbcfdf0c87b 100644 (file)
@@ -1090,7 +1090,8 @@ gnuv3_get_typeid (struct value *value)
   struct type *type;
   struct gdbarch *gdbarch;
   struct value *result;
-  std::string type_name, canonical;
+  std::string type_name;
+  gdb::unique_xmalloc_ptr<char> canonical;
 
   /* We have to handle values a bit trickily here, to allow this code
      to work properly with non_lvalue values that are really just
@@ -1118,8 +1119,9 @@ gnuv3_get_typeid (struct value *value)
      uses.  E.g., GDB tends to use "const char *" as a type name, but
      the demangler uses "char const *".  */
   canonical = cp_canonicalize_string (type_name.c_str ());
-  if (!canonical.empty ())
-    type_name = canonical;
+  const char *name = (canonical == nullptr
+                     ? type_name.c_str ()
+                     : canonical.get ());
 
   typeinfo_type = gnuv3_get_typeid_type (gdbarch);
 
@@ -1135,19 +1137,19 @@ gnuv3_get_typeid (struct value *value)
       vtable = gnuv3_get_vtable (gdbarch, type, address);
       if (vtable == NULL)
        error (_("cannot find typeinfo for object of type '%s'"),
-              type_name.c_str ());
+              name);
       typeinfo_value = value_field (vtable, vtable_field_type_info);
       result = value_ind (value_cast (make_pointer_type (typeinfo_type, NULL),
                                      typeinfo_value));
     }
   else
     {
-      std::string sym_name = std::string ("typeinfo for ") + type_name;
+      std::string sym_name = std::string ("typeinfo for ") + name;
       bound_minimal_symbol minsym
        = lookup_minimal_symbol (sym_name.c_str (), NULL, NULL);
 
       if (minsym.minsym == NULL)
-       error (_("could not find typeinfo symbol for '%s'"), type_name.c_str ());
+       error (_("could not find typeinfo symbol for '%s'"), name);
 
       result = value_at_lazy (typeinfo_type, BMSYMBOL_VALUE_ADDRESS (minsym));
     }
index 6e4fe6cb771418a695f3703ab7f09e3da1cd9add..6007cd22eaf1dfe189e9b195f512d418ae944b38 100644 (file)
@@ -3898,9 +3898,10 @@ find_linespec_symbols (struct linespec_state *state,
                       std::vector <block_symbol> *symbols,
                       std::vector<bound_minimal_symbol> *minsyms)
 {
-  std::string canon = cp_canonicalize_string_no_typedefs (lookup_name);
-  if (!canon.empty ())
-    lookup_name = canon.c_str ();
+  gdb::unique_xmalloc_ptr<char> canon
+    = cp_canonicalize_string_no_typedefs (lookup_name);
+  if (canon != nullptr)
+    lookup_name = canon.get ();
 
   /* It's important to not call expand_symtabs_matching unnecessarily
      as it can really slow things down (by unnecessarily expanding
index 5ef7748a9ebfc59bd9345e6001386d10ef1712ee..eac47407105a7b81068b00a3c5b78da7310002c8 100644 (file)
@@ -738,7 +738,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
   else
     {
     normal:
-      std::string new_name;
+      gdb::unique_xmalloc_ptr<char> new_name;
 
       if (sym->language () == language_cplus)
        {
@@ -748,8 +748,8 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
          name[p - string] = '\0';
          new_name = cp_canonicalize_string (name);
        }
-      if (!new_name.empty ())
-       sym->compute_and_set_names (new_name, true, objfile->per_bfd);
+      if (new_name != nullptr)
+       sym->compute_and_set_names (new_name.get (), true, objfile->per_bfd);
       else
        sym->compute_and_set_names (gdb::string_view (string, p - string), true,
                                    objfile->per_bfd);
@@ -1637,10 +1637,10 @@ again:
              memcpy (name, *pp, p - *pp);
              name[p - *pp] = '\0';
 
-             std::string new_name = cp_canonicalize_string (name);
-             if (!new_name.empty ())
+             gdb::unique_xmalloc_ptr<char> new_name = cp_canonicalize_string (name);
+             if (new_name != nullptr)
                type_name = obstack_strdup (&objfile->objfile_obstack,
-                                           new_name);
+                                           new_name.get ());
            }
          if (type_name == NULL)
            {
index 652384cd469e414a1e5b7adfb118234795e167bd..ffd9707d9e745f482e068c182b843d24b0c3df92 100644 (file)
@@ -1838,9 +1838,9 @@ demangle_for_lookup (const char *name, enum language lang,
 
       /* If we were given a non-mangled name, canonicalize it
         according to the language (so far only for C++).  */
-      std::string canon = cp_canonicalize_string (name);
-      if (!canon.empty ())
-       return storage.swap_string (canon);
+      gdb::unique_xmalloc_ptr<char> canon = cp_canonicalize_string (name);
+      if (canon != nullptr)
+       return storage.set_malloc_ptr (std::move (canon));
     }
   else if (lang == language_d)
     {
@@ -5349,10 +5349,10 @@ completion_list_add_symbol (completion_tracker &tracker,
       /* The call to canonicalize returns the empty string if the input
         string is already in canonical form, thanks to this we don't
         remove the symbol we just added above.  */
-      std::string str
+      gdb::unique_xmalloc_ptr<char> str
        = cp_canonicalize_string_no_typedefs (sym->natural_name ());
-      if (!str.empty ())
-       tracker.remove_completion (str.c_str ());
+      if (str != nullptr)
+       tracker.remove_completion (str.get ());
     }
 }
 
index 5c5db0fabacffaee52cf8dd7517c856c99f8a0cf..764c567a90bb9dd1ca176625f9267861517b2c67 100644 (file)
@@ -2299,20 +2299,18 @@ bool iterate_over_symbols_terminated
 /* Storage type used by demangle_for_lookup.  demangle_for_lookup
    either returns a const char * pointer that points to either of the
    fields of this type, or a pointer to the input NAME.  This is done
-   this way because the underlying functions that demangle_for_lookup
-   calls either return a std::string (e.g., cp_canonicalize_string) or
-   a malloc'ed buffer (libiberty's demangled), and we want to avoid
-   unnecessary reallocation/string copying.  */
+   this way to avoid depending on the precise details of the storage
+   for the string.  */
 class demangle_result_storage
 {
 public:
 
-  /* Swap the std::string storage with STR, and return a pointer to
-     the beginning of the new string.  */
-  const char *swap_string (std::string &str)
+  /* Swap the malloc storage to STR, and return a pointer to the
+     beginning of the new string.  */
+  const char *set_malloc_ptr (gdb::unique_xmalloc_ptr<char> &&str)
   {
-    std::swap (m_string, str);
-    return m_string.c_str ();
+    m_malloc = std::move (str);
+    return m_malloc.get ();
   }
 
   /* Set the malloc storage to now point at PTR.  Any previous malloc
@@ -2326,7 +2324,6 @@ public:
 private:
 
   /* The storage.  */
-  std::string m_string;
   gdb::unique_xmalloc_ptr<char> m_malloc;
 };