Blame SOURCES/gdb-rhbz1103894-slow-gstack-performance.patch

7ab123
<https://sourceware.org/ml/gdb-patches/2014-10/msg00031.html>
7ab123
7ab123
Date: Thu, 2 Oct 2014 17:56:53 +0200
7ab123
From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
7ab123
To: Doug Evans <dje at google dot com>
7ab123
Cc: gdb-patches at sourceware dot org
7ab123
Subject: [patchv2] Fix 100x slowdown regression on DWZ files
7ab123
Message-ID: <20141002155653.GA9001@host2.jankratochvil.net>
7ab123
7ab123
7ab123
--cNdxnHkX5QqsyA0e
7ab123
Content-Type: text/plain; charset=us-ascii
7ab123
Content-Disposition: inline
7ab123
7ab123
On Thu, 02 Oct 2014 01:51:38 +0200, Doug Evans wrote:
7ab123
> I tested this patch with --target_board=dwarf4-gdb-index
7ab123
> and got a failure in m-static.exp:
7ab123
7ab123
That is particularly with -fdebug-types-section.
7ab123
7ab123
7ab123
> Type units read the line table in a separate path,
7ab123
7ab123
OK, therefore I dropped that separate struct dwarf2_lineinfo
7ab123
and reused struct line_header instead.
7ab123
7ab123
7ab123
> OTOH, I do want to avoid any confusion that this patch may inadvertently
7ab123
> introduce. For example, IIUC with your patch as is,
7ab123
> if we read a partial_unit first, before a compile_unit
7ab123
> that has the same stmt_list value, we'll do more processing in
7ab123
> dwarf_decode_lines than we really need to since we only need a file
7ab123
> number to symtab mapping. And if we later read in a compile_unit
7ab123
> with the same stmt_value we'll call dwarf_decode_lines again,
7ab123
> and this time we need the pc/line mapping it computes.
7ab123
> Whereas if we process these in the opposite order we'll only call
7ab123
> dwarf_decode_lines once. I'm sure this will be confusing at first
7ab123
> to some later developer going through this code.
7ab123
> [I could be missing something of course, and I'm happy for any corrections.]
7ab123
7ab123
Implemented (omitting some story why I did not include it before).
7ab123
7ab123
7ab123
> The code that processes stmt_list for type_units is in setup_type_unit_groups.
7ab123
> Note that this code goes to the trouble of re-initializing the buildsym
7ab123
> machinery (see the calls to restart_symtab in dwarf2read.c) when we process
7ab123
> the second and subsequent type units that share a stmt_list value.
7ab123
> This is something that used to be done before your patch and will no
7ab123
> longer be done with your patch (since if we get a cache hit we exit).
7ab123
> It may be that the type_unit support is doing this unnecessarily,
7ab123
> which would be great because we can then simplify it.
7ab123
7ab123
I hope this patch should no longer break -fdebug-types-section.
7ab123
If it additionally enables some future optimization for -fdebug-types-section
7ab123
the better.
7ab123
7ab123
7ab123
>  > +  /* Offset of line number information in .debug_line section.  */
7ab123
>  > +  sect_offset offset;
7ab123
>  > +  unsigned offset_in_dwz : 1;
7ab123
> 
7ab123
> IWBN to document why offset_in_dwz is here.
7ab123
> It's not obvious why it's needed.
7ab123
+
7ab123
On Thu, 02 Oct 2014 01:57:03 +0200, Doug Evans wrote:
7ab123
> Ah, I guess the offset_in_dwz flag will ensure dwarf_decode_lines gets called
7ab123
> twice regardless of order.  But is that the only reason for the flag?
7ab123
7ab123
I have added there now:
7ab123
+  /* OFFSET is for struct dwz_file associated with dwarf2_per_objfile.  */
7ab123
7ab123
If one removes it regressions really happen.  What happens is that this
7ab123
line_header_hash (former lineinfo_hash) is in struct dwarf2_per_objfile which
7ab123
is common for both objfile and its objfile.dwz (that one is normally in
7ab123
/usr/lib/debug/.dwz/ - common for multiple objfiles).  And there are two
7ab123
different DIEs at offset 0xb - one in objfile and one in objfile.dwz - which
7ab123
would match single line_header if offset_in_dwz was not there.
7ab123
7ab123
Also existing dwarf2read.c code usually transfers "dwz flag" together with DIE
7ab123
offset, such as:
7ab123
	dwarf2_find_containing_comp_unit (sect_offset offset,
7ab123
					  unsigned int offset_in_dwz,
7ab123
					  struct objfile *objfile)
7ab123
This reminds me - why doesn't similar ambiguity happen also for dwp_file?
7ab123
I am unfortunately not much aware of the dwp implementation details.
7ab123
7ab123
7ab123
>  > -      struct line_header *line_header
7ab123
>  > -	= dwarf_decode_line_header (line_offset, cu);
7ab123
>  > +      dwarf2_per_objfile->lineinfo_hash =
7ab123
> 
7ab123
> As much as I prefer "=" going here, convention says to put it on the
7ab123
> next line.
7ab123
7ab123
I have changed it but this was just blind copy from existing line 21818.
7ab123
7ab123
7ab123
>  > +	htab_create_alloc_ex (127, dwarf2_lineinfo_hash, dwarf2_lineinfo_eq,
7ab123
> 
7ab123
> I don't have any data, but 127 seems high.
7ab123
7ab123
I have not changed it but this was just blind copy from existing line 21818.
7ab123
7ab123
7ab123
> I wouldn't change it, I just wanted to ask if you have any data
7ab123
> guiding this choice.
7ab123
7ab123
Tuning some constants really makes no sense when GDB has missing + insanely
7ab123
complicated data structures and in consequence GDB is using inappropriate data
7ab123
structures with bad algorithmic complexity.  One needs to switch GDB to C++
7ab123
and its STL before one can start talking about data structures performance.
7ab123
7ab123
7ab123
No regressions on {x86_64,x86_64-m32,i686}-fedora20-linux-gnu in DWZ mode and
7ab123
in -fdebug-types-section mode.
7ab123
7ab123
7ab123
Thanks,
7ab123
Jan
7ab123
7ab123
--cNdxnHkX5QqsyA0e
7ab123
Content-Type: text/plain; charset=us-ascii
7ab123
Content-Disposition: inline; filename="partialunit5.patch"
7ab123
7ab123
gdb/
7ab123
2014-10-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
7ab123
7ab123
	Fix 100x slowdown regression on DWZ files.
7ab123
	* dwarf2read.c (struct dwarf2_per_objfile): Add line_header_hash.
7ab123
	(struct line_header): Add offset and offset_in_dwz.
7ab123
	(dwarf_decode_lines): Add parameter decode_mapping to the declaration.
7ab123
	(free_line_header_voidp): New declaration.
7ab123
	(line_header_hash, line_header_eq): New functions.
7ab123
	(dwarf2_build_include_psymtabs): Update dwarf_decode_lines caller.
7ab123
	(handle_DW_AT_stmt_list): Use dwarf2_per_objfile->line_header_hash.
7ab123
	(free_line_header_voidp): New function.
7ab123
	(dwarf_decode_line_header): Initialize offset and offset_in_dwz.
7ab123
	(dwarf_decode_lines): New parameter decode_mapping, use it.
7ab123
7ab123
Index: gdb-7.6.1/gdb/dwarf2read.c
7ab123
===================================================================
7ab123
--- gdb-7.6.1.orig/gdb/dwarf2read.c
7ab123
+++ gdb-7.6.1/gdb/dwarf2read.c
7ab123
@@ -272,6 +272,9 @@ struct dwarf2_per_objfile
7ab123
 
7ab123
   /* The CUs we recently read.  */
7ab123
   VEC (dwarf2_per_cu_ptr) *just_read_cus;
7ab123
+
7ab123
+  /* Table containing line_header indexed by offset and offset_in_dwz.  */
7ab123
+  htab_t line_header_hash;
7ab123
 };
7ab123
 
7ab123
 static struct dwarf2_per_objfile *dwarf2_per_objfile;
7ab123
@@ -861,6 +864,12 @@ typedef void (die_reader_func_ftype) (co
7ab123
    which contains the following information.  */
7ab123
 struct line_header
7ab123
 {
7ab123
+  /* Offset of line number information in .debug_line section.  */
7ab123
+  sect_offset offset;
7ab123
+
7ab123
+  /* OFFSET is for struct dwz_file associated with dwarf2_per_objfile.  */
7ab123
+  unsigned offset_in_dwz : 1;
7ab123
+
7ab123
   unsigned int total_length;
7ab123
   unsigned short version;
7ab123
   unsigned int header_length;
7ab123
@@ -1413,7 +1422,7 @@ static struct line_header *dwarf_decode_
7ab123
 
7ab123
 static void dwarf_decode_lines (struct line_header *, const char *,
7ab123
 				struct dwarf2_cu *, struct partial_symtab *,
7ab123
-				int);
7ab123
+				int, int decode_mapping);
7ab123
 
7ab123
 static void dwarf2_start_subfile (char *, const char *, const char *);
7ab123
 
7ab123
@@ -1746,6 +1755,8 @@ static void process_cu_includes (void);
7ab123
 
7ab123
 static void check_producer (struct dwarf2_cu *cu);
7ab123
 
7ab123
+static void free_line_header_voidp (void *arg);
7ab123
+
7ab123
 #if WORDS_BIGENDIAN
7ab123
 
7ab123
 /* Convert VALUE between big- and little-endian.  */
7ab123
@@ -2141,6 +2152,29 @@ dwarf2_get_dwz_file (void)
7ab123
   dwarf2_per_objfile->dwz_file = result;
7ab123
   return result;
7ab123
 }
7ab123
+
7ab123
+/* Hash function for line_header_hash.  */
7ab123
+
7ab123
+static hashval_t
7ab123
+line_header_hash (const void *item)
7ab123
+{
7ab123
+  const struct line_header *ofs = item;
7ab123
+
7ab123
+  return ofs->offset.sect_off ^ ofs->offset_in_dwz;
7ab123
+}
7ab123
+
7ab123
+/* Equality function for line_header_hash.  */
7ab123
+
7ab123
+static int
7ab123
+line_header_eq (const void *item_lhs, const void *item_rhs)
7ab123
+{
7ab123
+  const struct line_header *ofs_lhs = item_lhs;
7ab123
+  const struct line_header *ofs_rhs = item_rhs;
7ab123
+
7ab123
+  return (ofs_lhs->offset.sect_off == ofs_rhs->offset.sect_off
7ab123
+	  && ofs_lhs->offset_in_dwz == ofs_rhs->offset_in_dwz);
7ab123
+}
7ab123
+
7ab123
 
7ab123
 /* DWARF quick_symbols_functions support.  */
7ab123
 
7ab123
@@ -4139,7 +4173,7 @@ dwarf2_build_include_psymtabs (struct dw
7ab123
     return;  /* No linetable, so no includes.  */
7ab123
 
7ab123
   /* NOTE: pst->dirname is DW_AT_comp_dir (if present).  */
7ab123
-  dwarf_decode_lines (lh, pst->dirname, cu, pst, 1);
7ab123
+  dwarf_decode_lines (lh, pst->dirname, cu, pst, 1, 1);
7ab123
 
7ab123
   free_line_header (lh);
7ab123
 }
7ab123
@@ -7969,24 +8003,64 @@ static void
7ab123
 handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
7ab123
 			const char *comp_dir)
