Fix 100x slowdown regression on DWZ files
authorJan Kratochvil <jan.kratochvil@redhat.com>
Sat, 24 Jan 2015 14:44:52 +0000 (15:44 +0100)
committerJan Kratochvil <jan.kratochvil@redhat.com>
Sat, 24 Jan 2015 14:44:52 +0000 (15:44 +0100)
Since Fedora started to use DWZ DWARF compressor:
http://fedoraproject.org/wiki/Features/DwarfCompressor
GDB has slowed down a lot.  To make it clear - DWZ is DWARF structure
rearrangement, "compressor" does not mean any zlib style data compression.

This patch reduces LibreOffice backtrace from 5 minutes to 3 seconds (100x)
and it also reduces memory consumption 20x.
[ benchmark is at the bottom of this mail ]

Example of DWZ output:
------------------------------------------------------------------------------
  Compilation Unit @ offset 0xc4:
 <0><cf>: Abbrev Number: 17 (DW_TAG_partial_unit)
    <d0>   DW_AT_stmt_list   : 0x0
    <d4>   DW_AT_comp_dir    : (indirect string, offset: 0x6f): /usr/src/debug/gdb-7.7.1/build-x86_64-redhat-linux-gnu/gdb
 <1><d8>: Abbrev Number: 9 (DW_TAG_typedef)
    <d9>   DW_AT_name        : (indirect string, offset: 0x827dc): size_t
    <dd>   DW_AT_decl_file   : 4
    <de>   DW_AT_decl_line   : 212
    <df>   DW_AT_type        : <0xae>

  Compilation Unit @ offset 0xe4:
 <0><ef>: Abbrev Number: 13 (DW_TAG_partial_unit)
    <f0>   DW_AT_stmt_list   : 0x0
    <f4>   DW_AT_comp_dir    : (indirect string, offset: 0x6f): /usr/src/debug/gdb-7.7.1/build-x86_64-redhat-linux-gnu/gdb
 <1><f8>: Abbrev Number: 45 (DW_TAG_typedef)
    <f9>   DW_AT_name        : (indirect string, offset: 0x251): __off_t
    <fd>   DW_AT_decl_file   : 3
    <fe>   DW_AT_decl_line   : 131
    <ff>   DW_AT_type        : <0x68>

  Compilation Unit @ offset 0x62d9f9:
 <0><62da04>: Abbrev Number: 20 (DW_TAG_compile_unit)
[...]
    <62da12>   DW_AT_low_pc  : 0x807e10
    <62da1a>   DW_AT_high_pc     : 134
    <62da1c>   DW_AT_stmt_list   : 0xf557e
 <1><62da20>: Abbrev Number: 7 (DW_TAG_imported_unit)
    <62da21>   DW_AT_import  : <0xcf> [Abbrev Number: 17]
------------------------------------------------------------------------------

One can see all DW_TAG_partial_unit have DW_AT_stmt_list 0x0 which causes
repeated decoding of that .debug_line unit on each DW_TAG_imported_unit.

This was OK before as each DW_TAG_compile_unit has its own .debug_line unit.
But since the introduction of DW_TAG_partial_unit by DWZ one should cache
read-in DW_AT_stmt_list .debug_line units.

Fortunately one does not need to cache whole
        struct linetable *symtab->linetable
and other data from .debug_line mapping PC<->lines
------------------------------------------------------------------------------
 Line Number Statements:
  Extended opcode 2: set Address to 0x45c880
  Advance Line by 25 to 26
  Copy
------------------------------------------------------------------------------
as the only part of .debug_line which GDB needs for DW_TAG_partial_unit is:
------------------------------------------------------------------------------
 The Directory Table:
  ../../gdb
  /usr/include/bits
[...]
 The File Name Table:
  Entry Dir     Time    Size    Name
  1     1 0 0 gdb.c
  2     2 0 0 string3.h
[...]
------------------------------------------------------------------------------
specifically referenced in GDB for DW_AT_decl_file at a single place:
------------------------------------------------------------------------------
              fe = &cu->line_header->file_names[file_index - 1];
              SYMBOL_SYMTAB (sym) = fe->symtab;
------------------------------------------------------------------------------

This is because for some reason DW_TAG_partial_unit never contains PC-related
DWARF information.  I do not know exactly why, the compression ratio is a bit
lower due to it but thanksfully currently it is that way:
dwz.c:
------------------------------------------------------------------------------
        /* These attributes reference code, prevent moving
           DIEs with them.  */
        case DW_AT_low_pc:
        case DW_AT_high_pc:
        case DW_AT_entry_pc:
        case DW_AT_ranges:
          die->die_ck_state = CK_BAD;
+
  /* State of checksum computation.  Not computed yet, computed and
     suitable for moving into partial units, currently being computed
     and finally determined unsuitable for moving into partial units.  */
  enum { CK_UNKNOWN, CK_KNOWN, CK_BEING_COMPUTED, CK_BAD } die_ck_state : 2;
