From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8092 invoked by alias); 26 Apr 2012 17:27:39 -0000 Received: (qmail 8031 invoked by uid 22791); 26 Apr 2012 17:27:36 -0000 X-SWARE-Spam-Status: No, hits=-5.5 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 26 Apr 2012 17:27:19 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q3QHRImI016759 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 26 Apr 2012 13:27:18 -0400 Received: from host2.jankratochvil.net (ovpn-116-17.ams2.redhat.com [10.36.116.17]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q3QHRCSZ006124 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 26 Apr 2012 13:27:15 -0400 Date: Thu, 26 Apr 2012 18:01:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [3/10] introduce psymtab users Message-ID: <20120426172711.GA19350@host2.jankratochvil.net> References: <87haw7brks.fsf@fleche.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87haw7brks.fsf@fleche.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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 X-SW-Source: 2012-04/txt/msg00935.txt.bz2 On Wed, 25 Apr 2012 20:21:23 +0200, Tom Tromey wrote: [...] > @@ -142,6 +167,10 @@ struct partial_symtab > > unsigned char psymtabs_addrmap_supported; > > + /* A flag that is temporarily used when searching psymtabs. */ > + > + unsigned char searched_flag; Please make it the enum. > @@ -760,6 +769,11 @@ lookup_partial_symbol (struct partial_symtab *pst, const char *name, > static struct symtab * > psymtab_to_symtab (struct partial_symtab *pst) > { > + /* If it is a shared psymtab, find an unshared psymtab that includes > + it. Any such psymtab will do. */ > + while (pst->users != NULL) > + pst = pst->users[0]; Currently GDB already has a problem that if it needs to expand symtab A to get symbol X it already expands also symtabs B, C and D, all of them pretty big for C++. With such random choosing of a symtab for expansion this multi-expand performance hit may get even worse for big C++ programs. (Not sure if you do not already plan some more general fix for that.) > @@ -1236,6 +1260,102 @@ map_matching_symbols_psymtab (const char *name, domain_enum namespace, > } > } > > +/* A convenience enum to give names to some constants used in > + recursively_search_psymtabs. */ > + > +enum > + { > + PST_NOT_SEARCHED, > + PST_SEARCHED_AND_FOUND, > + PST_SEARCHED_AND_NOT_FOUND > + }; > + > +/* A helper for expand_symtabs_matching_via_partial that handles > + searching included psymtabs. */ Describe return value, please. > + > +static int > +recursively_search_psymtabs (struct partial_symtab *ps, > + struct objfile *objfile, > + enum search_domain kind, > + int (*name_matcher) (const char *, void *), > + void *data) > +{ > + struct partial_symbol **psym; > + struct partial_symbol **bound, **gbound, **sbound; > + int keep_going = 1; > + int result = PST_SEARCHED_AND_NOT_FOUND; > + int i; > + > + if (ps->searched_flag != PST_NOT_SEARCHED) > + return ps->searched_flag == PST_SEARCHED_AND_FOUND; > + > + /* Recurse into shared psymtabs first, because they may have already > + been searched, and this could save some time. */ > + for (i = 0; i < ps->number_of_dependencies; ++i) > + { > + int r; > + > + /* Skip non-shared dependencies, these are handled elsewhere. */ > + if (ps->dependencies[i]->users == NULL) > + continue; > + > + r = recursively_search_psymtabs (ps->dependencies[i], > + objfile, kind, name_matcher, data); > + if (r != 0) > + { > + ps->searched_flag = PST_SEARCHED_AND_FOUND; > + return 1; > + } > + } > + > + gbound = (objfile->global_psymbols.list > + + ps->globals_offset + ps->n_global_syms); nitpick: While I do not like it IMO GNU Coding Standard says: gbound = &objfile->global_psymbols.list[ps->globals_offset + ps->n_global_syms]; But it applies also to the current code. > + sbound = (objfile->static_psymbols.list > + + ps->statics_offset + ps->n_static_syms); > + bound = gbound; > + > + /* Go through all of the symbols stored in a partial > + symtab in one loop. */ > + psym = objfile->global_psymbols.list + ps->globals_offset; > + while (keep_going) I do not see why to use the 'keep_going' variable here, just use 'break;'. But it applies also to the current code. > + { > + if (psym >= bound) > + { > + if (bound == gbound && ps->n_static_syms != 0) > + { > + psym = objfile->static_psymbols.list + ps->statics_offset; > + bound = sbound; > + } > + else > + keep_going = 0; > + continue; > + } > + else > + { > + QUIT; > + > + if ((kind == ALL_DOMAIN > + || (kind == VARIABLES_DOMAIN > + && SYMBOL_CLASS (*psym) != LOC_TYPEDEF > + && SYMBOL_CLASS (*psym) != LOC_BLOCK) > + || (kind == FUNCTIONS_DOMAIN > + && SYMBOL_CLASS (*psym) == LOC_BLOCK) > + || (kind == TYPES_DOMAIN > + && SYMBOL_CLASS (*psym) == LOC_TYPEDEF)) > + && (*name_matcher) (SYMBOL_SEARCH_NAME (*psym), data)) > + { > + /* Found a match, so notify our caller. */ > + result = PST_SEARCHED_AND_FOUND; > + keep_going = 0; > + } > + } > + psym++; > + } > + > + ps->searched_flag = result; > + return result == PST_SEARCHED_AND_FOUND; > +} > + > static void > expand_symtabs_matching_via_partial > (struct objfile *objfile, Thanks, Jan