From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22208 invoked by alias); 11 Jul 2002 20:47:52 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 22198 invoked from network); 11 Jul 2002 20:47:51 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by sources.redhat.com with SMTP; 11 Jul 2002 20:47:51 -0000 Received: from dsl254-114-096.nyc1.dsl.speakeasy.net ([216.254.114.96] helo=nevyn.them.org) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 17Skqr-0005ep-00; Thu, 11 Jul 2002 15:47:49 -0500 Received: from drow by nevyn.them.org with local (Exim 3.35 #1 (Debian)) id 17Skqt-0001Nu-00; Thu, 11 Jul 2002 16:47:51 -0400 Date: Thu, 11 Jul 2002 13:51:00 -0000 From: Daniel Jacobowitz To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa] Symbol hashing (for the last time?) Message-ID: <20020711204751.GA14577@nevyn.them.org> Mail-Followup-To: Elena Zannoni , gdb-patches@sources.redhat.com References: <20011030112756.A1546@nevyn.them.org> <20020711005652.GA17895@nevyn.them.org> <15661.58306.728341.299965@localhost.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <15661.58306.728341.299965@localhost.redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2002-07/txt/msg00253.txt.bz2 On Thu, Jul 11, 2002 at 04:00:02PM -0400, Elena Zannoni wrote: > Daniel Jacobowitz writes: > > Here's a patch from last October, dusted off and merged to the current > > sources. The only substantial changes were some fixes for ada-lang.c, > > merged after I wrote the original patch. I've verified no regressions > > on i386-linux for GCC (2.95,3.0.4,3.1)/(stabs,dwarf2). > > > > This converts the normal symbol table lookups into hash tables. A few > > sorts of symbol tables aren't hashed: those produced by mdebugread.c > > and dstread.c, because they build symbol tables in lots of ad-hoc code, > > and symbol tables which are actually the arguments to a function > > (because order matters, or at least comments suggest so). > > Would you mind adding the paragraph above, to a comment in the block > structure, maybe above the hashtable flag? Sure. > > A next step > > will be to convert mdebugread.c, delete dstread.c (it's marked for an > > upcoming obsoletion, isn't it?), and then delete all the complicated > > binary search code since the only remaining unhashed symtabs will be > > argument lists, which are small. > > > > how about os9kread? Anything needs to be done there? Nope (though that one's scheduled to go away too, I think?). Any reader which uses finish_block is fine. > > (ALL_BLOCK_SYMBOLS): New macro. > > This is not really true. The macro was there already. Yup, just noticed that. Thanks. I added it in an earlier patch and never updated my changelog. > > (BLOCK_SHOULD_SORT): Do not sort hashed blocks. > > (struct symbol): Add `hash_next' pointer. > > * symtab.c (lookup_block_symbol): Search using the hash table when > > possible. > > (find_pc_sect_symtab): Use ALL_BLOCK_SYMBOLS. > > (search_symbols, find_addr_symbol): Likewise. > > > > * dstread.c (process_dst_block): Clear hashtable bit for new block. > > (read_dst_symtab): Likewise. > > * jv-lang.c (get_java_class_symtab): Likewise. > > * mdebugread.c: Include "gdb_assert.h". > > (shrink_block): Assert that the block being modified is not hashed. > > * coffread.c (patch_opaque_types): Use ALL_BLOCK_SYMBOLS. > > * symmisc.c (free_symtab_block): Walk the hash table when freeing > > symbols. > > (dump_symtab): Recognize hashed blocks. > > * printcmd.c (print_frame_args): Assert that function blocks do not > > have hashed symbol tables. > > * ada-lang.c (symtab_for_sym): Use ALL_BLOCK_SYMBOLS. > > (fill_in_ada_prototype, debug_print_block): Likewise. > > (ada_add_block_symbols): Use ALL_BLOCK_SYMBOLS. Handle hash tables. > > > > I now wonder if it would be better to define another 'for' macro for this: > for (sym = BLOCK_BUCKET (block, hash_index); sym; sym = sym->hash_next) > it seems to reoccur a few times. Even though it's not that important. I think I'll avoid introducing any more of those if I possibly can.... > I don't like too much the use of the hardcoded '5' maybe define a variable? I thought about this for a little bit and settled on: /* Macro used to set the size of a hashtable for N symbols. */ #define BLOCK_HASHTABLE_SIZE(n) ((N)/5 + 1) I don't really see the point in making it user tunable; if we ever find a case that this one is bad for, I'll gladly change my mind, though. > Otherwise it's ok. > (modulus the problem you found). Thanks! Committed. -- Daniel Jacobowitz Carnegie Mellon University MontaVista Software Debian GNU/Linux Developer