From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18754 invoked by alias); 10 Mar 2002 05:42:37 -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 18544 invoked from network); 10 Mar 2002 05:42:31 -0000 Received: from unknown (HELO localhost.redhat.com) (24.112.135.44) by sources.redhat.com with SMTP; 10 Mar 2002 05:42:31 -0000 Received: from cygnus.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id E072A3E0B; Sun, 10 Mar 2002 00:42:29 -0500 (EST) Message-ID: <3C8AF245.7060504@cygnus.com> Date: Sat, 09 Mar 2002 21:42:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:0.9.8) Gecko/20020210 X-Accept-Language: en-us MIME-Version: 1.0 To: Klee Dienes Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] Cached function lookup References: <8A9ABA76-1A1B-11D6-BA6D-0030653FA4C6@apple.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2002-03/txt/msg00131.txt.bz2 > (This is basically the same patch I sent last week, just updated > to the latest source base.) > > This patch allows functions in the target used by GDB ("malloc", > "scm_lookup_cstr", and later a ton of Objective-C functions) to have > their values cached and re-used unless the symbol table has changed > in-between calls. This is a performance win overall, and a particular > win when dispatching Objective-C method calls and looking up > Objective-C type information from the runtime. Klee, I think there are three changes in this: o a change to the signature of the function find_function_in_inferior(). I guess this is either connected the print (float) f(3.0) change or something related to an ObjectiveC patch. I'll skip that for now. o a change to breakpoint.c so it can more efficiently handle symbol table updates (defering things until when breakpoint_update is called). I'll comment on that separatly - if it is separated out there is a much better chance of MichaelS approving it :-) o a change add a function value cache I'll comment on the third, I think with that resolved and committed, it should be possible for the second to just fall out. The patch to breakpoint.c introduced the global variable symbol_generation. I don't think it is a good idea for the function cache code to be depending on a global variable controlled by breakpoint.c. Have a look at the target_new_objfile_hook() chain as a way of notifying the function-cache code of symtab updates updates? (I'm getting the feeling that the hook may have been wrongly named (?)). Can I suggest changing ``struct cached_value { ... }'' in value.h to the opaque (and hopefully better named): struct cached_function; /* or cached_function_value???? */ > + if (cval->generation != symbol_generation) > + { > + val = find_function_in_inferior (cval->name, cval->type); > + cval->val = *val; > + cval->val.next = NULL; > + cval->generation = symbol_generation; > + } I think: free_value(cval->val) cval->val = find.....() release_value(cval->val) is more correct. cvsl->val would need to become a ``struct value *'' but that is a good as as ``struct value'' should be treated as an opaque. > + val = allocate_value (cval->val.type); > + next = val->next; > + *val = cval->val; > + val->next = next; I think value_copy() is more correct. enjoy, Andrew