Re: RFA/symtab: Let search_symbols find exact matches

[Daniel Jacobowitz <drow@mvista.com>]

On Mon, Feb 10, 2003 at 12:10:50PM -0800, David Carlton wrote:
> On Mon, 10 Feb 2003 11:01:07 -0500, Daniel Jacobowitz <drow@mvista.com> said:
> 
> > This patch renames search_symbols to search_symbols_aux, and lets it
> > take an argument that specified regex or exact (well, strcmp_iw)
> > matching.  Then search_symbols becomes a wrapper.
> 
> > The new function is used by make_symbol_overload_list, which shaves
> > 50% time off of some tests in the C++ directory if your system libc
> > has debugging symbols; it removes the bogusity where all psymtabs
> > were converted to symtabs during overload resolution.  Whew.  This
> > cuts memory for namespace.exp from 70MB to 7MB, allowing some of my
> > hardware to actually run the test without crashing.
> 
> I love combining functions, but your patch makes me a bit nervous:
> search_symbols and make_symbol_overload_list are fairly different
> beasts.  If all you want to do is get rid of the aforementioned
> bogosity, how about the much more modest patch that I've included at
> the end of this message?
> 
> Benefits:
> 
> * It's more or less the minimal patch necessary to avoid converting
>   unnecessary symtabs.
> 
> * It should be faster: it uses the fact that global psymbols are
>   sorted.

[Bah, we should hash them.  We hash everything else.]

> Drawbacks:
> 
> * I'm to busy to spend too much time today thinking about whether or
>   not lookup_partial_symbol really works well enough for this patch to
>   work.  It certainly depends on partial symbols being demangled, but
>   they are now.  My guess is that lookup_partial_symbol still doesn't
>   work quite right (both because of improper use of
>   SYMBOL_SOURCE_NAME, and because of improper use of strcmp instead of
>   strcmp_iw).  The former shouldn't be a big deal; the latter might be
>   a big deal in some situations but probably shouldn't affect
>   lookup_partial_symbol (I _think_).  The correct fix there is to fix
>   lookup_partial_symbol, though, rather than avoiding its use.  (I
>   have run the patch through the testsuite; it passes, but that means
>   almost nothing, given how little the testsuite exercises partial
>   symbols.)
> 
> Could be either:
> 
> * If you really want some of the extra stuff that search_symbols does
>   and that make_symbol_overload_list doesn't (e.g. look at minimal
>   symbols), then of course your patch is a better idea.  But if you
>   don't, then probably you should avoid using search_symbols.
> 
> Anyways, give it a run on your platform to see if it works.  Compared
> to unpatched GDB, I'm getting a 3x memory improvement and a 2x speed
> improvement when running namespace.exp.
> 
> (If anybody else out there wants a laugh, just try to figure out what
> the part of lookup_partial_symbols that this patch removes actually
> does.  It's rather different from what the comment claims...)

There are like a million ways in GDB to find a list of symbols.  Me, I
think that's a recipe for suffering.  They're all subtly different.  I
want the same set of symbols considered for overload resolution and tab
completion, and there's no reason that this should be different from
the results of "break".  I intend some day to condense them all.

I imagine your patch will work; I'd still rather centralize the logic. 
I'm open to other opinions, though.

You had a good point about the sorted-list thing, since we really only
want "does at least one partial symbol in this psymtab match".  That
can be done as a follow-up patch to search_symbols_aux.  Really doing
this properly requires symbol functions which can return lists, and
we'll have that someday.  Then the logic would look like:
  ALL_SYMTABS
    get all matching functions in this symtab
  ALL_PSYMTABS
    if readin continue
    find a matching function in this psymtab
    if there is one
      read in
      get all matching functions in the new symtab

--  
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer