[gdb/symtab] Add reset_compunit_symtab

Add a new function reset_compunit_symtab.

BRANCH-PART

@@ -0,0 +1 @@
+D

COVER-LETTER

@@ -0,0 +1,104 @@
+[gdb/symtab] Cover letter -- Lazy expansion of full symbol table
+
+[ I'm not posting the experimental patch series as such for now.  Available
+here ( https://github.com/vries/gdb/commits/lazy-full-symtab-v3 ). ]
+
+In PR23710, the stated problem is that gdb is slow and memory hungry when
+consuming debug information generated by GCC with LTO.
+
+I. Measurements.
+
+Taking the range of final releases 8.1.1 to 10.2, as well as a recent trunk
+commit (3633d4fb446), and an experiment using cc1:
+...
+$ gdb -q -batch cc1 -ex "b do_rpo_vn"
+...
+we get:
+...
++---------+----------------+
+| Version | real (seconds) |
++---------+----------------+
+| 8.1.1   | 9.42           |
+| 8.2.1   | - (PR23712)    |
+| 8.3.1   | 9.31           |
+| 9.2     | 8.50           |
+| 10.2    | 5.86           |
+| trunk   | 6.36           |
++---------+----------------+
+...
+which is nice progress in the releases.  The regression on trunk since 10.2
+has been filed as PR27937.
+
+[ The 10.2 score can be further improved to 5.23, by setting dwarf
+max-cache-age to 1000.  Defaults to 5, see PR25703. ]
+
+However, the best score is still more than a factor 3 slower than lldb:
+...
++-------------+----------------+
+| Version     | real (seconds) |
++-------------+----------------+
+| gdb 10.2    | 5.86           |
+| lldb 10.0.1 | 1.74           |
++-------------+----------------+
+...
+
+II. Analysis.
+
+Breaking down the 10.2 time of 5.86, we have:
+...
++-----------------+----------------+
+|                 | real (seconds) |
++-----------------+----------------+
+| Minimal symbols | 0.18           |
+| Partial symbols | 2.34           |
+| Full symbols    | 3.34           |
++-----------------+----------------+
+...
+
+So:
+- the minimal symbols and partial symbols are processed for all CUs, while
+  the full symbols are processed only for the necessary CUs
+- still the majority of the time is spent for the full symbols
+
+This is due to the combination of:
+- the one-CU-at-a-time strategy of gdb, and
+- the code generation for LTO which combines several CUs into an
+  artificial CU.
+In other words, LTO increases the scope of processing from individual CUs to
+larger artificial CUs, and consequently things take much longer.
+
+III. Proposed solution.
+
+A way to fix this is to do processing of the full symbols in a lazy fashion.
+
+This patch series implements a first attempt at this.
+
+IV. How to implement.
+
+The current state of trunk is that expanding full symbols is a two part
+process:
+- a builder is created during expansion
+- after expansion the builder is destroyed after delivering the
+  end result: a symbol table.
+
+The problem is that we need a way to do this gradually instead:
+- expand a few symbols
+- get the corresponding symbol table
+- expand a few more symbols
+- get the updated symbol table containing all expanded symbols.
+
+This patch series takes the following approach: it throws away incomplete full
+symbols when it needs to expand more symbols.
+
+V. Resulting performance improvement.
+
+REMEASURE!!!
+
+With current trunk (commit 987610f2d68), we get 3.44, instead of the 6.44
+without this patch series.
+
+VI. Patch series.
+
+The patch series consists of:
+
+[ Output of "git log --reverse --pretty=%s origin/master..HEAD". ]

gdb/dwarf2/read.c

@@ -90,6 +90,8 @@
 #include "count-one-bits.h"
 #include <unordered_set>
 
+extern bool lazy_expand_symtab_p;
+
 /* When == 1, print basic high level tracing messages.
    When > 1, be more verbose.
    This is in contrast to the low level DIE reading of dwarf_die_debug.  */
@@ -1727,7 +1729,8 @@ dwarf2_per_objfile::set_symtab (const dwarf2_per_cu_data *per_cu,
 				compunit_symtab *symtab)
 {
   gdb_assert (per_cu->index < this->m_symtabs.size ());
-  gdb_assert (this->m_symtabs[per_cu->index] == nullptr);
+  gdb_assert (this->m_symtabs[per_cu->index] == nullptr
+	      || symtab == nullptr);
 
   this->m_symtabs[per_cu->index] = symtab;
 }
@@ -5672,6 +5675,10 @@ struct dwarf2_include_psymtab : public partial_symtab
     return nullptr;
   }
 
+  void reset_compunit_symtab (struct objfile *objfile) override
+  {
+  }
+
 private:
   partial_symtab *includer () const
   {
@@ -7811,6 +7818,16 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
   partial_symbol psymbol;
   memset (&psymbol, 0, sizeof (psymbol));
   psymbol.ginfo.set_language (cu->language, &objfile->objfile_obstack);
+  sect_offset sect_off = sect_offset (0);
+  if (lazy_expand_symtab_p)
+    {
+      struct partial_die_info *top_level = pdi;
+      while (top_level->die_parent != nullptr
+	     && top_level->die_parent->tag != DW_TAG_compile_unit
+	     && top_level->die_parent->tag != DW_TAG_partial_unit)
+	top_level = top_level->die_parent;
+      sect_off = top_level->sect_off;
+    }
   psymbol.ginfo.set_section_index (-1);
 
   /* The code below indicates that the psymbol should be installed by
@@ -7978,7 +7995,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
 	}
       cu->per_cu->v.psymtab->add_psymbol
 	(psymbol, *where, per_objfile->per_bfd->partial_symtabs.get (),
-	 objfile);
+	 objfile, sect_off);
     }
 }
 
@@ -8354,7 +8371,13 @@ dwarf2_psymtab::read_symtab (struct objfile *objfile)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
-  gdb_assert (!per_objfile->symtab_set_p (per_cu_data));
+  if (lazy_expand_symtab_p)
+    per_cu_data->interesting_symbols = &interesting_symbols;
+  else
+    {
+      gdb_assert (!per_objfile->symtab_set_p (per_cu_data));
+      per_cu_data->interesting_symbols = nullptr;
+    }
 
   /* If this psymtab is constructed from a debug-only objfile, the
      has_section_at_zero flag will not necessarily be correct.  We
@@ -8569,6 +8592,13 @@ dwarf2_psymtab::get_compunit_symtab (struct objfile *objfile) const
   return per_objfile->get_symtab (per_cu_data);
 }
 
+void
+dwarf2_psymtab::reset_compunit_symtab (struct objfile *objfile)
+{
+  dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
+  per_objfile->set_symtab (per_cu_data, nullptr);
+}
+
 /* Trivial hash function for die_info: the hash value of a DIE
    is its offset in .debug_info for this objfile.  */
 
@@ -10531,15 +10561,41 @@ read_file_scope (struct die_info *die, struct dwarf2_cu *cu)
   handle_DW_AT_stmt_list (die, cu, fnd.comp_dir, lowpc);
 
   /* Process all dies in compilation unit.  */
-  if (die->child != NULL)
+  if (lazy_expand_symtab_p && cu->per_cu->interesting_symbols
+      && cu->per_cu->interesting_symbols->size () > 0)
     {
-      child_die = die->child;
-      while (child_die && child_die->tag)
+      std::set<sect_offset> expanded;
+      while (expanded.size () != cu->per_cu->interesting_symbols->size ())
 	{
-	  process_die (child_die, cu);
-	  child_die = child_die->sibling;
+	  auto interesting_symbol_it = cu->per_cu->interesting_symbols->cbegin ();
+
+	  for (child_die = die->child; child_die != nullptr && child_die->tag != 0;
+	       child_die = child_die->sibling)
+	    {
+	      if (interesting_symbol_it == cu->per_cu->interesting_symbols->cend ())
+		break;
+
+	      sect_offset interesting_symbol = *interesting_symbol_it;
+
+	      if (interesting_symbol > child_die->sect_off)
+		continue;
+
+	      gdb_assert (interesting_symbol == child_die->sect_off);
+	      std::advance (interesting_symbol_it, 1);
+
+	      if (expanded.count (interesting_symbol) == 1)
+		continue;
+	      expanded.emplace (interesting_symbol);
+
+	      process_die (child_die, cu);
+	    }
 	}
     }
+ else
+   for (child_die = die->child; child_die != nullptr && child_die->tag != 0;
+	child_die = child_die->sibling)
+     process_die (child_die, cu);
+
   per_objfile->sym_cu = nullptr;
 
   /* Decode macro information, if present.  Dwarf 2 macro information
@@ -18986,7 +19042,11 @@ load_partial_dies (const struct die_reader_specs *reader,
 	  if (pdi.raw_name == NULL)
 	    complaint (_("malformed enumerator DIE ignored"));
 	  else if (building_psymtab)
-	    add_partial_symbol (&pdi, cu);
+	    {
+	      if (lazy_expand_symtab_p)
+		pdi.die_parent = parent_die;
+	      add_partial_symbol (&pdi, cu);
+	    }
 
 	  info_ptr = locate_pdi_sibling (reader, &pdi, info_ptr);
 	  continue;
@@ -22262,6 +22322,16 @@ lookup_die_type (struct die_info *die, const struct attribute *attr,
 	 from an inter-CU reference and the type's CU got expanded before
 	 ours.  */
       this_type = read_type_die (type_die, type_cu);
+      if (lazy_expand_symtab_p
+	  && type_cu == cu && type_cu->per_cu->interesting_symbols)
+	{
+	  struct die_info *interesting_die = type_die;
+	  while (interesting_die->parent != nullptr
+		 && interesting_die->parent->parent != nullptr)
+	    interesting_die = interesting_die->parent;
+	  gdb_assert (type_cu->header.sect_off < interesting_die->sect_off);
+	  type_cu->per_cu->interesting_symbols->emplace (interesting_die->sect_off);
+	}
     }
 
   /* If we still don't have a type use an error marker.  */

gdb/dwarf2/read.h

@@ -217,6 +217,8 @@ struct dwarf2_per_cu_data
      functions above.  */
   std::vector <dwarf2_per_cu_data *> *imported_symtabs = nullptr;
 
+  std::set <sect_offset> *interesting_symbols = nullptr;
+
   /* Return true of IMPORTED_SYMTABS is empty or not yet allocated.  */
   bool imported_symtabs_empty () const
   {
@@ -624,6 +626,7 @@ struct dwarf2_psymtab : public partial_symtab
   void expand_psymtab (struct objfile *) override;
   bool readin_p (struct objfile *) const override;
   compunit_symtab *get_compunit_symtab (struct objfile *) const override;
+  void reset_compunit_symtab (struct objfile *) override;
 
   struct dwarf2_per_cu_data *per_cu_data;
 };

gdb/psympriv.h

@@ -24,6 +24,8 @@
 #include "objfiles.h"
 #include "gdbsupport/gdb_string_view.h"
 
+#include <unordered_map>
+
 /* A partial_symbol records the name, domain, and address class of
    symbols whose types we have not parsed yet.  For functions, it also
    contains their memory address, so we can find them from a PC value.
@@ -171,6 +173,9 @@ struct partial_symtab
   virtual struct compunit_symtab *get_compunit_symtab
     (struct objfile *) const = 0;
 
+  virtual void reset_compunit_symtab
+    (struct objfile *) = 0;
+
   /* Return the raw low text address of this partial_symtab.  */
   CORE_ADDR raw_text_low () const
   {
@@ -251,7 +256,7 @@ struct partial_symtab
   void add_psymbol (const partial_symbol &psym,
 		    psymbol_placement where,
 		    psymtab_storage *partial_symtabs,
-		    struct objfile *objfile);
+		    struct objfile *objfile, sect_offset sect_off = sect_offset (0));
 
 
   /* Indicate that this partial symtab is complete.  */
@@ -326,6 +331,35 @@ struct partial_symtab
 
   struct partial_symtab **dependencies = nullptr;
 
+  std::set<sect_offset> interesting_symbols;
+
+  void note_interesting_symbol (partial_symbol *psym)
+  {
+    if (expansion_state == partial_symtab::full)
+      return;
+    expansion_state = partial_symtab::lazy;
+    auto it = sect_off.find (psym);
+    if (it == sect_off.end ())
+      {
+	expansion_state = partial_symtab::full;
+	return;
+      }
+    interesting_symbols.emplace (it->second);
+  }
+
+  void note_no_interesting_symbol ()
+  {
+    if (expansion_state == partial_symtab::full)
+      return;
+    expansion_state = partial_symtab::full;
+    interesting_symbols.clear ();
+    expanded_interesting_symbols = 0;
+  }
+
+  enum expansion_state { unexpanded, lazy, full };
+  enum expansion_state expansion_state = unexpanded;
+  size_t expanded_interesting_symbols = 0;
+
   int number_of_dependencies = 0;
 
   /* Global symbol list.  This list will be sorted after readin to
@@ -343,6 +377,10 @@ struct partial_symtab
 
   std::vector<partial_symbol *> static_psymbols;
 
+  /* The sect_offset corresponding to the partial symbols in this partial
+     symbol table. */
+  std::unordered_map<partial_symbol *, sect_offset> sect_off;
+
   /* True iff objfile->psymtabs_addrmap is properly populated for this
      partial_symtab.  For discontiguous overlapping psymtabs is the only usable
      info in PSYMTABS_ADDRMAP.  */
@@ -393,6 +431,11 @@ struct standard_psymtab : public partial_symtab
     return compunit_symtab;
   }
 
