From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15418 invoked by alias); 10 Feb 2003 20:25:22 -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 15411 invoked from network); 10 Feb 2003 20:25:21 -0000 Received: from unknown (HELO crack.them.org) (65.125.64.184) by 172.16.49.205 with SMTP; 10 Feb 2003 20:25:21 -0000 Received: from nevyn.them.org ([66.93.61.169] ident=mail) by crack.them.org with asmtp (Exim 3.12 #1 (Debian)) id 18iMNL-0008NM-00; Mon, 10 Feb 2003 16:26:07 -0600 Received: from drow by nevyn.them.org with local (Exim 3.36 #1 (Debian)) id 18iKUE-0004hh-00; Mon, 10 Feb 2003 15:25:06 -0500 Date: Mon, 10 Feb 2003 20:25:00 -0000 From: Daniel Jacobowitz To: David Carlton Cc: gdb-patches@sources.redhat.com Subject: Re: RFA/symtab: Let search_symbols find exact matches Message-ID: <20030210202506.GA17910@nevyn.them.org> Mail-Followup-To: David Carlton , gdb-patches@sources.redhat.com References: <20030210160107.GA587@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.1i X-SW-Source: 2003-02/txt/msg00263.txt.bz2 On Mon, Feb 10, 2003 at 12:10:50PM -0800, David Carlton wrote: > On Mon, 10 Feb 2003 11:01:07 -0500, Daniel Jacobowitz 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