From e3bcba8731632036e309f248be4f64ef8aabd052 Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Mon, 6 Oct 2014 15:01:39 -0400 Subject: [PATCH] Fix 'Slow gstack performance' (RH BZ 1103894, Jan Kratochvil). --- gdb-slow-gstack-performance.patch | 349 ++++++++++++++++++++++++++++++ gdb.spec | 9 +- 2 files changed, 357 insertions(+), 1 deletion(-) create mode 100644 gdb-slow-gstack-performance.patch diff --git a/gdb-slow-gstack-performance.patch b/gdb-slow-gstack-performance.patch new file mode 100644 index 0000000..6a93988 --- /dev/null +++ b/gdb-slow-gstack-performance.patch @@ -0,0 +1,349 @@ +Date: Thu, 2 Oct 2014 17:56:53 +0200 +From: Jan Kratochvil +To: Doug Evans +Cc: gdb-patches at sourceware dot org +Subject: [patchv2] Fix 100x slowdown regression on DWZ files +Message-ID: <20141002155653.GA9001@host2.jankratochvil.net> + +--cNdxnHkX5QqsyA0e +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline + +On Thu, 02 Oct 2014 01:51:38 +0200, Doug Evans wrote: +> I tested this patch with --target_board=dwarf4-gdb-index +> and got a failure in m-static.exp: + +That is particularly with -fdebug-types-section. + + +> Type units read the line table in a separate path, + +OK, therefore I dropped that separate struct dwarf2_lineinfo +and reused struct line_header instead. + + +> OTOH, I do want to avoid any confusion that this patch may inadvertently +> introduce. For example, IIUC with your patch as is, +> if we read a partial_unit first, before a compile_unit +> that has the same stmt_list value, we'll do more processing in +> dwarf_decode_lines than we really need to since we only need a file +> number to symtab mapping. And if we later read in a compile_unit +> with the same stmt_value we'll call dwarf_decode_lines again, +> and this time we need the pc/line mapping it computes. +> Whereas if we process these in the opposite order we'll only call +> dwarf_decode_lines once. I'm sure this will be confusing at first +> to some later developer going through this code. +> [I could be missing something of course, and I'm happy for any corrections.] + +Implemented (omitting some story why I did not include it before). + + +> The code that processes stmt_list for type_units is in setup_type_unit_groups. +> Note that this code goes to the trouble of re-initializing the buildsym +> machinery (see the calls to restart_symtab in dwarf2read.c) when we process +> the second and subsequent type units that share a stmt_list value. +> This is something that used to be done before your patch and will no +> longer be done with your patch (since if we get a cache hit we exit). +> It may be that the type_unit support is doing this unnecessarily, +> which would be great because we can then simplify it. + +I hope this patch should no longer break -fdebug-types-section. +If it additionally enables some future optimization for -fdebug-types-section +the better. + + +> > + /* Offset of line number information in .debug_line section. */ +> > + sect_offset offset; +> > + unsigned offset_in_dwz : 1; +> +> IWBN to document why offset_in_dwz is here. +> It's not obvious why it's needed. ++ +On Thu, 02 Oct 2014 01:57:03 +0200, Doug Evans wrote: +> Ah, I guess the offset_in_dwz flag will ensure dwarf_decode_lines gets called +> twice regardless of order. But is that the only reason for the flag? + +I have added there now: ++ /* OFFSET is for struct dwz_file associated with dwarf2_per_objfile. */ + +If one removes it regressions really happen. What happens is that this +line_header_hash (former lineinfo_hash) is in struct dwarf2_per_objfile which +is common for both objfile and its objfile.dwz (that one is normally in +/usr/lib/debug/.dwz/ - common for multiple objfiles). And there are two +different DIEs at offset 0xb - one in objfile and one in objfile.dwz - which +would match single line_header if offset_in_dwz was not there. + +Also existing dwarf2read.c code usually transfers "dwz flag" together with DIE +offset, such as: + dwarf2_find_containing_comp_unit (sect_offset offset, + unsigned int offset_in_dwz, + struct objfile *objfile) +This reminds me - why doesn't similar ambiguity happen also for dwp_file? +I am unfortunately not much aware of the dwp implementation details. + + +> > - struct line_header *line_header +> > - = dwarf_decode_line_header (line_offset, cu); +> > + dwarf2_per_objfile->lineinfo_hash = +> +> As much as I prefer "=" going here, convention says to put it on the +> next line. + +I have changed it but this was just blind copy from existing line 21818. + + +> > + htab_create_alloc_ex (127, dwarf2_lineinfo_hash, dwarf2_lineinfo_eq, +> +> I don't have any data, but 127 seems high. + +I have not changed it but this was just blind copy from existing line 21818. + + +> I wouldn't change it, I just wanted to ask if you have any data +> guiding this choice. + +Tuning some constants really makes no sense when GDB has missing + insanely +complicated data structures and in consequence GDB is using inappropriate data +structures with bad algorithmic complexity. One needs to switch GDB to C++ +and its STL before one can start talking about data structures performance. + + +No regressions on {x86_64,x86_64-m32,i686}-fedora20-linux-gnu in DWZ mode and +in -fdebug-types-section mode. + + +Thanks, +Jan + +--cNdxnHkX5QqsyA0e +Content-Type: text/plain; charset=us-ascii +Content-Disposition: inline; filename="partialunit5.patch" + +gdb/ +2014-10-02 Jan Kratochvil + + 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_eq): New functions. + (dwarf2_build_include_psymtabs): Update dwarf_decode_lines caller. + (handle_DW_AT_stmt_list): Use dwarf2_per_objfile->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. + +Index: gdb-7.8/gdb/dwarf2read.c +=================================================================== +--- gdb-7.8.orig/gdb/dwarf2read.c ++++ gdb-7.8/gdb/dwarf2read.c +@@ -312,6 +312,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; +@@ -1028,6 +1031,12 @@ typedef void (die_reader_func_ftype) (co + 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; +@@ -1516,7 +1525,7 @@ static struct line_header *dwarf_decode_ + + static void dwarf_decode_lines (struct line_header *, const char *, + struct dwarf2_cu *, struct partial_symtab *, +- int); ++ int, int decode_mapping); + + static void dwarf2_start_subfile (const char *, const char *, const char *); + +@@ -1849,6 +1858,8 @@ static void process_cu_includes (void); + + static void check_producer (struct dwarf2_cu *cu); + ++static void free_line_header_voidp (void *arg); ++ + static int + attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die, + struct dwarf2_cu *cu, struct dynamic_prop *prop, +@@ -1920,6 +1931,29 @@ dwarf2_invalid_attrib_class_complaint (c + _("invalid attribute class or form for '%s' in '%s'"), + arg1, arg2); + } ++ ++/* Hash function for line_header_hash. */ ++ ++static hashval_t ++line_header_hash (const void *item) ++{ ++ const struct line_header *ofs = item; ++ ++ return ofs->offset.sect_off ^ ofs->offset_in_dwz; ++} ++ ++/* Equality function for line_header_hash. */ ++ ++static int ++line_header_eq (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); ++} ++ + + #if WORDS_BIGENDIAN + +@@ -4459,7 +4493,7 @@ dwarf2_build_include_psymtabs (struct dw + return; /* No linetable, so no includes. */ + + /* NOTE: pst->dirname is DW_AT_comp_dir (if present). */ +- dwarf_decode_lines (lh, pst->dirname, cu, pst, 1); ++ dwarf_decode_lines (lh, pst->dirname, cu, pst, 1, 1); + + free_line_header (lh); + } +@@ -8989,24 +9023,64 @@ static void + handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu, + const char *comp_dir) /* ARI: editCase function */ + { ++ struct objfile *objfile = dwarf2_per_objfile->objfile; + struct attribute *attr; ++ unsigned int line_offset; ++ struct line_header *line_header, line_header_local; ++ 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); ++ ++ 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); +- +- if (line_header) +- { +- cu->line_header = line_header; +- make_cleanup (free_cu_line_header, cu); +- dwarf_decode_lines (line_header, comp_dir, cu, NULL, 1); +- } ++ dwarf2_per_objfile->line_header_hash ++ = htab_create_alloc_ex (127, line_header_hash, line_header_eq, ++ 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; ++ slot = htab_find_slot (dwarf2_per_objfile->line_header_hash, ++ &line_header_local, NO_INSERT); ++ ++ /* For DW_TAG_compile_unit we need info like symtab::linetable which ++ is not present in *SLOT. */ ++ if (die->tag == DW_TAG_partial_unit && slot != NULL) ++ { ++ gdb_assert (*slot != NULL); ++ cu->line_header = *slot; ++ return; ++ } ++ ++ line_header = dwarf_decode_line_header (line_offset, cu); ++ if (line_header == NULL) ++ return; ++ cu->line_header = line_header; ++ ++ slot = htab_find_slot (dwarf2_per_objfile->line_header_hash, ++ &line_header_local, INSERT); ++ gdb_assert (slot != NULL); ++ if (*slot == NULL) ++ *slot = line_header; ++ else ++ { ++ 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 (line_header, comp_dir, cu, NULL, 1, ++ decode_mapping); + } + + /* Process DW_TAG_compile_unit or DW_TAG_partial_unit. */ +@@ -16969,6 +17043,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 +@@ -17099,6 +17183,9 @@ dwarf_decode_line_header (unsigned int o + 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. */ +@@ -17596,18 +17683,22 @@ dwarf_decode_lines_1 (struct line_header + as the corresponding symtab. Since COMP_DIR is not used in the name of the + symtab we don't use it in the name of the psymtabs we create. + E.g. expand_line_sal requires this when finding psymtabs to expand. +- A good testcase for this is mb-inline.exp. */ ++ A good testcase for this is mb-inline.exp. ++ ++ Boolean DECODE_MAPPING specifies we need to fully decode .debug_line ++ for its PC<->lines mapping information. Otherwise only filenames ++ tables is read in. */ + + static void + dwarf_decode_lines (struct line_header *lh, const char *comp_dir, + struct dwarf2_cu *cu, struct partial_symtab *pst, +- int want_line_info) ++ int want_line_info, int decode_mapping) + { + struct objfile *objfile = cu->objfile; + const int decode_for_pst_p = (pst != NULL); + struct subfile *first_subfile = current_subfile; + +- if (want_line_info) ++ if (want_line_info && decode_mapping) + dwarf_decode_lines_1 (lh, comp_dir, cu, pst); + + if (decode_for_pst_p) diff --git a/gdb.spec b/gdb.spec index 82924fb..420d167 100644 --- a/gdb.spec +++ b/gdb.spec @@ -26,7 +26,7 @@ Version: 7.8 # The release always contains a leading reserved number, start it at 1. # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing. -Release: 24%{?dist} +Release: 25%{?dist} License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL Group: Development/Debuggers @@ -544,6 +544,9 @@ Patch970: gdb-async-stopped-on-pid-arg-testsuite.patch # (BZ 1146170, Miroslav Franc). Patch971: gdb-save-breakpoints-fix.patch +# Fix 'Slow gstack performance' (RH BZ 1103894, Jan Kratochvil). +Patch973: gdb-slow-gstack-performance.patch + %if 0%{!?rhel:1} || 0%{?rhel} > 6 # RL_STATE_FEDORA_GDB would not be found for: # Patch642: gdb-readline62-ask-more-rh.patch @@ -835,6 +838,7 @@ find -name "*.info*"|xargs rm -f %patch931 -p1 %patch970 -p1 %patch971 -p1 +%patch973 -p1 %patch848 -p1 %if 0%{!?el6:1} @@ -1335,6 +1339,9 @@ then fi %changelog +* Fri Oct 03 2014 Sergio Durigan Junior - 7.8-25.fc21 +- Fix 'Slow gstack performance' (RH BZ 1103894, Jan Kratochvil). + * Fri Oct 3 2014 Jan Kratochvil - 7.8-24.fc21 - Fix "save breakpoints" for signal catchpoints and disabled breakpoints (BZ 1146170, Miroslav Franc).