+  void reset_compunit_symtab (struct objfile *) override
+  {
+    compunit_symtab = nullptr;
+  }
+
   /* True if the symtab corresponding to this psymtab has been
      readin.  */
 

gdb/psymtab.c

@@ -37,6 +37,8 @@
 #include <algorithm>
 #include <set>
 
+bool lazy_expand_symtab_p = true;
+
 static struct partial_symbol *lookup_partial_symbol (struct objfile *,
 						     struct partial_symtab *,
 						     const lookup_name_info &,
@@ -196,12 +198,13 @@ psymbol_functions::find_pc_sect_psymtab (struct objfile *objfile,
 			 pc - baseaddr));
       if (pst != NULL)
 	{
+	  struct partial_symbol *p = nullptr;
+
 	  /* FIXME: addrmaps currently do not handle overlayed sections,
 	     so fall back to the non-addrmap case if we're debugging
 	     overlays and the addrmap returned the wrong section.  */
 	  if (overlay_debugging && msymbol.minsym != NULL && section != NULL)
 	    {
-	      struct partial_symbol *p;
 
 	      /* NOTE: This assumes that every psymbol has a
 		 corresponding msymbol, which is not necessarily
@@ -212,6 +215,9 @@ psymbol_functions::find_pc_sect_psymtab (struct objfile *objfile,
 		  || (p->address (objfile)
 		      != BMSYMBOL_VALUE_ADDRESS (msymbol)))
 		goto next;
+
+	      if (lazy_expand_symtab_p)
+		pst->note_interesting_symbol (p);
 	    }
 
 	  /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
@@ -219,6 +225,13 @@ psymbol_functions::find_pc_sect_psymtab (struct objfile *objfile,
 	     granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
 	     a worse chosen section due to the TEXTLOW/TEXTHIGH ranges
 	     overlap.  */
+	  if (lazy_expand_symtab_p && p == nullptr)
+	    {
+	      struct partial_symbol *p2;
+	      p2 = find_pc_sect_psymbol (objfile, pst, pc, section);
+	      if (p2 == nullptr)
+		pst->note_no_interesting_symbol ();
+	    }
 
 	  return pst;
 	}
@@ -244,7 +257,14 @@ psymbol_functions::find_pc_sect_psymtab (struct objfile *objfile,
 	best_pst = find_pc_sect_psymtab_closer (objfile, pc, section, pst,
 						msymbol);
 	if (best_pst != NULL)
-	  return best_pst;
+	  {
+	    struct partial_symbol *p2;
+	    p2 = find_pc_sect_psymbol (objfile, best_pst, pc, section);
+	    if (p2 == nullptr)
+	      best_pst->note_no_interesting_symbol ();
+
+	    return best_pst;
+	  }
       }
 
   return NULL;
@@ -339,6 +359,10 @@ find_pc_sect_psymbol (struct objfile *objfile,
 	}
     }
 
