From patchwork Wed Jan 14 20:26:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 4686 Received: (qmail 30036 invoked by alias); 14 Jan 2015 20:26:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 30025 invoked by uid 89); 14 Jan 2015 20:26:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.9 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 14 Jan 2015 20:26:28 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0EKQOBA002092 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 14 Jan 2015 15:26:25 -0500 Received: from host2.jankratochvil.net (ovpn-116-51.ams2.redhat.com [10.36.116.51]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0EKQIul024909 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Wed, 14 Jan 2015 15:26:20 -0500 Date: Wed, 14 Jan 2015 21:26:18 +0100 From: Jan Kratochvil To: Doug Evans Cc: gdb-patches@sourceware.org Subject: [patchv3] Fix 100x slowdown regression on DWZ files Message-ID: <20150114202618.GA21056@host2.jankratochvil.net> References: <21548.37770.274873.760290@ruffy2.mtv.corp.google.com> <20141002155653.GA9001@host2.jankratochvil.net> <20141231192335.GA8188@host2.jankratochvil.net> <21677.57646.178793.836948@ruffy2.mtv.corp.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <21677.57646.178793.836948@ruffy2.mtv.corp.google.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes Hi Doug, On Thu, 08 Jan 2015 02:45:18 +0100, Doug Evans wrote: > + 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); > > Do hashtables support calling htab_find_slot with INSERT but then > not assigning the slot a value if it was empty? > I could be wrong but I think it does. > If so, we can remove one call to htab_find_slot here. If dwarf_decode_line_header() fails we have nothing to put there. If we had done INSERT it is a problem as discussed in previous mail. > + /* 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, lowpc, > + decode_mapping); > > This is a bit confusing to follow. > If the slot was empty we save line_header in it (and don't record a cleanup), > but if wasn't empty we don't update *slot but do record a cleanup. > Presumably there's a reason, but it's hard to follow. I do not see how to make it differently. I have put there more comments. > It would be simpler to just free any previous entry and conditionally > update *slot. Is there a reason to not do that? > Can you add a clarifying comment? We cannot - comment with the reason is now in the code. > Plus I'm worried about increased memory usage in the non-dwz case. > IIUC, the non-dwz case will always have *slot == NULL, and thus we'll > always be saving line header entries we'll never need again. Those are few bytes for each expanded CU. The problem of GDB is that it needlessly expands thousands of CUs. Saving each byte of a decoded CU is not the right way how to fix the excessive memory consumption. But added there for it 'dwarf2_per_objfile->seen_partial_unit'. > Also, it looks like line_header_hash (and its entries) aren't > deleted when the objfile goes away. Missing call to htab_delete. Fixed. > A few comments inline. BTW I would prefer s/^/> / for the patch, the comments are difficult to find this way. No regressions on {x86_64,x86_64-m32,i686}-fedora22pre-linux-gnu in default mode, other modes still run but hopefully they will be OK. Thanks, Jan 2015-01-14 Jan Kratochvil Fix 100x slowdown regression on DWZ files. * dwarf2read.c (struct dwarf2_per_objfile): Add seen_partial_unit and 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 seen_partial_unit and 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. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 96b2537..fadc258 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -285,6 +285,9 @@ struct dwarf2_per_objfile or we are faking it for OBJF_READNOW's sake. */ unsigned char using_index; + /* True if we have already seen a DW_TAG_partial_unit. */ + unsigned char seen_partial_unit; + /* The mapped index, or NULL if .gdb_index is missing or not being used. */ struct mapped_index *index_table; @@ -308,6 +311,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 +1030,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 +1524,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 +1856,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); /* Various complaints about symbol reading that don't abort the process. */ @@ -1910,6 +1924,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); +} + #if WORDS_BIGENDIAN @@ -4452,7 +4497,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 +9040,90 @@ 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); + + if (die->tag == DW_TAG_partial_unit) + dwarf2_per_objfile->seen_partial_unit = 1; + + if (dwarf2_per_objfile->line_header_hash == NULL + && dwarf2_per_objfile->seen_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->seen_partial_unit) { - 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. */ + 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->seen_partial_unit) + 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_line_header (*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. */ + 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. */ @@ -16909,6 +17020,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 @@ -17039,6 +17160,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. */ @@ -17666,17 +17790,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) { @@ -21773,6 +21902,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. */ }