From mboxrd@z Thu Jan 1 00:00:00 1970 From: Elena Zannoni To: Daniel Jacobowitz Cc: Elena Zannoni , gdb-patches@sources.redhat.com Subject: Re: [rfa] symbol hashing, part 1/n - updates to hash functions Date: Fri, 12 Oct 2001 08:59:00 -0000 Message-id: <15303.5384.908325.196958@krustylu.cygnus.com> References: <20011009105716.A7317@nevyn.them.org> <15302.12828.829882.614493@krustylu.cygnus.com> <20011011200054.B22256@nevyn.them.org> X-SW-Source: 2001-10/msg00168.html Daniel Jacobowitz writes: > On Thu, Oct 11, 2001 at 07:58:20PM -0400, Elena Zannoni wrote: > > Daniel Jacobowitz writes: > > > This patch still has two logical parts; if you strongly prefer I can break > > > it up further, but they are somewhat intertwined and I think neither should > > > be objectionable. They are: > > > - Fix a looping bug in msymbol_hash_iw. It would not stop on '(' if there > > > was whitespace before it. > > > - Update to use the identifier hash function that libiberty uses, and > > > more buckets. > > > > > > Is this OK? > > > > Looks ok to me in theory. Except that, why was the > > > > '% MINIMAL_SYMBOL_HASH_SIZE;' > > > > bit moved outside of the msymbol_hash and msymbol_hash_iw functions? > > You still do the same operation with the results returned by the two > > functions anyway. > > > > Also, where are these 2 functions used besides mynsyms.c? I think we > > should make them static and remove the extern from symtab.h. > > Both the moving of modulus and the no-other-uses are addressed by the > hashing patches. These are the hash functions I will use on the > symtabs; they work for symbols as well as for minsyms. A symtab has a > dynamic number of buckets. > Ok, I can see what's coming. But then, I would definitely prefer to move the '%' out of the functions only when the rest of the patch is submitted. It doesn't really fit with the changes you are making in this patch. > > Can you give me an example where the '(' error comes up? (Just so I > > understand it better). How did you come up with the number of > > buckets? Is this also used in libiberty? > > The '(' error looks like this: > > Hash the string "operator* ()". > At one point, string = " ()". The initial whitespace loop changes this > to "()". Then the character is not hashed (because of the if test > already present), but ++string is triggered. The while loop now > continues, because *string == ')' instead of '('. > OK, thanks. > The number of blocks I just came up with by experimentation (well, Dan > did, and then I experimented with it and was satisfied). Libiberty > uses expandable hash tables; I could simply use them instead, but I'd > rather postpone that change until we've got the rest of hashing in > place. > Yes, ok. > > Can you fix it and resubmit? > > After my explanations, does anything else need fixing? > Just hold on the '%' move until the next patch. Everything else is fine. > Thanks for looking at these patches! > Thanks for bearing with me! Elena > -- > Daniel Jacobowitz Carnegie Mellon University > MontaVista Software Debian GNU/Linux Developer