------------------------------------------------------------------------------
I have also verified also real-world Fedora debuginfo files really comply with
that assumption with dwgrep
https://github.com/pmachata/dwgrep
using:
------------------------------------------------------------------------------
dwgrep -e 'entry ?DW_TAG_partial_unit child* ( ?DW_AT_low_pc , ?DW_AT_high_pc , ?DW_AT_ranges )' /usr/lib/debug/**
------------------------------------------------------------------------------

BTW I think GDB already does not support the whole DW_TAG_imported_unit and
DW_TAG_partial_unit usage possibilities as specified by the DWARF standard.
I think GDB would not work if DW_TAG_imported_unit was used in some inner
level and not at the CU level (readelf -wi level <1>) - this is how DWZ is
using DW_TAG_imported_unit.  Therefore I do not think further assumptions
about DW_TAG_imported_unit and DW_TAG_partial_unit usage by DWZ are a problem
for GDB.

One could save the whole .debug_line decoded PC<->lines mapping (and not just
the DW_AT_decl_file table) but:
 * there are some problematic corner cases so one could do it incorrectly
 * there are no real world data to really test such patch extension
 * such extension could be done perfectly incrementally on top of this patch

------------------------------------------------------------------------------

benchmark - on Fedora 20 x86_64 and FSF GDB HEAD:
echo -e 'thread apply all bt\nset confirm no\nq'|./gdb -p `pidof soffice.bin` -ex 'set pagination off' -ex 'maintenance set per-command
space' -ex 'maintenance set per-command symtab' -ex 'maintenance set per-command time'

FSF GDB HEAD ("thread apply all bt"):
Command execution time: 333.693000 (cpu), 335.587539 (wall)
                                          ---sec
Space used: 1736404992 (+1477189632 for this command)
                         ----MB
vs.
THIS PATCH ("thread apply all bt"):
Command execution time: 2.595000 (cpu), 2.607573 (wall)
                                        -sec
Space used: 340058112 (+85917696 for this command)
                        --MB

FSF GDB HEAD ("thread apply all bt full"):
Command execution time: 466.751000 (cpu), 468.345837 (wall)
                                          ---sec
Space used: 2330132480 (+2070974464 for this command)
                         ----MB
vs.
THIS PATCH ("thread apply all bt full"):
Command execution time: 18.907000 (cpu), 18.964125 (wall)
                                         --sec
Space used: 364462080 (+110325760 for this command)
                        ---MB

------------------------------------------------------------------------------

gdb/ChangeLog
2015-01-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

Fix 100x slowdown regression on DWZ files.
* dwarf2read.c (struct dwarf2_per_objfile): Add line_header_hash.
(struct line_header): Add offset and offset_in_dwz.
(dwarf_decode_lines): Add parameter decode_mapping to the declaration.
(free_line_header_voidp): New declaration.
(line_header_hash, line_header_hash_voidp, line_header_eq_voidp): New
functions.
(dwarf2_build_include_psymtabs): Update dwarf_decode_lines caller.
(handle_DW_AT_stmt_list): Use line_header_hash.
(free_line_header_voidp): New function.
(dwarf_decode_line_header): Initialize offset and offset_in_dwz.
(dwarf_decode_lines): New parameter decode_mapping, use it.
(dwarf2_free_objfile): Free line_header_hash.

gdb/ChangeLog
gdb/dwarf2read.c

index 10c3855d2459592a66b8d9e98f5daa218abd39c1..2e7e55945ffa9a1187cd25cd50e9c82b948f4c20 100644 (file)
@@ -1,3 +1,19 @@
+2015-01-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+       Fix 100x slowdown regression on DWZ files.
+       * dwarf2read.c (struct dwarf2_per_objfile): Add line_header_hash.
+       (struct line_header): Add offset and offset_in_dwz.
+       (dwarf_decode_lines): Add parameter decode_mapping to the declaration.
+       (free_line_header_voidp): New declaration.
+       (line_header_hash, line_header_hash_voidp, line_header_eq_voidp): New
+       functions.
+       (dwarf2_build_include_psymtabs): Update dwarf_decode_lines caller.
+       (handle_DW_AT_stmt_list): Use line_header_hash.
+       (free_line_header_voidp): New function.
+       (dwarf_decode_line_header): Initialize offset and offset_in_dwz.
+       (dwarf_decode_lines): New parameter decode_mapping, use it.
+       (dwarf2_free_objfile): Free line_header_hash.
+
 2015-01-23  Simon Marchi  <simon.marchi@ericsson.com>
 
        PR gdb/17416
index 9d765c5091ddbffad74302b2bcc80b1a9c287e97..715b090abc565a469afb236f5360f9db57041740 100644 (file)
@@ -308,6 +308,9 @@ struct dwarf2_per_objfile
 
   /* The CUs we recently read.  */
   VEC (dwarf2_per_cu_ptr) *just_read_cus;
