This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
[rfa] delete lookup_symbol_aux_minsyms
- From: David Carlton <carlton at math dot stanford dot edu>
- To: gdb-patches at sources dot redhat dot com
- Cc: Elena Zannoni <ezannoni at redhat dot com>, Jim Blandy <jimb at redhat dot com>
- Date: 22 Feb 2003 11:54:40 -0800
- Subject: [rfa] delete lookup_symbol_aux_minsyms
As posted in
<http://sources.redhat.com/ml/gdb-patches/2003-02/msg00496.html>, 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 at math dot stanford dot edu
2003-02-22 David Carlton <carlton at math dot stanford dot edu>
* 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 */