From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4408 invoked by alias); 22 Feb 2003 19:54:42 -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 4401 invoked from network); 22 Feb 2003 19:54:41 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by 172.16.49.205 with SMTP; 22 Feb 2003 19:54:41 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id h1MJse207566; Sat, 22 Feb 2003 11:54:40 -0800 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: gdb-patches@sources.redhat.com Cc: Elena Zannoni , Jim Blandy Subject: [rfa] delete lookup_symbol_aux_minsyms From: David Carlton Date: Sat, 22 Feb 2003 19:54:00 -0000 Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-02/txt/msg00545.txt.bz2 As posted in , I believe that I have good reason to believe that: * The code in lookup_symbol_aux_minsyms only existed because once partial symbols didn't contain demangled names. * The 'else clause' in that function has been actively harmful (in correctness terms) in the past, and may still be so. * The rest of the function isn't harmful in correctness terms; it could be either an optimization or a pessimization So, it seems to me that, at the very least, the 'else clause' of lookup_symbol_aux_minsyms should be deleted; quite possibly, the whole function should be deleted. So: when might it be an optimization, and when might it be a pessimization? 1) If we're looking for a local symbol or a symbol that's in a symtab that's already been loaded, then lookup_symbol_aux_minsyms will never be reached, so it doesn't matter. 2) If we're looking for a function that's in a symtab that hasn't been loaded, then lookup_symbol_aux_minsyms will find it. However, if we deleted the call to lookup_symbol_aux_minsyms, then lookup_symbol_aux_psymtabs would also find it. So the question here is whether a minsym search is faster than a partial symbol table search. 3) If we're looking for something that's not in a loaded symtab (e.g. a type, a non-function variable, or a symbol that actually doesn't exist at all), then the call to lookup_symbol_aux_minsyms is a wasted of time, and is a pessimization. So the questions are: * Is case 2) an optimization or not? If it is, how much of an optimization is it? * How much of a pessimization is case 3)? * In practice, what's the relative frequency of case 2 vs. case 3? I wanted to test this; the obvious place to do this is by calling 'search_symbols', because that contains a lot of lookup_symbol calls. Unfortunately, when I investigated further, it turns out that search_symbols usually reads in all or almost all of the partial symbol tables, so we're usually in case 1. Oops. The thing is, though, I don't know of any other good way to generate lots of calls to lookup_symbol! What this suggests is that, in practice, performance issues won't play a key role here, so we should base our decision on whatever is correct and simplest. Just to see what would happen, I tried timing 'info functions' and 'info variables' (with the target program being mainline GDB); this might get some situations where case 3 happens (because my C library doesn't have debugging symbols). I tried these tests on mainline GDB, on a version of GDB with only the else clause removed, and on a version of GDB with all of lookup_symbol_aux removed. All three versions produced the same output, which is good! The results: * Mainline GDB and GDB with only the else clause removed produced the same range of times. (Full disclosure: removing the else clause might be making things ever so slightly slower, which I don't understand at all. But the time ranges were really very similar indeed, so this may just be an artifact of random fluctuations.) * The version with lookup_symbol_aux_minsyms removed entirely was consistently but slightly faster, along the lines of 1 or 2 percent. Not a bad thing, but by no means decisive: if we want to speed up search_symbols, I bet there are much better ways to do it. So what's the conclusion? Performance considerations don't seem to give a clear answer. So we should go with whatever's cleanest. My recommendation: * Delete the 'else' clause: it might cause correctness problems. * Comment out the remaining part of lookup_symbol_aux_minsyms: if somebody comes up with a situation where we spend lots of time searching for functions that aren't in a loaded symtab, we can consider uncommenting it and adding it back in. On a side note, there's the question of what to do with the HP bit. The comment in the HP bit says that the call to lookup_symbol_aux_minsyms was moved to the end because it had been forcing incorrect NULL returns. I'm 95% sure that that's a manifestation of the weird control flow that manifested itself in the 'force_return' variable that I had to introduce when creating lookup_symbol_aux_minsyms. If so, then deleting force_return (which I did in December) meant that the HP special case could have gone away. So the HP bit can go away, too. Here's a patch that implements my recommendations. Tested on i686-pc-linux-gnu/GCC3.1/DWARF-2, as well as by checking that it doesn't change the output of 'info symbols' or 'info variables' in one particular case. No new regressions; OK to commit? David Carlton carlton@math.stanford.edu 2003-02-22 David Carlton * symtab.c (lookup_symbol_aux): Comment out non-HP call to lookup_symbol_aux_minsyms; delete HP call. (lookup_symbol_aux_minsyms): Delete 'else' branch; comment out the rest of the function. Index: symtab.c =================================================================== RCS file: /cvs/src/src/gdb/symtab.c,v retrieving revision 1.92 diff -u -p -r1.92 symtab.c --- symtab.c 21 Feb 2003 15:24:18 -0000 1.92 +++ symtab.c 22 Feb 2003 18:52:12 -0000 @@ -115,12 +115,14 @@ struct symbol *lookup_symbol_aux_psymtab const namespace_enum namespace, struct symtab **symtab); +#if 0 static struct symbol *lookup_symbol_aux_minsyms (const char *name, const char *mangled_name, const namespace_enum namespace, int *is_a_field_of_this, struct symtab **symtab); +#endif static struct symbol *find_active_alias (struct symbol *sym, CORE_ADDR addr); @@ -981,8 +983,7 @@ lookup_symbol_aux (const char *name, con if (sym != NULL) return sym; -#ifndef HPUXHPPA - +#if 0 /* Check for the possibility of the symbol being a function or a mangled variable that is stored in one of the minimal symbol tables. Eventually, all global symbols might be resolved in this way. */ @@ -993,7 +994,6 @@ lookup_symbol_aux (const char *name, con if (sym != NULL) return sym; - #endif sym = lookup_symbol_aux_psymtabs (GLOBAL_BLOCK, name, mangled_name, @@ -1017,33 +1017,6 @@ lookup_symbol_aux (const char *name, con if (sym != NULL) return sym; -#ifdef HPUXHPPA - - /* Check for the possibility of the symbol being a function or - a global variable that is stored in one of the minimal symbol tables. - The "minimal symbol table" is built from linker-supplied info. - - RT: I moved this check to last, after the complete search of - the global (p)symtab's and static (p)symtab's. For HP-generated - symbol tables, this check was causing a premature exit from - lookup_symbol with NULL return, and thus messing up symbol lookups - of things like "c::f". It seems to me a check of the minimal - symbol table ought to be a last resort in any case. I'm vaguely - worried about the comment below which talks about FORTRAN routines "foo_" - though... is it saying we need to do the "minsym" check before - the static check in this case? - */ - - - sym = lookup_symbol_aux_minsyms (name, mangled_name, - namespace, is_a_field_of_this, - symtab); - - if (sym != NULL) - return sym; - -#endif - if (symtab != NULL) *symtab = NULL; return NULL; @@ -1219,6 +1192,7 @@ lookup_symbol_aux_psymtabs (int block_in return NULL; } +#if 0 /* Check for the possibility of the symbol being a function or a mangled variable that is stored in one of the minimal symbol tables. Eventually, all global symbols might be resolved in this @@ -1232,6 +1206,13 @@ lookup_symbol_aux_psymtabs (int block_in some additional conditions held as well, and it caused problems with HP-generated symbol tables. */ +/* NOTE: carlton/2003-02-22: As far as I can tell, this code only + existed to handle the fact that partial symbols once only contained + mangled names. This is no longer the case; I've deleted the code + that is now potentially harmful. What remains may be an + optimization or a pessimization; since there's no evidence that + it's an optimization, I'm commenting it out. */ + static struct symbol * lookup_symbol_aux_minsyms (const char *name, const char *mangled_name, @@ -1332,21 +1313,12 @@ lookup_symbol_aux_minsyms (const char *n *symtab = s; return fixup_symbol_section (sym, s->objfile); } - else if (MSYMBOL_TYPE (msymbol) != mst_text - && MSYMBOL_TYPE (msymbol) != mst_file_text - && !STREQ (name, SYMBOL_NAME (msymbol))) - { - /* This is a mangled variable, look it up by its - mangled name. */ - return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, - NULL, namespace, is_a_field_of_this, - symtab); - } } } return NULL; } +#endif /* Look, in partial_symtab PST, for symbol NAME. Check the global symbols if GLOBAL, the static symbols if not */