+  if (lazy_expand_symtab_p
+      && best != nullptr)
+    psymtab->note_interesting_symbol (best);
+
   return best;
 }
 
@@ -571,10 +595,18 @@ lookup_partial_symbol (struct objfile *objfile,
 static struct compunit_symtab *
 psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst)
 {
+  gdb_assert (pst->expansion_state != partial_symtab::unexpanded);
+
   /* If it is a shared psymtab, find an unshared psymtab that includes
      it.  Any such psymtab will do.  */
   while (pst->user != NULL)
-    pst = pst->user;
+    {
+      pst = pst->user;
+      if (pst->expansion_state == partial_symtab::unexpanded)
+	pst->expansion_state = partial_symtab::lazy;
+    }
+
+  gdb_assert (pst->expansion_state != partial_symtab::unexpanded);
 
   /* If it's been looked up before, return it.  */
   if (pst->get_compunit_symtab (objfile))
@@ -626,6 +658,7 @@ psymbol_functions::find_last_source_symtab (struct objfile *ofp)
 	}
       else
 	{
+	  cs_pst->note_no_interesting_symbol ();
 	  struct compunit_symtab *cust = psymtab_to_symtab (ofp, cs_pst);
 
 	  if (cust == NULL)
@@ -991,10 +1024,16 @@ psymbol_functions::expand_matching_symbols
   for (partial_symtab *ps : require_partial_symbols (objfile))
     {
       QUIT;
-      if (!ps->readin_p (objfile)
-	  && match_partial_symbol (objfile, ps, global, name, domain,
-				   ordered_compare))
-	psymtab_to_symtab (objfile, ps);
+      if (ps->readin_p (objfile))
+	continue;
+
+      partial_symbol *psym
+	= match_partial_symbol (objfile, ps, global, name, domain,
+				ordered_compare);
+      if (psym == nullptr)
+	continue;
+      ps->note_interesting_symbol (psym);
+      psymtab_to_symtab (objfile, ps);
     }
 }
 
