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  <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.
This commit is contained in:
Tom Tromey 2020-05-08 14:14:05 -06:00 committed by Tom Tromey
parent bf4cb9bee2
commit 596dc4adff
13 changed files with 104 additions and 79 deletions

View 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

View 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;
}
}

View 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 ());
}
;

View 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);
}

View 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 (strcmp (us.get (), string) == 0)
return nullptr;
if (ret == string)
return std::string ();
return ret;
return us;
}
/* Convert a mangled name to a demangle_component tree. *MEMORY is

View 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);

View 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 ());
}
}

View 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;

View 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));
}

View 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

View 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)
{

View 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 ());
}
}

View 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;
};