[PATCH 0/4] Improve Ada name lookup performance

[PATCH 0/4] Improve Ada name lookup performance

[Tom Tromey]

This series started when a user noticed that gdb was very slow to
print Ada records in a large program.  I tracked this down and fixed
it in patch #1.  However, then I noticed that, with a little work, an
entire method could be removed from quick_symbol_functions.  This is
desirable because this method was only used by Ada.

I regression tested this on x86-64 Fedora 38.  I also used regression
tested using the .debug_names and .gdb_index target boards.  It's also
been run through the AdaCore internal test suite.

---
Tom Tromey (4):
      Improve performance of Ada name searches
      Always use expand_symtabs_matching in ada-lang.c
      Remove split_style::UNDERSCORE
      Remove quick_symbol_functions::expand_matching_symbols

 gdb/ada-lang.c                | 111 ++++--------------------
 gdb/ada-lang.h                |  14 +++-
 gdb/dwarf2/cooked-index.c     |   6 +-
 gdb/dwarf2/read-debug-names.c |  46 ----------
 gdb/dwarf2/read-gdb-index.c   | 190 ------------------------------------------
 gdb/dwarf2/read.c             |  54 ------------
 gdb/objfiles.h                |   6 --
 gdb/psymtab.c                 | 104 -----------------------
 gdb/psymtab.h                 |   7 --
 gdb/quick-symbol.h            |  27 ------
 gdb/split-name.c              |  13 ---
 gdb/split-name.h              |   2 -
 gdb/symfile-debug.c           |  18 ----
 gdb/symtab.h                  |  13 ++-
 14 files changed, 40 insertions(+), 571 deletions(-)
---
base-commit: 97b29a61f7f001550fe985bf2deea1550e3c20dd
change-id: 20231121-ada-lookup-perf-67f46d937484

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/4] Improve performance of Ada name searches

[Tom Tromey]

A user reported that certain operations -- like printing a large
structure -- could be slow.  I tracked this down to
ada-lang.c:map_matching_symbols taking an inordinate amount of time.
Specifically, calls like the one to look for a parallel "__XVZ"
variable, in ada_to_fixed_type_1, could result in gdb walking over all
the entries in the cooked index over and over.

Looking into this reveals that
cooked_index_functions::expand_matching_symbols is not written
efficiently -- it ignores its "ordered_compare" parameter.  While
fixing this would be good, it turns out that this entire method isn't
needed; so this series removes it.