+
+  /* Table containing line_header indexed by offset and offset_in_dwz.  */
+  htab_t line_header_hash;
 };
 
 static struct dwarf2_per_objfile *dwarf2_per_objfile;
@@ -1024,6 +1027,12 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
    which contains the following information.  */
 struct line_header
 {
+  /* Offset of line number information in .debug_line section.  */
+  sect_offset offset;
+
+  /* OFFSET is for struct dwz_file associated with dwarf2_per_objfile.  */
+  unsigned offset_in_dwz : 1;
+
   unsigned int total_length;
   unsigned short version;
   unsigned int header_length;
@@ -1512,7 +1521,7 @@ static struct line_header *dwarf_decode_line_header (unsigned int offset,
 
 static void dwarf_decode_lines (struct line_header *, const char *,
                                struct dwarf2_cu *, struct partial_symtab *,
-                               CORE_ADDR);
+                               CORE_ADDR, int decode_mapping);
 
 static void dwarf2_start_subfile (const char *, const char *);
 
@@ -1844,6 +1853,8 @@ static void free_dwo_file_cleanup (void *);
 static void process_cu_includes (void);
 
 static void check_producer (struct dwarf2_cu *cu);
+
+static void free_line_header_voidp (void *arg);
 \f
 /* Various complaints about symbol reading that don't abort the process.  */
 
@@ -1910,6 +1921,37 @@ dwarf2_invalid_attrib_class_complaint (const char *arg1, const char *arg2)
             _("invalid attribute class or form for '%s' in '%s'"),
             arg1, arg2);
 }
+
+/* Hash function for line_header_hash.  */
+
+static hashval_t
+line_header_hash (const struct line_header *ofs)
+{
+  return ofs->offset.sect_off ^ ofs->offset_in_dwz;
+}
+
+/* Hash function for htab_create_alloc_ex for line_header_hash.  */
+
+static hashval_t
+line_header_hash_voidp (const void *item)
+{
+  const struct line_header *ofs = item;
+
+  return line_header_hash (ofs);
+}
+
+/* Equality function for line_header_hash.  */
+
+static int
+line_header_eq_voidp (const void *item_lhs, const void *item_rhs)
+{
+  const struct line_header *ofs_lhs = item_lhs;
+  const struct line_header *ofs_rhs = item_rhs;
+
+  return (ofs_lhs->offset.sect_off == ofs_rhs->offset.sect_off
+         && ofs_lhs->offset_in_dwz == ofs_rhs->offset_in_dwz);
+}
+
 \f
 #if WORDS_BIGENDIAN
 
@@ -4452,7 +4494,7 @@ dwarf2_build_include_psymtabs (struct dwarf2_cu *cu,
     return;  /* No linetable, so no includes.  */
 
   /* NOTE: pst->dirname is DW_AT_comp_dir (if present).  */
-  dwarf_decode_lines (lh, pst->dirname, cu, pst, pst->textlow);
+  dwarf_decode_lines (lh, pst->dirname, cu, pst, pst->textlow, 1);
 
   free_line_header (lh);
 }
