* [pushed v2 1/2] Avoid some copying in psymtab.c
2020-04-01 13:55 [pushed v2 0/2] Avoid copying in name lookup Tom Tromey
@ 2020-04-01 13:55 ` Tom Tromey
2020-04-01 13:55 ` [pushed v2 2/2] Avoid copying in lookup_name_info Tom Tromey
1 sibling, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2020-04-01 13:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I noticed that psymtab.c was always copying the search string in
psymtab_search_name, even when it wasn't necessary. This patch
removes this function in favor of using the make_ignore_params feature
of lookup_name_info.
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-04-01 Tom Tromey <tromey@adacore.com>
* psymtab.c (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 | 8 +++++++
gdb/psymtab.c | 59 +++++++++++++++------------------------------------
2 files changed, 25 insertions(+), 42 deletions(-)
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 56c1b68b541..d5e61f88dc6 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);
@@ -482,9 +483,12 @@ psym_lookup_symbol (struct objfile *objfile,
lookup_name_info lookup_name (name, symbol_name_match_type::FULL);
+ lookup_name_info psym_lookup_name = lookup_name.make_ignore_params ();
+
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 +616,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 +634,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 +658,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 +1016,17 @@ static void
psym_expand_symtabs_for_function (struct objfile *objfile,
const char *func_name)
{
+ lookup_name_info base_lookup (func_name, symbol_name_match_type::FULL);
+ lookup_name_info lookup_name = base_lookup.make_ignore_params ();
+
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);
}
--
2.21.1
^ permalink raw reply [flat|nested] 3+ messages in thread* [pushed v2 2/2] Avoid copying in lookup_name_info
2020-04-01 13:55 [pushed v2 0/2] Avoid copying in name lookup Tom Tromey
2020-04-01 13:55 ` [pushed v2 1/2] Avoid some copying in psymtab.c Tom Tromey
@ 2020-04-01 13:55 ` Tom Tromey
1 sibling, 0 replies; 3+ messages in thread
From: Tom Tromey @ 2020-04-01 13:55 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
lookup_name_info always copies the name that is passed in. However,
normally a copy is not needed. This patch changes this class to avoid
copying. This required changing the "name" method to return something
else; I chose a gdb::string_view, to avoid excessive calls to strlen
in the code using the lookup_name_info. However, as this class does
not allow an arbitrary string_view, I've also added a c_str method
that guarantees a \0-terminated result -- a pedantic difference but
one that respects the string_view contract, IMO.
gdb/ChangeLog
2020-04-01 Tom Tromey <tromey@adacore.com>
* symtab.h (class lookup_name_info) <lookup_name_info>: Change
"name" parameter to rvalue reference. Initialize m_name_holder.
<lookup_name_info>: New overloads.
<name>: Return gdb::string_view.
<c_str>: New method.
<make_ignore_params>: Update.
<search_name_hash>: Update.
<language_lookup_name>: Return const char *.
<m_name>: Change type.
* symtab.c (demangle_for_lookup_info::demangle_for_lookup_info)
(demangle_for_lookup_info::demangle_for_lookup_info): Update.
(lookup_name_info::match_any): Update.
* psymtab.c (match_partial_symbol, lookup_partial_symbol):
Update.
* minsyms.c (linkage_name_str): Update.
* language.c (default_symbol_name_matcher): Update.
* dwarf2/read.c (mapped_index_base::find_name_components_bounds):
Update.
* ada-lang.c (ada_fold_name): Change parameter to string_view.
(ada_lookup_name_info::ada_lookup_name_info): Update.
(literal_symbol_name_matcher): Update.
---
gdb/ChangeLog | 24 +++++++++++++++++++++
gdb/ada-lang.c | 36 ++++++++++++++++---------------
gdb/dwarf2/read.c | 2 +-
gdb/language.c | 4 ++--
gdb/minsyms.c | 2 +-
gdb/psymtab.c | 5 ++---
gdb/symtab.c | 8 +++----
gdb/symtab.h | 55 ++++++++++++++++++++++++++++++++++++-----------
8 files changed, 95 insertions(+), 41 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 565299a5ca1..029a7912a03 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -1016,17 +1016,17 @@ ada_encode (const char *decoded)
to next call. */
static char *
-ada_fold_name (const char *name)
+ada_fold_name (gdb::string_view name)
{
static char *fold_buffer = NULL;
static size_t fold_buffer_size = 0;
- int len = strlen (name);
+ int len = name.size ();
GROW_VECT (fold_buffer, fold_buffer_size, len + 1);
if (name[0] == '\'')
{
- strncpy (fold_buffer, name + 1, len - 2);
+ strncpy (fold_buffer, name.data () + 1, len - 2);
fold_buffer[len - 2] = '\000';
}
else
@@ -5657,8 +5657,8 @@ add_nonlocal_symbols (struct obstack *obstackp,
if (num_defns_collected (obstackp) == 0 && global && !is_wild_match)
{
const char *name = ada_lookup_name (lookup_name);
- lookup_name_info name1 (std::string ("<_ada_") + name + '>',
- symbol_name_match_type::FULL);
+ std::string bracket_name = std::string ("<_ada_") + name + '>';
+ lookup_name_info name1 (bracket_name, symbol_name_match_type::FULL);
for (objfile *objfile : current_program_space->objfiles ())
{
@@ -13946,14 +13946,16 @@ do_exact_match (const char *symbol_search_name,
ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
{
- const std::string &user_name = lookup_name.name ();
+ gdb::string_view user_name = lookup_name.name ();
if (user_name[0] == '<')
{
if (user_name.back () == '>')
- m_encoded_name = user_name.substr (1, user_name.size () - 2);
+ m_encoded_name
+ = user_name.substr (1, user_name.size () - 2).to_string ();
else
- m_encoded_name = user_name.substr (1, user_name.size () - 1);
+ m_encoded_name
+ = user_name.substr (1, user_name.size () - 1).to_string ();
m_encoded_p = true;
m_verbatim_p = true;
m_wild_match_p = false;
@@ -13963,19 +13965,19 @@ ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
{
m_verbatim_p = false;
- m_encoded_p = user_name.find ("__") != std::string::npos;
+ m_encoded_p = user_name.find ("__") != gdb::string_view::npos;
if (!m_encoded_p)
{
- const char *folded = ada_fold_name (user_name.c_str ());
+ const char *folded = ada_fold_name (user_name);
const char *encoded = ada_encode_1 (folded, false);
if (encoded != NULL)
m_encoded_name = encoded;
else
- m_encoded_name = user_name;
+ m_encoded_name = user_name.to_string ();
}
else
- m_encoded_name = user_name;
+ m_encoded_name = user_name.to_string ();
/* Handle the 'package Standard' special case. See description
of m_standard_p. */
@@ -14022,12 +14024,12 @@ literal_symbol_name_matcher (const char *symbol_search_name,
const lookup_name_info &lookup_name,
completion_match_result *comp_match_res)
{
- const std::string &name = lookup_name.name ();
+ gdb::string_view name_view = lookup_name.name ();
- int cmp = (lookup_name.completion_mode ()
- ? strncmp (symbol_search_name, name.c_str (), name.size ())
- : strcmp (symbol_search_name, name.c_str ()));
- if (cmp == 0)
+ if (lookup_name.completion_mode ()
+ ? (strncmp (symbol_search_name, name_view.data (),
+ name_view.size ()) == 0)
+ : symbol_search_name == name_view)
{
if (comp_match_res != NULL)
comp_match_res->set_match (symbol_search_name);
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 1ec5c1e582b..6ee33fcd6ed 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3681,7 +3681,7 @@ mapped_index_base::find_name_components_bounds
= this->name_components_casing == case_sensitive_on ? strcmp : strcasecmp;
const char *lang_name
- = lookup_name_without_params.language_lookup_name (lang).c_str ();
+ = lookup_name_without_params.language_lookup_name (lang);
/* Comparison function object for lower_bound that matches against a
given symbol name. */
diff --git a/gdb/language.c b/gdb/language.c
index 454c6dc45a7..c13fd1a406a 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -699,14 +699,14 @@ default_symbol_name_matcher (const char *symbol_search_name,
const lookup_name_info &lookup_name,
completion_match_result *comp_match_res)
{
- const std::string &name = lookup_name.name ();
+ gdb::string_view name = lookup_name.name ();
completion_match_for_lcd *match_for_lcd
= (comp_match_res != NULL ? &comp_match_res->match_for_lcd : NULL);
strncmp_iw_mode mode = (lookup_name.completion_mode ()
? strncmp_iw_mode::NORMAL
: strncmp_iw_mode::MATCH_PARAMS);
- if (strncmp_iw_with_mode (symbol_search_name, name.c_str (), name.size (),
+ if (strncmp_iw_with_mode (symbol_search_name, name.data (), name.size (),
mode, language_minimal, match_for_lcd) == 0)
{
if (comp_match_res != NULL)
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index e238355dc11..d2ac8172eea 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -467,7 +467,7 @@ linkage_name_str (const lookup_name_info &lookup_name)
if (current_language->la_language == language_ada)
return lookup_name.ada ().lookup_name ().c_str ();
- return lookup_name.name ().c_str ();
+ return lookup_name.c_str ();
}
/* See minsyms.h. */
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index d5e61f88dc6..d3569ff0130 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -578,8 +578,7 @@ match_partial_symbol (struct objfile *objfile,
gdb_assert (center < top);
enum language lang = (*center)->ginfo.language ();
- const char *lang_ln
- = name.language_lookup_name (lang).c_str ();
+ const char *lang_ln = name.language_lookup_name (lang);
if (ordered_compare ((*center)->ginfo.search_name (),
lang_ln) >= 0)
@@ -658,7 +657,7 @@ lookup_partial_symbol (struct objfile *objfile,
internal_error (__FILE__, __LINE__,
_("failed internal consistency check"));
if (strcmp_iw_ordered ((*center)->ginfo.search_name (),
- lookup_name.name ().c_str ()) >= 0)
+ lookup_name.c_str ()) >= 0)
{
top = center;
}
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f300d759e03..680280105f5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1790,7 +1790,7 @@ demangle_for_lookup_info::demangle_for_lookup_info
if (lookup_name.ignore_parameters () && lang == language_cplus)
{
gdb::unique_xmalloc_ptr<char> without_params
- = cp_remove_params_if_any (lookup_name.name ().c_str (),
+ = cp_remove_params_if_any (lookup_name.c_str (),
lookup_name.completion_mode ());
if (without_params != NULL)
@@ -1803,9 +1803,9 @@ demangle_for_lookup_info::demangle_for_lookup_info
}
if (lookup_name.match_type () == symbol_name_match_type::SEARCH_NAME)
- m_demangled_name = lookup_name.name ();
+ m_demangled_name = lookup_name.c_str ();
else
- m_demangled_name = demangle_for_lookup (lookup_name.name ().c_str (),
+ m_demangled_name = demangle_for_lookup (lookup_name.c_str (),
lang, storage);
}
@@ -1816,7 +1816,7 @@ lookup_name_info::match_any ()
{
/* Lookup any symbol that "" would complete. I.e., this matches all
symbol names. */
- static const lookup_name_info lookup_name ({}, symbol_name_match_type::FULL,
+ static const lookup_name_info lookup_name ("", symbol_name_match_type::FULL,
true);
return lookup_name;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 18be5d51b85..77f60e6c24a 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -185,29 +185,58 @@ struct demangle_for_lookup_info final
class lookup_name_info final
{
public:
- /* Create a new object. */
- lookup_name_info (std::string name,
+ /* We delete this overload so that the callers are required to
+ explicitly handle the lifetime of the name. */
+ lookup_name_info (std::string &&name,
+ symbol_name_match_type match_type,
+ bool completion_mode = false,
+ bool ignore_parameters = false) = delete;
+
+ /* This overload requires that NAME have a lifetime at least as long
+ as the lifetime of this object. */
+ lookup_name_info (const std::string &name,
+ symbol_name_match_type match_type,
+ bool completion_mode = false,
+ bool ignore_parameters = false)
+ : m_match_type (match_type),
+ m_completion_mode (completion_mode),
+ m_ignore_parameters (ignore_parameters),
+ m_name (name)
+ {}
+
+ /* This overload requires that NAME have a lifetime at least as long
+ as the lifetime of this object. */
+ lookup_name_info (const char *name,
symbol_name_match_type match_type,
bool completion_mode = false,
bool ignore_parameters = false)
: m_match_type (match_type),
m_completion_mode (completion_mode),
m_ignore_parameters (ignore_parameters),
- m_name (std::move (name))
+ m_name (name)
{}
/* Getters. See description of each corresponding field. */
symbol_name_match_type match_type () const { return m_match_type; }
bool completion_mode () const { return m_completion_mode; }
- const std::string &name () const { return m_name; }
+ gdb::string_view name () const { return m_name; }
const bool ignore_parameters () const { return m_ignore_parameters; }
+ /* Like the "name" method but guarantees that the returned string is
+ \0-terminated. */
+ const char *c_str () const
+ {
+ /* Actually this is always guaranteed due to how the class is
+ constructed. */
+ return m_name.data ();
+ }
+
/* Return a version of this lookup name that is usable with
comparisons against symbols have no parameter info, such as
psymbols and GDB index symbols. */
lookup_name_info make_ignore_params () const
{
- return lookup_name_info (m_name, m_match_type, m_completion_mode,
+ return lookup_name_info (c_str (), m_match_type, m_completion_mode,
true /* ignore params */);
}
@@ -218,27 +247,27 @@ class lookup_name_info final
if (!m_demangled_hashes_p[lang])
{
m_demangled_hashes[lang]
- = ::search_name_hash (lang, language_lookup_name (lang).c_str ());
+ = ::search_name_hash (lang, language_lookup_name (lang));
m_demangled_hashes_p[lang] = true;
}
return m_demangled_hashes[lang];
}
/* Get the search name for searches in language LANG. */
- const std::string &language_lookup_name (language lang) const
+ const char *language_lookup_name (language lang) const
{
switch (lang)
{
case language_ada:
- return ada ().lookup_name ();
+ return ada ().lookup_name ().c_str ();
case language_cplus:
- return cplus ().lookup_name ();
+ return cplus ().lookup_name ().c_str ();
case language_d:
- return d ().lookup_name ();
+ return d ().lookup_name ().c_str ();
case language_go:
- return go ().lookup_name ();
+ return go ().lookup_name ().c_str ();
default:
- return m_name;
+ return m_name.data ();
}
}
@@ -287,7 +316,7 @@ class lookup_name_info final
symbol_name_match_type m_match_type;
bool m_completion_mode;
bool m_ignore_parameters;
- std::string m_name;
+ gdb::string_view m_name;
/* Language-specific info. These fields are filled lazily the first
time a lookup is done in the corresponding language. They're
--
2.21.1
^ permalink raw reply [flat|nested] 3+ messages in thread