However, the deletion is not done in this patch.  This one, instead,
fixes the immediate cause of the slowdown, by using
objfile::expand_symtabs_matching when possible.  This approach is
faster because it is more selective about which index entries to
examine.
---
 gdb/ada-lang.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index ff7222c7eed..8c5ab93f3ca 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5572,8 +5572,16 @@ map_matching_symbols (struct objfile *objfile,
 		      match_data &data)
 {
   data.objfile = objfile;
-  objfile->expand_matching_symbols (lookup_name, domain, global,
-				    is_wild_match ? nullptr : compare_names);
+  if (is_wild_match || lookup_name.ada ().standard_p ())
+    objfile->expand_matching_symbols (lookup_name, domain, global,
+				      is_wild_match ? nullptr : compare_names);
+  else
+    objfile->expand_symtabs_matching (nullptr, &lookup_name,
+				      nullptr, nullptr,
+				      global
+				      ? SEARCH_GLOBAL_BLOCK
+				      : SEARCH_STATIC_BLOCK,
+				      domain, ALL_DOMAIN);
 
   const int block_kind = global ? GLOBAL_BLOCK : STATIC_BLOCK;
   for (compunit_symtab *symtab : objfile->compunits ())

-- 
2.41.0

[PATCH 2/4] Always use expand_symtabs_matching in ada-lang.c

[Tom Tromey]

The previous patch fixed the immediate performance problem with Ada
name matching, by having a subset of matches call
expand_symtabs_matching rather than expand_matching_symbols.  However,
it seemed to me that expand_matching_symbols should not be needed at
all.

To achieve this, this patch changes ada_lookup_name_info::split_name
to use the decoded name, rather than the encoded name.  In order to
make this work correctly, a new decoded form is used: one that does
not decode operators (this is already done) and also does not decode
wide characters.  The latter change is done so that changes to the Ada
source charset don't affect the DWARF index.

With this in place, we can change ada-lang.c to always use
expand_symtabs_matching rather than expand_matching_symbols.
---
 gdb/ada-lang.c            | 119 ++++++----------------------------------------
 gdb/ada-lang.h            |  14 ++++--
 gdb/dwarf2/cooked-index.c |   6 ++-
 gdb/symtab.h              |  13 +++--
 4 files changed, 40 insertions(+), 112 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 8c5ab93f3ca..edd68cd2c32 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -1308,7 +1308,7 @@ convert_from_hex_encoded (std::string &out, const char *str, int n)
 /* See ada-lang.h.  */
 
 std::string
-ada_decode (const char *encoded, bool wrap, bool operators)
+ada_decode (const char *encoded, bool wrap, bool operators, bool wide)
 {
   int i;
   int len0;
@@ -1502,7 +1502,7 @@ ada_decode (const char *encoded, bool wrap, bool operators)
 	    i++;
 	}
 
-      if (i < len0 + 3 && encoded[i] == 'U' && isxdigit (encoded[i + 1]))
+      if (wide && i < len0 + 3 && encoded[i] == 'U' && isxdigit (encoded[i + 1]))
 	{
 	  if (convert_from_hex_encoded (decoded, &encoded[i + 1], 2))
 	    {
@@ -1510,7 +1510,7 @@ ada_decode (const char *encoded, bool wrap, bool operators)
 	      continue;
 	    }
 	}
-      else if (i < len0 + 5 && encoded[i] == 'W' && isxdigit (encoded[i + 1]))
+      else if (wide && i < len0 + 5 && encoded[i] == 'W' && isxdigit (encoded[i + 1]))
 	{
 	  if (convert_from_hex_encoded (decoded, &encoded[i + 1], 4))
 	    {
@@ -1518,7 +1518,7 @@ ada_decode (const char *encoded, bool wrap, bool operators)
 	      continue;
 	    }
 	}
-      else if (i < len0 + 10 && encoded[i] == 'W' && encoded[i + 1] == 'W'
+      else if (wide && i < len0 + 10 && encoded[i] == 'W' && encoded[i + 1] == 'W'
 	       && isxdigit (encoded[i + 2]))
 	{
 	  if (convert_from_hex_encoded (decoded, &encoded[i + 2], 8))
@@ -5465,91 +5465,6 @@ ada_add_block_renamings (std::vector<struct block_symbol> &result,
   return result.size () != defns_mark;
 }
 
-/* Implements compare_names, but only applying the comparision using
-   the given CASING.  */
-
-static int
-compare_names_with_case (const char *string1, const char *string2,
-			 enum case_sensitivity casing)
-{
-  while (*string1 != '\0' && *string2 != '\0')
-    {
-      char c1, c2;
-
-      if (isspace (*string1) || isspace (*string2))
-	return strcmp_iw_ordered (string1, string2);
-
-      if (casing == case_sensitive_off)
-	{
-	  c1 = tolower (*string1);
-	  c2 = tolower (*string2);
-	}
-      else
-	{
-	  c1 = *string1;
-	  c2 = *string2;
-	}
-      if (c1 != c2)
-	break;
-
-      string1 += 1;
-      string2 += 1;
-    }
-
-  switch (*string1)
-    {
-    case '(':
-      return strcmp_iw_ordered (string1, string2);
-    case '_':
-      if (*string2 == '\0')
-	{
-	  if (is_name_suffix (string1))
-	    return 0;
-	  else
-	    return 1;
-	}
-      /* FALLTHROUGH */
-    default:
-      if (*string2 == '(')
-	return strcmp_iw_ordered (string1, string2);
-      else
-	{
-	  if (casing == case_sensitive_off)
-	    return tolower (*string1) - tolower (*string2);
-	  else
-	    return *string1 - *string2;
-	}
-    }
-}
-
-/* Compare STRING1 to STRING2, with results as for strcmp.
-   Compatible with strcmp_iw_ordered in that...
-
-       strcmp_iw_ordered (STRING1, STRING2) <= 0
-
-   ... implies...
-
-       compare_names (STRING1, STRING2) <= 0
-
-   (they may differ as to what symbols compare equal).  */
-
-static int
-compare_names (const char *string1, const char *string2)
-{
-  int result;
-
-  /* Similar to what strcmp_iw_ordered does, we need to perform
-     a case-insensitive comparison first, and only resort to
-     a second, case-sensitive, comparison if the first one was
-     not sufficient to differentiate the two strings.  */
-
-  result = compare_names_with_case (string1, string2, case_sensitive_off);
-  if (result == 0)
-    result = compare_names_with_case (string1, string2, case_sensitive_on);
-
-  return result;
-}
-
 /* Convenience function to get at the Ada encoded lookup name for
    LOOKUP_NAME, as a C string.  */
 
@@ -5559,29 +5474,24 @@ ada_lookup_name (const lookup_name_info &lookup_name)
   return lookup_name.ada ().lookup_name ().c_str ();
 }
 
-/* A helper for add_nonlocal_symbols.  Call expand_matching_symbols
+/* A helper for add_nonlocal_symbols.  Expand all necessary symtabs
    for OBJFILE, then walk the objfile's symtabs and update the
    results.  */
 
 static void
 map_matching_symbols (struct objfile *objfile,
 		      const lookup_name_info &lookup_name,
-		      bool is_wild_match,
 		      domain_enum domain,
 		      int global,
 		      match_data &data)
 {
   data.objfile = objfile;
-  if (is_wild_match || lookup_name.ada ().standard_p ())
-    objfile->expand_matching_symbols (lookup_name, domain, global,
-				      is_wild_match ? nullptr : compare_names);
-  else
-    objfile->expand_symtabs_matching (nullptr, &lookup_name,
-				      nullptr, nullptr,
-				      global
-				      ? SEARCH_GLOBAL_BLOCK
-				      : SEARCH_STATIC_BLOCK,
-				      domain, ALL_DOMAIN);
+  objfile->expand_symtabs_matching (nullptr, &lookup_name,
+				    nullptr, nullptr,
+				    global
+				    ? SEARCH_GLOBAL_BLOCK
+				    : SEARCH_STATIC_BLOCK,
+				    domain, ALL_DOMAIN);
 
   const int block_kind = global ? GLOBAL_BLOCK : STATIC_BLOCK;
   for (compunit_symtab *symtab : objfile->compunits ())
@@ -5610,8 +5520,7 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
 
   for (objfile *objfile : current_program_space->objfiles ())
     {
-      map_matching_symbols (objfile, lookup_name, is_wild_match, domain,
-			    global, data);
+      map_matching_symbols (objfile, lookup_name, domain, global, data);
 
       for (compunit_symtab *cu : objfile->compunits ())
 	{
@@ -5631,7 +5540,7 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
       lookup_name_info name1 (bracket_name, symbol_name_match_type::FULL);
 
       for (objfile *objfile : current_program_space->objfiles ())
-	map_matching_symbols (objfile, name1, false, domain, global, data);
+	map_matching_symbols (objfile, name1, domain, global, data);
     }
 }
 
@@ -13297,6 +13206,8 @@ ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
       else
 	m_standard_p = false;
 
+      m_decoded_name = ada_decode (m_encoded_name.c_str (), true, false, false);
+
       /* If the name contains a ".", then the user is entering a fully
 	 qualified entity name, and the match must not be done in wild
 	 mode.  Similarly, if the user wants to complete what looks
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index 9eb9326a86c..14a0be4c037 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -216,10 +216,18 @@ extern const char *ada_decode_symbol (const struct general_symbol_info *);
    the name does not appear to be GNAT-encoded, then the result
    depends on WRAP.  If WRAP is true (the default), then the result is
    simply wrapped in <...>.  If WRAP is false, then the empty string
-   will be returned.  Also, when OPERATORS is false, operator names
-   will not be decoded.  */
+   will be returned.
+
+   When OPERATORS is false, operator names will not be decoded.  By
+   default, they are decoded, e.g., 'Oadd' will be transformed to
+   '"+"'.
+
+   When WIDE is false, wide characters will be left as-is.  By
+   default, they converted from their hex encoding to the host
+   charset.  */
 extern std::string ada_decode (const char *name, bool wrap = true,
-			       bool operators = true);
+			       bool operators = true,
+			       bool wide = true);
 
 extern std::vector<struct block_symbol> ada_lookup_symbol_list
      (const char *, const struct block *, domain_enum);
diff --git a/gdb/dwarf2/cooked-index.c b/gdb/dwarf2/cooked-index.c
index 10631dccecf..ba77f9cb373 100644
--- a/gdb/dwarf2/cooked-index.c
+++ b/gdb/dwarf2/cooked-index.c
@@ -263,7 +263,11 @@ gdb::unique_xmalloc_ptr<char>
 cooked_index_shard::handle_gnat_encoded_entry (cooked_index_entry *entry,
 					       htab_t gnat_entries)
 {
-  std::string canonical = ada_decode (entry->name, false, false);
+  /* We decode Ada names in a particular way: operators and wide
+     characters are left as-is.  This is done to make name matching a
+     bit simpler; and for wide characters, it means the choice of Ada
+     source charset does not affect the indexer directly.  */
+  std::string canonical = ada_decode (entry->name, false, false, false);
   if (canonical.empty ())
     return {};
   std::vector<std::string_view> names = split_name (canonical.c_str (),
diff --git a/gdb/symtab.h b/gdb/symtab.h
index ec2ac4942d3..ff201e3ff26 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -128,21 +128,26 @@ class ada_lookup_name_info final
      peculiarities.  */
   std::vector<std::string_view> split_name () const
   {
-    if (m_verbatim_p || m_standard_p)
+    if (m_verbatim_p)
       {
+	/* For verbatim matches, just return the encoded name
+	   as-is.  */
 	std::vector<std::string_view> result;
-	if (m_standard_p)
-	  result.emplace_back ("standard");
 	result.emplace_back (m_encoded_name);
 	return result;
       }
-    return ::split_name (m_encoded_name.c_str (), split_style::UNDERSCORE);
+    /* Otherwise, split the decoded name for matching.  */
+    return ::split_name (m_decoded_name.c_str (), split_style::DOT_STYLE);
   }
 
 private:
   /* The Ada-encoded lookup name.  */
   std::string m_encoded_name;
 
+  /* The decoded lookup name.  This is formed by calling ada_decode
+     with both 'operators' and 'wide' set to false.  */
+  std::string m_decoded_name;
+
   /* Whether the user-provided lookup name was Ada encoded.  If so,
      then return encoded names in the 'matches' method's 'completion
      match result' output.  */

-- 
2.41.0

[PATCH 3/4] Remove split_style::UNDERSCORE

[Tom Tromey]

The recent changes to the way Ada names are matched means that
split_style::UNDERSCORE is no longer used.  This patch removes it.
---
 gdb/split-name.c | 13 -------------
 gdb/split-name.h |  2 --
 2 files changed, 15 deletions(-)

diff --git a/gdb/split-name.c b/gdb/split-name.c
index 0805cb82700..4ef022cf58e 100644
--- a/gdb/split-name.c
+++ b/gdb/split-name.c
@@ -45,19 +45,6 @@ split_name (const char *name, split_style style)
 	}
       break;
 
-    case split_style::UNDERSCORE:
-      /* Handle the Ada encoded (aka mangled) form here.  */
-      for (const char *iter = strstr (name, "__");
-	   iter != nullptr;
-	   iter = strstr (iter, "__"))
-	{
-	  result.emplace_back (&name[previous_len],
-			       iter - &name[previous_len]);
-	  iter += 2;
-	  previous_len = iter - name;
-	}
-      break;
-
     case split_style::DOT_STYLE:
       /* D and Go-style names.  */
       for (const char *iter = strchr (name, '.');
diff --git a/gdb/split-name.h b/gdb/split-name.h
index ad2862e222d..2674c9b8d27 100644
--- a/gdb/split-name.h
+++ b/gdb/split-name.h
@@ -33,8 +33,6 @@ enum class split_style
   /* Split at ".".  Used by Ada, Go, D.  This has a funny name to work
      around a bug in Bison 2.3, which is used on macOS.  */
   DOT_STYLE,
-  /* Split at "__".  Used by Ada encoded names.  */
-  UNDERSCORE,
 };
 
 /* Split NAME into components at module boundaries.  STYLE indicates

-- 
2.41.0

[PATCH 4/4] Remove quick_symbol_functions::expand_matching_symbols

[Tom Tromey]

The only caller of quick_symbol_functions::expand_matching_symbols was
removed, so now this method and all implementations of it can be
removed.
---
 gdb/dwarf2/read-debug-names.c |  46 ----------
 gdb/dwarf2/read-gdb-index.c   | 190 ------------------------------------------
 gdb/dwarf2/read.c             |  54 ------------
 gdb/objfiles.h                |   6 --
 gdb/psymtab.c                 | 104 -----------------------
 gdb/psymtab.h                 |   7 --
 gdb/quick-symbol.h            |  27 ------
 gdb/symfile-debug.c           |  18 ----
 8 files changed, 452 deletions(-)

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 78e0df27314..3793e3a58b3 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -84,13 +84,6 @@ struct dwarf2_debug_names_index : public dwarf2_base_index_functions
 {
   void dump (struct objfile *objfile) override;
 
-  void expand_matching_symbols
-    (struct objfile *,
-     const lookup_name_info &lookup_name,
-     domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare) override;
-
   bool expand_symtabs_matching
     (struct objfile *objfile,
      gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
@@ -957,45 +950,6 @@ dwarf2_debug_names_index::dump (struct objfile *objfile)
   gdb_printf (".debug_names: exists\n");
 }
 
-void
-dwarf2_debug_names_index::expand_matching_symbols
-  (struct objfile *objfile,
-   const lookup_name_info &name, domain_enum domain,
-   int global,
-   symbol_compare_ftype *ordered_compare)
-{
-  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-
-  mapped_debug_names &map
-    = *(gdb::checked_static_cast<mapped_debug_names *>
-	(per_objfile->per_bfd->index_table.get ()));
-  const block_search_flags block_flags
-    = global ? SEARCH_GLOBAL_BLOCK : SEARCH_STATIC_BLOCK;
-
-  const char *match_name = name.ada ().lookup_name ().c_str ();
-  auto matcher = [&] (const char *symname)
-    {
-      if (ordered_compare == nullptr)
-	return true;
-      return ordered_compare (symname, match_name) == 0;
-    };
-
-  dw2_expand_symtabs_matching_symbol (map, name, matcher,
-				      [&] (offset_type namei)
-    {
-      /* The name was matched, now expand corresponding CUs that were
-	 marked.  */
-      dw2_debug_names_iterator iter (map, block_flags, domain, namei,
-				     per_objfile);
-
-      struct dwarf2_per_cu_data *per_cu;
-      while ((per_cu = iter.next ()) != NULL)
-	dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
-					 nullptr);
-      return true;
-    }, per_objfile);
-}
-
 bool
 dwarf2_debug_names_index::expand_symtabs_matching
   (struct objfile *objfile,
diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 7788626b67a..b1f7d4621e9 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -139,13 +139,6 @@ struct dwarf2_gdb_index : public dwarf2_base_index_functions
      gdb.dwarf2/gdb-index.exp testcase.  */
   void dump (struct objfile *objfile) override;
 
-  void expand_matching_symbols
-    (struct objfile *,
-     const lookup_name_info &lookup_name,
-     domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare) override;
-
   bool expand_symtabs_matching
     (struct objfile *objfile,
      gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
@@ -173,189 +166,6 @@ dwarf2_gdb_index::dump (struct objfile *objfile)
   gdb_printf ("\n");
 }
 
-/* Struct used to manage iterating over all CUs looking for a symbol.  */
-
-struct dw2_symtab_iterator
-{
-  /* The dwarf2_per_objfile owning the CUs we are iterating on.  */
-  dwarf2_per_objfile *per_objfile;
-  /* If set, only look for symbols that match that block.  Valid values are
-     GLOBAL_BLOCK and STATIC_BLOCK.  */
-  std::optional<block_enum> block_index;
-  /* The kind of symbol we're looking for.  */
-  domain_enum domain;
-  /* The list of CUs from the index entry of the symbol,
-     or NULL if not found.  */
-  offset_view vec;
-  /* The next element in VEC to look at.  */
-  int next;
-  /* The number of elements in VEC, or zero if there is no match.  */
-  int length;
-  /* Have we seen a global version of the symbol?
-     If so we can ignore all further global instances.
-     This is to work around gold/15646, inefficient gold-generated
-     indices.  */
-  int global_seen;
-};
-
-/* Initialize the index symtab iterator ITER, offset_type NAMEI variant.  */
-
-static void
-dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
-		      dwarf2_per_objfile *per_objfile,
-		      std::optional<block_enum> block_index,
-		      domain_enum domain, offset_type namei,
-		      mapped_gdb_index &index)
-{
-  iter->per_objfile = per_objfile;
-  iter->block_index = block_index;
-  iter->domain = domain;
-  iter->next = 0;
-  iter->global_seen = 0;
-  iter->vec = {};
-  iter->length = 0;
-
-  gdb_assert (!index.symbol_name_slot_invalid (namei));
-  offset_type vec_idx = index.symbol_vec_index (namei);
-
-  iter->vec = offset_view (index.constant_pool.slice (vec_idx));
-  iter->length = iter->vec[0];
-}
-
-/* Return the next matching CU or NULL if there are no more.  */
-
-static struct dwarf2_per_cu_data *
-dw2_symtab_iter_next (struct dw2_symtab_iterator *iter,
-		      mapped_gdb_index &index)
-{
-  dwarf2_per_objfile *per_objfile = iter->per_objfile;
-
-  for ( ; iter->next < iter->length; ++iter->next)
-    {
-      offset_type cu_index_and_attrs = iter->vec[iter->next + 1];
-      offset_type cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs);
-      gdb_index_symbol_kind symbol_kind =
-	GDB_INDEX_SYMBOL_KIND_VALUE (cu_index_and_attrs);
-      /* Only check the symbol attributes if they're present.
-	 Indices prior to version 7 don't record them,
-	 and indices >= 7 may elide them for certain symbols
-	 (gold does this).  */
-      int attrs_valid = (index.version >= 7
-			 && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
-
-      /* Don't crash on bad data.  */
-      if (cu_index >= per_objfile->per_bfd->all_units.size ())
-	{
-	  complaint (_(".gdb_index entry has bad CU index"
-		       " [in module %s]"), objfile_name (per_objfile->objfile));
-	  continue;
-	}
-
-      dwarf2_per_cu_data *per_cu = per_objfile->per_bfd->get_cu (cu_index);
-
-      /* Skip if already read in.  */
-      if (per_objfile->symtab_set_p (per_cu))
-	continue;
-
-      /* Check static vs global.  */
-      if (attrs_valid)
-	{
-	  bool is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs);
-
-	  if (iter->block_index.has_value ())
-	    {
-	      bool want_static = *iter->block_index == STATIC_BLOCK;
-
-	      if (is_static != want_static)
-		continue;
-	    }
-
-	  /* Work around gold/15646.  */
-	  if (!is_static
-	      && symbol_kind == GDB_INDEX_SYMBOL_KIND_TYPE)
-	    {
-	      if (iter->global_seen)
-		continue;
-
-	      iter->global_seen = 1;
-	    }
-	}
-
-      /* Only check the symbol's kind if it has one.  */
-      if (attrs_valid)
-	{
-	  switch (iter->domain)
-	    {
-	    case VAR_DOMAIN:
-	      if (symbol_kind != GDB_INDEX_SYMBOL_KIND_VARIABLE
-		  && symbol_kind != GDB_INDEX_SYMBOL_KIND_FUNCTION
-		  /* Some types are also in VAR_DOMAIN.  */
-		  && symbol_kind != GDB_INDEX_SYMBOL_KIND_TYPE)
-		continue;
-	      break;
-	    case STRUCT_DOMAIN:
-	      if (symbol_kind != GDB_INDEX_SYMBOL_KIND_TYPE)
-		continue;
-	      break;
-	    case LABEL_DOMAIN:
-	      if (symbol_kind != GDB_INDEX_SYMBOL_KIND_OTHER)
-		continue;
-	      break;
-	    case MODULE_DOMAIN:
-	      if (symbol_kind != GDB_INDEX_SYMBOL_KIND_OTHER)
-		continue;
-	      break;
-	    default:
-	      break;
-	    }
-	}
-
-      ++iter->next;
-      return per_cu;
-    }
-
-  return NULL;
-}
-
-void
-dwarf2_gdb_index::expand_matching_symbols
-  (struct objfile *objfile,
-   const lookup_name_info &name, domain_enum domain,
-   int global,
-   symbol_compare_ftype *ordered_compare)
-{
-  /* Used for Ada.  */
-  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-
-  const block_enum block_kind = global ? GLOBAL_BLOCK : STATIC_BLOCK;
-
-  mapped_gdb_index &index
-    = *(gdb::checked_static_cast<mapped_gdb_index *>
-	(per_objfile->per_bfd->index_table.get ()));
-
-  const char *match_name = name.ada ().lookup_name ().c_str ();
-  auto matcher = [&] (const char *symname)
-  {
-    if (ordered_compare == nullptr)
-      return true;
-    return ordered_compare (symname, match_name) == 0;
-  };
-
-  dw2_expand_symtabs_matching_symbol (index, name, matcher,
-				      [&] (offset_type namei)
-    {
-      struct dw2_symtab_iterator iter;
-      struct dwarf2_per_cu_data *per_cu;
-
-      dw2_symtab_iter_init (&iter, per_objfile, block_kind, domain, namei,
-			    index);
-      while ((per_cu = dw2_symtab_iter_next (&iter, index)) != NULL)
-	dw2_expand_symtabs_matching_one (per_cu, per_objfile, nullptr,
-					 nullptr);
-      return true;
-    }, per_objfile);
-}
-
 /* Helper for dw2_expand_matching symtabs.  Called on each symbol
    matched, to expand corresponding CUs that were marked.  IDX is the
    index of the symbol name that matched.  */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ccf9f18b31b..c8632d1c24c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1635,15 +1635,6 @@ struct readnow_functions : public dwarf2_base_index_functions
   {
   }
 