@@ -8995,24 +9037,95 @@ static void
 handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
                        const char *comp_dir, CORE_ADDR lowpc) /* ARI: editCase function */
 {
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct attribute *attr;
+  unsigned int line_offset;
+  struct line_header line_header_local;
+  hashval_t line_header_local_hash;
+  unsigned u;
+  void **slot;
+  int decode_mapping;
 
   gdb_assert (! cu->per_cu->is_debug_types);
 
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
-  if (attr)
+  if (attr == NULL)
+    return;
+
+  line_offset = DW_UNSND (attr);
+
+  /* The line header hash table is only created if needed (it exists to
+     prevent redundant reading of the line table for partial_units).
+     If we're given a partial_unit, we'll need it.  If we're given a
+     compile_unit, then use the line header hash table if it's already
+     created, but don't create one just yet.  */
+
+  if (dwarf2_per_objfile->line_header_hash == NULL
+      && die->tag == DW_TAG_partial_unit)
+    {
+      dwarf2_per_objfile->line_header_hash
+       = htab_create_alloc_ex (127, line_header_hash_voidp,
+                               line_header_eq_voidp,
+                               free_line_header_voidp,
+                               &objfile->objfile_obstack,
+                               hashtab_obstack_allocate,
+                               dummy_obstack_deallocate);
+    }
+
+  line_header_local.offset.sect_off = line_offset;
+  line_header_local.offset_in_dwz = cu->per_cu->is_dwz;
+  line_header_local_hash = line_header_hash (&line_header_local);
+  if (dwarf2_per_objfile->line_header_hash != NULL)
     {
-      unsigned int line_offset = DW_UNSND (attr);
-      struct line_header *line_header
-       = dwarf_decode_line_header (line_offset, cu);
+      slot = htab_find_slot_with_hash (dwarf2_per_objfile->line_header_hash,
+                                      &line_header_local,
+                                      line_header_local_hash, NO_INSERT);
 
-      if (line_header)
+      /* For DW_TAG_compile_unit we need info like symtab::linetable which
+        is not present in *SLOT (since if there is something in *SLOT then
+        it will be for a partial_unit).  */
+      if (die->tag == DW_TAG_partial_unit && slot != NULL)
        {
-         cu->line_header = line_header;
-         make_cleanup (free_cu_line_header, cu);
-         dwarf_decode_lines (line_header, comp_dir, cu, NULL, lowpc);
+         gdb_assert (*slot != NULL);
+         cu->line_header = *slot;
+         return;
        }
     }
+
+  /* dwarf_decode_line_header does not yet provide sufficient information.
+     We always have to call also dwarf_decode_lines for it.  */
+  cu->line_header = dwarf_decode_line_header (line_offset, cu);
+  if (cu->line_header == NULL)
+    return;
+
+  if (dwarf2_per_objfile->line_header_hash == NULL)
+    slot = NULL;
+  else
+    {
+      slot = htab_find_slot_with_hash (dwarf2_per_objfile->line_header_hash,
+                                      &line_header_local,
+                                      line_header_local_hash, INSERT);
+      gdb_assert (slot != NULL);
+    }
+  if (slot != NULL && *slot == NULL)
+    {
+      /* This newly decoded line number information unit will be owned
+        by line_header_hash hash table.  */
+      *slot = cu->line_header;
+    }
+  else
+    {
+      /* We cannot free any current entry in (*slot) as that struct line_header
+         may be already used by multiple CUs.  Create only temporary decoded
+        line_header for this CU - it may happen at most once for each line
+        number information unit.  And if we're not using line_header_hash
+        then this is what we want as well.  */
+      gdb_assert (die->tag != DW_TAG_partial_unit);
+      make_cleanup (free_cu_line_header, cu);
+    }
+  decode_mapping = (die->tag != DW_TAG_partial_unit);
+  dwarf_decode_lines (cu->line_header, comp_dir, cu, NULL, lowpc,
+                     decode_mapping);
 }
 
 /* Process DW_TAG_compile_unit or DW_TAG_partial_unit.  */
@@ -16915,6 +17028,16 @@ free_line_header (struct line_header *lh)
   xfree (lh);
 }
 
+/* Stub for free_line_header to match void * callback types.  */
+
+static void
+free_line_header_voidp (void *arg)
+{
+  struct line_header *lh = arg;
+
+  free_line_header (lh);
+}
+
 /* Add an entry to LH's include directory table.  */
 
 static void
@@ -17045,6 +17168,9 @@ dwarf_decode_line_header (unsigned int offset, struct dwarf2_cu *cu)
   back_to = make_cleanup ((make_cleanup_ftype *) free_line_header,
                           (void *) lh);
 
+  lh->offset.sect_off = offset;
+  lh->offset_in_dwz = cu->per_cu->is_dwz;
+
   line_ptr = section->buffer + offset;
 
   /* Read in the header.  */
@@ -17672,17 +17798,22 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
    E.g. expand_line_sal requires this when finding psymtabs to expand.
    A good testcase for this is mb-inline.exp.
 
-   LOWPC is the lowest address in CU (or 0 if not known).  */
+   LOWPC is the lowest address in CU (or 0 if not known).
+
+   Boolean DECODE_MAPPING specifies we need to fully decode .debug_line
+   for its PC<->lines mapping information.  Otherwise only the filename
+   table is read in.  */
 
 static void
 dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
                    struct dwarf2_cu *cu, struct partial_symtab *pst,
-                   CORE_ADDR lowpc)
+                   CORE_ADDR lowpc, int decode_mapping)
 {
   struct objfile *objfile = cu->objfile;
   const int decode_for_pst_p = (pst != NULL);
 
-  dwarf_decode_lines_1 (lh, cu, decode_for_pst_p, lowpc);
+  if (decode_mapping)
+    dwarf_decode_lines_1 (lh, cu, decode_for_pst_p, lowpc);
 
   if (decode_for_pst_p)
     {
@@ -21779,6 +21910,9 @@ dwarf2_free_objfile (struct objfile *objfile)
   if (dwarf2_per_objfile->quick_file_names_table)
     htab_delete (dwarf2_per_objfile->quick_file_names_table);
 
+  if (dwarf2_per_objfile->line_header_hash)
+    htab_delete (dwarf2_per_objfile->line_header_hash);
+
   /* Everything else should be on the objfile obstack.  */
 }