From 596dc4adfff347b4d8dc1f7e4eb57b8f2f342281 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 8 May 2020 14:14:05 -0600 Subject: [PATCH] Speed up psymbol reading by removing a copy 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 * symtab.h (class demangle_result_storage) : New overload. : 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. --- gdb/ChangeLog | 23 +++++++++++++++++++++++ gdb/break-catch-throw.c | 12 +++++++----- gdb/c-exp.y | 11 ++++++----- gdb/c-typeprint.c | 4 ++-- gdb/cp-support.c | 41 ++++++++++++++++++++--------------------- gdb/cp-support.h | 11 ++++++----- gdb/dbxread.c | 9 +++++---- gdb/dwarf2/read.c | 10 ++++------ gdb/gnu-v3-abi.c | 14 ++++++++------ gdb/linespec.c | 7 ++++--- gdb/stabsread.c | 12 ++++++------ gdb/symtab.c | 12 ++++++------ gdb/symtab.h | 17 +++++++---------- 13 files changed, 104 insertions(+), 79 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 5edf06faa55..dbfc94ca052 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,26 @@ +2020-05-08 Tom Tromey + + * symtab.h (class demangle_result_storage) : New + overload. + : 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 * infrun.c (follow_fork): Copy current_line and current_symtab to diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 07dcc7dc0e7..59293c4e570 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -156,26 +156,28 @@ check_status_exception_catchpoint (struct bpstats *bs) if (self->pattern == NULL) return; + const char *name = nullptr; + gdb::unique_xmalloc_ptr 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; } } diff --git a/gdb/c-exp.y b/gdb/c-exp.y index feab51a8e2c..f84691a62e7 100644 --- a/gdb/c-exp.y +++ b/gdb/c-exp.y @@ -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 canon + = cp_canonicalize_string (name.c_str ()); + if (canon != nullptr) + name = canon.get (); + $$ = operator_stoken ((" " + name).c_str ()); } ; diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c index 50d0eaa2dde..aaf9e0dfe0a 100644 --- a/gdb/c-typeprint.c +++ b/gdb/c-typeprint.c @@ -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 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); } diff --git a/gdb/cp-support.c b/gdb/cp-support.c index 6601272e717..92a2e3b4904 100644 --- a/gdb/cp-support.c +++ b/gdb/cp-support.c @@ -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 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 cp_canonicalize_string_full (const char *string, canonicalization_ftype *finder, void *data) { - std::string ret; unsigned int estimated_len; std::unique_ptr 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 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 cp_canonicalize_string (const char *string) { std::unique_ptr 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 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 diff --git a/gdb/cp-support.h b/gdb/cp-support.h index 6f148342714..6ca898315bb 100644 --- a/gdb/cp-support.h +++ b/gdb/cp-support.h @@ -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 cp_canonicalize_string + (const char *string); -extern std::string cp_canonicalize_string_no_typedefs (const char *string); +extern gdb::unique_xmalloc_ptr 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 cp_canonicalize_string_full + (const char *string, canonicalization_ftype *finder, void *data); extern char *cp_class_name_from_physname (const char *physname); diff --git a/gdb/dbxread.c b/gdb/dbxread.c index c0155593e3b..1e1a5dc9b97 100644 --- a/gdb/dbxread.c +++ b/gdb/dbxread.c @@ -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 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 ()); } } diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index ac208991ff7..60b56b8ea81 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -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 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; diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c index 70558437f9c..83deed59657 100644 --- a/gdb/gnu-v3-abi.c +++ b/gdb/gnu-v3-abi.c @@ -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 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)); } diff --git a/gdb/linespec.c b/gdb/linespec.c index 6e4fe6cb771..6007cd22eaf 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -3898,9 +3898,10 @@ find_linespec_symbols (struct linespec_state *state, std::vector *symbols, std::vector *minsyms) { - std::string canon = cp_canonicalize_string_no_typedefs (lookup_name); - if (!canon.empty ()) - lookup_name = canon.c_str (); + gdb::unique_xmalloc_ptr 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 diff --git a/gdb/stabsread.c b/gdb/stabsread.c index 5ef7748a9eb..eac47407105 100644 --- a/gdb/stabsread.c +++ b/gdb/stabsread.c @@ -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 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 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) { diff --git a/gdb/symtab.c b/gdb/symtab.c index 652384cd469..ffd9707d9e7 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -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 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 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 ()); } } diff --git a/gdb/symtab.h b/gdb/symtab.h index 5c5db0fabac..764c567a90b 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -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 &&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 m_malloc; }; -- 2.30.2