-  void expand_matching_symbols
-    (struct objfile *,
-     const lookup_name_info &lookup_name,
-     domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare) override
-  {
-  }
-
   bool expand_symtabs_matching
     (struct objfile *objfile,
      gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
@@ -16568,13 +16559,6 @@ struct cooked_index_functions : public dwarf2_base_index_functions
     index->dump (objfile->arch ());
   }
 
-  void expand_matching_symbols
-    (struct objfile *,
-     const lookup_name_info &lookup_name,
-     domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare) override;
-
   bool expand_symtabs_matching
     (struct objfile *objfile,
      gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
@@ -16671,44 +16655,6 @@ cooked_index_functions::find_compunit_symtab_by_address
   return dw2_instantiate_symtab (per_cu, per_objfile, false);
 }
 
-void
-cooked_index_functions::expand_matching_symbols
-     (struct objfile *objfile,
-      const lookup_name_info &lookup_name,
-      domain_enum domain,
-      int global,
-      symbol_compare_ftype *ordered_compare)
-{
-  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
-  cooked_index *table
-    = (gdb::checked_static_cast<cooked_index *>
-       (per_objfile->per_bfd->index_table.get ()));
-  if (table == nullptr)
-    return;
-
-  const block_search_flags search_flags = (global
-					   ? SEARCH_GLOBAL_BLOCK
-					   : SEARCH_STATIC_BLOCK);
-  const language_defn *lang = language_def (language_ada);
-  symbol_name_matcher_ftype *name_match
-    = lang->get_symbol_name_matcher (lookup_name);
-
-  for (const cooked_index_entry *entry : table->all_entries ())
-    {
-      QUIT;
-
-      if (entry->parent_entry != nullptr)
-	continue;
-
-      if (!entry->matches (search_flags)
-	  || !entry->matches (domain))
-	continue;
-
-      if (name_match (entry->canonical, lookup_name, nullptr))
-	dw2_instantiate_symtab (entry->per_cu, per_objfile, false);
-    }
-}
-
 bool
 cooked_index_functions::expand_symtabs_matching
      (struct objfile *objfile,
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index ec9d354e4a7..0cad5961659 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -584,12 +584,6 @@ struct objfile
      code, e.g., DW_TAG_type_unit for dwarf debug info.  */
   void expand_symtabs_with_fullname (const char *fullname);
 
-  /* See quick_symbol_functions.  */
-  void expand_matching_symbols
-    (const lookup_name_info &name, domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare);
-
   /* See quick_symbol_functions.  */
   bool expand_symtabs_matching
     (gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index d4cd5810c20..ca1bcf18c4d 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -318,90 +318,6 @@ psymbol_name_matches (partial_symbol *psym,
   return name_match (psym->ginfo.search_name (), lookup_name, NULL);
 }
 
-/* Look in PST for a symbol in DOMAIN whose name matches NAME.  Search
-   the global block of PST if GLOBAL, and otherwise the static block.
-   MATCH is the comparison operation that returns true iff MATCH (s,
-   NAME), where s is a SYMBOL_SEARCH_NAME.  If ORDERED_COMPARE is
-   non-null, the symbols in the block are assumed to be ordered
-   according to it (allowing binary search).  It must be compatible
-   with MATCH.  Returns the symbol, if found, and otherwise NULL.  */
-
-static struct partial_symbol *
-match_partial_symbol (struct objfile *objfile,
-		      struct partial_symtab *pst, int global,
-		      const lookup_name_info &name, domain_enum domain,
-		      symbol_compare_ftype *ordered_compare)
-{
-  struct partial_symbol **start, **psym;
-  struct partial_symbol **top, **real_top, **bottom, **center;
-  int length = (global
-		? pst->global_psymbols.size ()
-		: pst->static_psymbols.size ());
-  int do_linear_search = 1;
-
-  if (length == 0)
-    return NULL;
-
-  start = (global ?
-	   &pst->global_psymbols[0] :
-	   &pst->static_psymbols[0]);
-
-  if (global && ordered_compare)  /* Can use a binary search.  */
-    {
-      do_linear_search = 0;
-
-      /* Binary search.  This search is guaranteed to end with center
-	 pointing at the earliest partial symbol whose name might be
-	 correct.  At that point *all* partial symbols with an
-	 appropriate name will be checked against the correct
-	 domain.  */
-
-      bottom = start;
-      top = start + length - 1;
-      real_top = top;
-      while (top > bottom)
-	{
-	  center = bottom + (top - bottom) / 2;
-	  gdb_assert (center < top);
-
-	  enum language lang = (*center)->ginfo.language ();
-	  const char *lang_ln = name.language_lookup_name (lang);
-
-	  if (ordered_compare ((*center)->ginfo.search_name (),
-			       lang_ln) >= 0)
-	    top = center;
-	  else
-	    bottom = center + 1;
-	}
-      gdb_assert (top == bottom);
-
-      while (top <= real_top
-	     && psymbol_name_matches (*top, name))
-	{
-	  if (symbol_matches_domain ((*top)->ginfo.language (),
-				     (*top)->domain, domain))
-	    return *top;
-	  top++;
-	}
-    }
-
-  /* Can't use a binary search or else we found during the binary search that
-     we should also do a linear search.  */
-
-  if (do_linear_search)
-    {
-      for (psym = start; psym < start + length; psym++)
-	{
-	  if (symbol_matches_domain ((*psym)->ginfo.language (),
-				     (*psym)->domain, domain)
-	      && psymbol_name_matches (*psym, name))
-	    return *psym;
-	}
-    }
-
-  return NULL;
-}
-
 /* Look, in partial_symtab PST, for symbol whose natural name is
    LOOKUP_NAME.  Check the global symbols if GLOBAL, the static
    symbols if not.  */
@@ -877,26 +793,6 @@ psymtab_to_fullname (struct partial_symtab *ps)
   return ps->fullname;
 }
 
-/* Psymtab version of expand_matching_symbols.  See its definition in
-   the definition of quick_symbol_functions in symfile.h.  */
-
-void
-psymbol_functions::expand_matching_symbols
-  (struct objfile *objfile,
-   const lookup_name_info &name, domain_enum domain,
-   int global,
-   symbol_compare_ftype *ordered_compare)
-{
-  for (partial_symtab *ps : partial_symbols (objfile))
-    {
-      QUIT;
-      if (!ps->readin_p (objfile)
-	  && match_partial_symbol (objfile, ps, global, name, domain,
-				   ordered_compare))
-	psymtab_to_symtab (objfile, ps);
-    }
-}
-
 /* A helper for psym_expand_symtabs_matching that handles searching
    included psymtabs.  This returns true if a symbol is found, and
    false otherwise.  It also updates the 'searched_flag' on the
diff --git a/gdb/psymtab.h b/gdb/psymtab.h
index 005a63cbba8..54ad78bcd73 100644
--- a/gdb/psymtab.h
+++ b/gdb/psymtab.h
@@ -626,13 +626,6 @@ struct psymbol_functions : public quick_symbol_functions
 
   void expand_all_symtabs (struct objfile *objfile) override;
 
-  void expand_matching_symbols
-    (struct objfile *,
-     const lookup_name_info &lookup_name,
-     domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare) override;
-
   bool expand_symtabs_matching
     (struct objfile *objfile,
      gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,
diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h
index a7fea2ccb49..e48eeeda972 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -30,11 +30,6 @@ enum block_search_flag_values
 
 DEF_ENUM_FLAGS_TYPE (enum block_search_flag_values, block_search_flags);
 
-/* Comparison function for symbol look ups.  */
-
-typedef int (symbol_compare_ftype) (const char *string1,
-				    const char *string2);
-
 /* Callback for quick_symbol_functions->map_symbol_filenames.  */
 
 typedef void (symbol_filename_ftype) (const char *filename,
@@ -125,28 +120,6 @@ struct quick_symbol_functions
   /* Read all symbol tables associated with OBJFILE.  */
   virtual void expand_all_symtabs (struct objfile *objfile) = 0;
 
-  /* Find global or static symbols in all tables that are in DOMAIN
-     and for which MATCH (symbol name, NAME) == 0, reading in partial
-     symbol tables as needed.  Look through global symbols if GLOBAL
-     and otherwise static symbols.
-
-     MATCH must be weaker than strcmp_iw_ordered in the sense that
-     strcmp_iw_ordered(x,y) == 0 --> MATCH(x,y) == 0.  ORDERED_COMPARE,
-     if non-null, must be an ordering relation compatible with
-     strcmp_iw_ordered in the sense that
-	    strcmp_iw_ordered(x,y) == 0 --> ORDERED_COMPARE(x,y) == 0
-     and 
-	    strcmp_iw_ordered(x,y) <= 0 --> ORDERED_COMPARE(x,y) <= 0
-     (allowing strcmp_iw_ordered(x,y) < 0 while ORDERED_COMPARE(x, y) == 0).
-  */
-
-  virtual void expand_matching_symbols
-    (struct objfile *,
-     const lookup_name_info &lookup_name,
-     domain_enum domain,
-     int global,
-     symbol_compare_ftype *ordered_compare) = 0;
-
   /* Expand all symbol tables in OBJFILE matching some criteria.
 
      FILE_MATCHER is called for each file in OBJFILE.  The file name
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 85c43719dee..d31ff3353ee 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -388,24 +388,6 @@ objfile::expand_symtabs_with_fullname (const char *fullname)
 				   ALL_DOMAIN);
 }
 
-void
-objfile::expand_matching_symbols
-  (const lookup_name_info &name, domain_enum domain,
-   int global,
-   symbol_compare_ftype *ordered_compare)
-{
-  if (debug_symfile)
-    gdb_printf (gdb_stdlog,
-		"qf->expand_matching_symbols (%s, %s, %d, %s)\n",
-		objfile_debug_name (this),
-		domain_name (domain), global,
-		host_address_to_string (ordered_compare));
-
-  for (const auto &iter : qf_require_partial_symbols ())
-    iter->expand_matching_symbols (this, name, domain, global,
-				   ordered_compare);
-}
-
 bool
 objfile::expand_symtabs_matching
   (gdb::function_view<expand_symtabs_file_matcher_ftype> file_matcher,

-- 
2.41.0

Re: [PATCH 0/4] Improve Ada name lookup performance

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:

Tom> This series started when a user noticed that gdb was very slow to
Tom> print Ada records in a large program.  I tracked this down and fixed
Tom> it in patch #1.  However, then I noticed that, with a little work, an
Tom> entire method could be removed from quick_symbol_functions.  This is
Tom> desirable because this method was only used by Ada.

Tom> I regression tested this on x86-64 Fedora 38.  I also used regression
Tom> tested using the .debug_names and .gdb_index target boards.  It's also
Tom> been run through the AdaCore internal test suite.

I rebased this today and I think it needs a little work.  It regressed
gdb.dap/pause.exp, and then when looking at that I wondered what would
happen if an inferior call caused an unexpected stop... so I think it
may need another new test as well.

Tom

Re: [PATCH 0/4] Improve Ada name lookup performance

[Tom Tromey]

Tom> This series started when a user noticed that gdb was very slow to
Tom> print Ada records in a large program.  I tracked this down and fixed
Tom> it in patch #1.  However, then I noticed that, with a little work, an
Tom> entire method could be removed from quick_symbol_functions.  This is
Tom> desirable because this method was only used by Ada.

Tom> I regression tested this on x86-64 Fedora 38.  I also used regression
Tom> tested using the .debug_names and .gdb_index target boards.  It's also
Tom> been run through the AdaCore internal test suite.

> I rebased this today and I think it needs a little work.  It regressed
> gdb.dap/pause.exp, and then when looking at that I wondered what would
> happen if an inferior call caused an unexpected stop... so I think it
> may need another new test as well.

I meant to send this reply to the DAP "stop reason" series.


I was going to send a different reply to the Ada name lookup series --
I'm checking it in.

Tom