@@ -1098,7 +1137,12 @@ recursively_search_psymtabs
 	    {
 	      /* Found a match, so notify our caller.  */
 	      result = PST_SEARCHED_AND_FOUND;
-	      keep_going = 0;
+	      if (lazy_expand_symtab_p)
+		ps->note_interesting_symbol (*psym);
+	      else
+		{
+		  keep_going = 0;
+		}
 	    }
 	}
       psym++;
@@ -1163,6 +1207,11 @@ psymbol_functions::expand_symtabs_matching
 					  *psym_lookup_name,
 					  symbol_matcher))
 	{
+	  if (symbol_matcher == NULL && lookup_name == NULL)
+	    ps->note_no_interesting_symbol ();
+	  else if (ps->expansion_state == partial_symtab::unexpanded)
+	    ps->expansion_state = partial_symtab::lazy;
+
 	  struct compunit_symtab *symtab =
 	    psymtab_to_symtab (objfile, ps);
 
@@ -1333,7 +1382,7 @@ void
 partial_symtab::add_psymbol (const partial_symbol &psymbol,
 			     psymbol_placement where,
 			     psymtab_storage *partial_symtabs,
-			     struct objfile *objfile)
+			     struct objfile *objfile, sect_offset sect_off)
 {
   bool added;
 
@@ -1353,6 +1402,9 @@ partial_symtab::add_psymbol (const partial_symbol &psymbol,
        ? static_psymbols
        : global_psymbols);
   list.push_back (psym);
+
+  if (to_underlying (sect_off) != 0)
+    this->sect_off.emplace (psym, sect_off);
 }
 
 /* See psympriv.h.  */
@@ -1952,4 +2004,12 @@ just the symbol table structures themselves."),
 	   _("\
 Check consistency of currently expanded psymtabs versus symtabs."),
 	   &maintenancelist);
+
+  add_setshow_boolean_cmd ("lazy-expand-symtab", class_maintenance,
+			   &lazy_expand_symtab_p,
+			   _("Set whether symtabs are expanded lazily."),
+			   _("Show whether symtabs are expanded lazily."),
+			   _("Control whether symtabs are expanded lazily."),
+			   NULL, NULL,
+			   &maintenance_set_cmdlist, &maintenance_show_cmdlist);
 }