From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12471 invoked by alias); 29 Oct 2013 18:31:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 12452 invoked by uid 89); 29 Oct 2013 18:31:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS 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 ESMTP; Tue, 29 Oct 2013 18:31:09 +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 r9TIV6Dr022466 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 29 Oct 2013 14:31:07 -0400 Received: from barimba (ovpn-113-94.phx2.redhat.com [10.3.113.94]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r9TIV4op000832 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 29 Oct 2013 14:31:05 -0400 From: Tom Tromey To: Doug Evans Cc: gdb-patches Subject: Re: [PATCH v2 9/9] add short-circuit logic to elfread.c References: <1382032193-9115-1-git-send-email-tromey@redhat.com> <1382032193-9115-10-git-send-email-tromey@redhat.com> <21089.47667.349994.537640@ruffy.mtv.corp.google.com> <87vc0rn2hi.fsf@fleche.redhat.com> <877gd6jw68.fsf@fleche.redhat.com> Date: Tue, 29 Oct 2013 18:31:00 -0000 In-Reply-To: (Doug Evans's message of "Mon, 21 Oct 2013 12:52:18 -0700") Message-ID: <8761sg7wuv.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-10/txt/msg00911.txt.bz2 >>>>> "Doug" == Doug Evans writes: Doug> The comments in the code suggest, to me, the compaction is important. Doug> Well, how important? Doug> [I realize there are reasons for the compaction other than speed, Doug> but according to the comments speed is a real concern here.] Ok, I added it back. Here's the updated patch. Tom commit 73a88b75971038390729c2bf84d451af9086b4ba Author: Tom Tromey Date: Tue Oct 15 11:50:58 2013 -0600 add short-circuit logic to elfread.c If minimal symbols have already been read into a per-BFD object, then a symbol reader can skip re-reading them. This changes the ELF reader to do so. We only skip the work if the file is ELF+DWARF. If it has stabs or mdebug sections, then I think extra information is computed during the minsym creation pass; and so we must still repeat it. Eventually even this will go away, once all symbol types have switched to being progspace-independent. In the meantime this has no negative effect -- it is just a missing optimization for a small set of users. This change also required a somewhat non-obvious change to the OBJSTAT accounting code. If a symbol reader skips re-reading minimal symbols, then the corresponding OBJSTAT will not be updated. This leads to a test failure in gdb.base/maint.exp. To fix this, I've moved the needed stat field out of objfile and into the per-BFD object. 2013-10-29 Tom Tromey * elfread.c (elf_read_minimal_symbols): Return early if minimal symbols have already been read. Add "ei" parameter. (elf_symfile_read): Call elf_read_minimal_symbols earlier. * minsyms.c (prim_record_minimal_symbol_full): Update. * objfiles.h (struct objstats) : Move... (struct objfile_per_bfd_storage) : ... here. * symmisc.c (print_objfile_statistics): Update. diff --git a/gdb/elfread.c b/gdb/elfread.c index 5bf4598..5bb3ddf 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -1076,7 +1076,8 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b) symbols. */ static void -elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags) +elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags, + const struct elfinfo *ei) { bfd *synth_abfd, *abfd = objfile->obfd; struct cleanup *back_to; @@ -1092,6 +1093,21 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags) objfile_name (objfile)); } + /* If we already have minsyms, then we can skip some work here. + However, if there were stabs or mdebug sections, we go ahead and + redo all the work anyway, because the psym readers for those + kinds of debuginfo need extra information found here. This can + go away once all types of symbols are in the per-BFD object. */ + if (objfile->per_bfd->minsyms_read + && ei->stabsect == NULL + && ei->mdebugsect == NULL) + { + if (symtab_create_debug) + fprintf_unfiltered (gdb_stdlog, + "... minimal symbols previously read\n"); + return; + } + init_minimal_symbol_collection (); back_to = make_cleanup_discard_minimal_symbols (); @@ -1234,16 +1250,11 @@ elf_symfile_read (struct objfile *objfile, int symfile_flags) bfd *abfd = objfile->obfd; struct elfinfo ei; - elf_read_minimal_symbols (objfile, symfile_flags); - memset ((char *) &ei, 0, sizeof (ei)); - - /* Now process debugging information, which is contained in - special ELF sections. */ - - /* We first have to find them... */ bfd_map_over_sections (abfd, elf_locate_sections, (void *) & ei); + elf_read_minimal_symbols (objfile, symfile_flags, &ei); + /* ELF debugging information is inserted into the psymtab in the order of least informative first - most informative last. Since the psymtab table is searched `most recent insertion first' this diff --git a/gdb/minsyms.c b/gdb/minsyms.c index 14ca922..37aa179 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -979,9 +979,11 @@ prim_record_minimal_symbol_full (const char *name, int name_len, int copy_name, /* If we already read minimal symbols for this objfile, then don't ever allocate a new one. */ if (!objfile->per_bfd->minsyms_read) - msym_bunch_index++; + { + msym_bunch_index++; + objfile->per_bfd->n_minsyms++; + } msym_count++; - OBJSTAT (objfile, n_minsyms++); return msymbol; } diff --git a/gdb/objfiles.h b/gdb/objfiles.h index ad4db99..99917b9 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -145,7 +145,6 @@ struct obj_section struct objstats { - int n_minsyms; /* Number of minimal symbols read */ int n_psyms; /* Number of partial symbols read */ int n_syms; /* Number of full symbols read */ int n_stabs; /* Number of ".stabs" read (if applicable) */ @@ -206,6 +205,12 @@ struct objfile_per_bfd_storage struct minimal_symbol *msymbols; int minimal_symbol_count; + /* The number of minimal symbols read, before any minimal symbol + de-duplication is applied. Note in particular that this has only + a passing relationship with the actual size of the table above; + use minimal_symbol_count if you need the true size. */ + int n_minsyms; + /* This is true if minimal symbols have already been read. Symbol readers can use this to bypass minimal symbol reading. Also, the minimal symbol table management code in minsyms.c uses this to diff --git a/gdb/symmisc.c b/gdb/symmisc.c index 8d91c8d..7837c1f 100644 --- a/gdb/symmisc.c +++ b/gdb/symmisc.c @@ -111,9 +111,9 @@ print_objfile_statistics (void) if (OBJSTAT (objfile, n_stabs) > 0) printf_filtered (_(" Number of \"stab\" symbols read: %d\n"), OBJSTAT (objfile, n_stabs)); - if (OBJSTAT (objfile, n_minsyms) > 0) + if (objfile->per_bfd->n_minsyms > 0) printf_filtered (_(" Number of \"minimal\" symbols read: %d\n"), - OBJSTAT (objfile, n_minsyms)); + objfile->per_bfd->n_minsyms); if (OBJSTAT (objfile, n_psyms) > 0) printf_filtered (_(" Number of \"partial\" symbols read: %d\n"), OBJSTAT (objfile, n_psyms));