7ab123
 {
7ab123
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
7ab123
   struct attribute *attr;
7ab123
+  unsigned int line_offset;
7ab123
+  struct line_header *line_header, line_header_local;
7ab123
+  unsigned u;
7ab123
+  void **slot;
7ab123
+  int decode_mapping;
7ab123
 
7ab123
   gdb_assert (! cu->per_cu->is_debug_types);
7ab123
 
7ab123
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
7ab123
-  if (attr)
7ab123
+  if (attr == NULL)
7ab123
+    return;
7ab123
+
7ab123
+  line_offset = DW_UNSND (attr);
7ab123
+
7ab123
+  if (dwarf2_per_objfile->line_header_hash == NULL)
7ab123
     {
7ab123
-      unsigned int line_offset = DW_UNSND (attr);
7ab123
-      struct line_header *line_header
7ab123
-	= dwarf_decode_line_header (line_offset, cu);
7ab123
-
7ab123
-      if (line_header)
7ab123
-	{
7ab123
-	  cu->line_header = line_header;
7ab123
-	  make_cleanup (free_cu_line_header, cu);
7ab123
-	  dwarf_decode_lines (line_header, comp_dir, cu, NULL, 1);
7ab123
-	}
7ab123
+      dwarf2_per_objfile->line_header_hash
7ab123
+	= htab_create_alloc_ex (127, line_header_hash, line_header_eq,
7ab123
+				free_line_header_voidp,
7ab123
+				&objfile->objfile_obstack,
7ab123
+				hashtab_obstack_allocate,
7ab123
+				dummy_obstack_deallocate);
7ab123
+    }
7ab123
+
7ab123
+  line_header_local.offset.sect_off = line_offset;
7ab123
+  line_header_local.offset_in_dwz = cu->per_cu->is_dwz;
7ab123
+  slot = htab_find_slot (dwarf2_per_objfile->line_header_hash,
7ab123
+			 &line_header_local, NO_INSERT);
7ab123
+
7ab123
+  /* For DW_TAG_compile_unit we need info like symtab::linetable which
7ab123
+     is not present in *SLOT.  */
7ab123
+  if (die->tag == DW_TAG_partial_unit && slot != NULL)
7ab123
+    {
7ab123
+      gdb_assert (*slot != NULL);
7ab123
+      cu->line_header = *slot;
7ab123
+      return;
7ab123
+    }
7ab123
+
7ab123
+  line_header = dwarf_decode_line_header (line_offset, cu);
7ab123
+  if (line_header == NULL)
7ab123
+    return;
7ab123
+  cu->line_header = line_header;
7ab123
+
7ab123
+  slot = htab_find_slot (dwarf2_per_objfile->line_header_hash,
7ab123
+			 &line_header_local, INSERT);
7ab123
+  gdb_assert (slot != NULL);
7ab123
+  if (*slot == NULL)
7ab123
+    *slot = line_header;
7ab123
+  else
7ab123
+    {
7ab123
+      gdb_assert (die->tag != DW_TAG_partial_unit);
7ab123
+      make_cleanup (free_cu_line_header, cu);
7ab123
     }
7ab123
+  decode_mapping = (die->tag != DW_TAG_partial_unit);
7ab123
+  dwarf_decode_lines (line_header, comp_dir, cu, NULL, 1,
7ab123
+		      decode_mapping);
7ab123
 }
7ab123
 
7ab123
 /* Process DW_TAG_compile_unit or DW_TAG_partial_unit.  */
7ab123
@@ -15203,6 +15277,16 @@ free_line_header (struct line_header *lh
7ab123
   xfree (lh);
7ab123
 }
7ab123
 
7ab123
+/* Stub for free_line_header to match void * callback types.  */
7ab123
+
7ab123
+static void
7ab123
+free_line_header_voidp (void *arg)
7ab123
+{
7ab123
+  struct line_header *lh = arg;
7ab123
+
7ab123
+  free_line_header (lh);
7ab123
+}
7ab123
+
7ab123
 /* Add an entry to LH's include directory table.  */
7ab123
 
7ab123
 static void
7ab123
@@ -15333,6 +15417,9 @@ dwarf_decode_line_header (unsigned int o
7ab123
   back_to = make_cleanup ((make_cleanup_ftype *) free_line_header,
7ab123
                           (void *) lh);
7ab123
 
7ab123
+  lh->offset.sect_off = offset;
7ab123
+  lh->offset_in_dwz = cu->per_cu->is_dwz;
7ab123
+
7ab123
   line_ptr = section->buffer + offset;
7ab123
 
7ab123
   /* Read in the header.  */
7ab123
@@ -15826,18 +15913,22 @@ dwarf_decode_lines_1 (struct line_header
7ab123
    as the corresponding symtab.  Since COMP_DIR is not used in the name of the
7ab123
    symtab we don't use it in the name of the psymtabs we create.
7ab123
    E.g. expand_line_sal requires this when finding psymtabs to expand.
7ab123
-   A good testcase for this is mb-inline.exp.  */
7ab123
+   A good testcase for this is mb-inline.exp.
7ab123
+
7ab123
+   Boolean DECODE_MAPPING specifies we need to fully decode .debug_line
7ab123
+   for its PC<->lines mapping information.  Otherwise only filenames
7ab123
+   tables is read in.  */
7ab123
 
7ab123
 static void
7ab123
 dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
7ab123
 		    struct dwarf2_cu *cu, struct partial_symtab *pst,
7ab123
-		    int want_line_info)
7ab123
+		    int want_line_info, int decode_mapping)
7ab123
 {
7ab123
   struct objfile *objfile = cu->objfile;
7ab123
   const int decode_for_pst_p = (pst != NULL);
7ab123
   struct subfile *first_subfile = current_subfile;
7ab123
 
7ab123
-  if (want_line_info)
7ab123
+  if (want_line_info && decode_mapping)
7ab123
     dwarf_decode_lines_1 (lh, comp_dir, cu, pst);
7ab123
 
7ab123
   if (decode_for_pst_p)