From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32304 invoked by alias); 13 Feb 2008 05:23:09 -0000 Received: (qmail 32282 invoked by uid 22791); 13 Feb 2008 05:23:07 -0000 X-Spam-Check-By: sourceware.org Received: from qnxmail.qnx.com (HELO qnxmail.qnx.com) (209.226.137.76) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 13 Feb 2008 05:22:45 +0000 Received: from smtp.ott.qnx.com (smtp.ott.qnx.com [10.42.96.5]) by hub.ott.qnx.com (8.9.3/8.9.3) with ESMTP id AAA30657; Wed, 13 Feb 2008 00:07:57 -0500 Received: from [192.168.20.164] (pptp_vpn-20-164 [192.168.20.164]) by smtp.ott.qnx.com (8.8.8/8.6.12) with ESMTP id AAA23742; Wed, 13 Feb 2008 00:22:42 -0500 Message-ID: <47B27EB1.6030606@qnx.com> Date: Wed, 13 Feb 2008 05:23:00 -0000 From: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.9 (Windows/20071031) MIME-Version: 1.0 To: Daniel Jacobowitz CC: gdb-patches@sourceware.org Subject: Re: [patch] Do not add partial_symbol again and again to the list References: <47B0AEC7.3070400@qnx.com> <20080211203809.GA29560@caradoc.them.org> <47B0B56F.4010607@qnx.com> <20080211210935.GA31767@caradoc.them.org> <47B0C0F4.4090302@qnx.com> <20080211214750.GA1953@caradoc.them.org> <47B0C7C6.9090605@qnx.com> <20080211223056.GA3833@caradoc.them.org> <47B0CF8A.6080306@qnx.com> <20080211225314.GA5832@caradoc.them.org> In-Reply-To: <20080211225314.GA5832@caradoc.them.org> Content-Type: multipart/mixed; boundary="------------040809060300090803040101" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-02/txt/msg00208.txt.bz2 This is a multi-part message in MIME format. --------------040809060300090803040101 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 4742 Daniel Jacobowitz wrote: > > Maybe there's some way we can avoid needing psymbols for types at all. > But I think we need to have a real design and some documentation for > it instead of just accidentally omitting them. > Here is a revised patch I am suggesting. It does not completely eliminate the need for psymbols for types, but does significantly reduce number of global type symbols. I tried to explain in detail why is it possible and safe. Here is the explanation: First the facts about the partial symbols: 1. add_partial_symbol This function gets called during reading the partial symbol information to add a partial_symbol to either global_list or static_list declared in struct objfile. The following table lists partial symbol types and scope. DW_TAG_ Lang Scope (DOMAIN,LOC) subprogram Ada global (VAR,BLOCK) All other global if ext or static (VAR,BLOCK) variable All global if ext or static (VAR,STATIC) typedef base_type subrange_type All static (VAR,TYPEDEF) namespace All global (VAR,TYPEDEF) class_type interface_type union_type enumeration_type cplus,java global (STRUCT,TYPEDEF) cplus,java,ada again to global (VAR,TYPEDEF) enumerator cplus,java global (VAR,CONST) All other static (VAR,CONST) Note that psymbol types namespace, class_type, interface_type, union_type, enumeration_type and enumerator, for c++ and java, always go to global list and will always have address 0 (anything else wouldn't make sense). 2. add_psymbol_to_list This function adds partial symbol first to bcache (objfile->psymbol_cache) and then to objfile->global_psymbols or objfile->static_psymbols list, depending on which one was passed in. It gets called by add_partial_symbol function. In this function we will add partial symbol to one of the lists; the partial symbols will always have exactly the same order in which they were added. This fact will be used when building partial symbol table which will use this list and with offset and number of symbols determine which partial symbols "belong" to it. The reason why partial symbol table needs to know about a symbol is to be able to eventually load full symbols. 3. lookup_partial_symbol Function looks up global or static list of objfile, but only subset of it determined by partial symtab's offset and number of symbols. Returns partial symbol found. This function is always called from a loop through all partial symtabs (this fact is important). When is a partial symbol really matched: Now let's look at the partial symbols which are added only to global partial symbol list: namespace, class_type, interface_type, union_type, enumeration_type, enumerator. The partial symbol will contain names and no address (it will be 0). Therefore, lookup_partial_symbol will always return the first partial symbol table where the psymbol was found. Once the psymbol was found, most probably full symbols will be loaded; once full symbols are loaded, all subsequent lookups for a symbol will first find it in full symtab and will never look for it in the partial symtab again. As a result, all but the first addition of such partial symbol will never be found and are therefore redundant and only take up time to qsort partial symbols within a partial symbol table. The patch: This revised patch adds global symbols only once to first partial symbol table where the symbol was encountered, but only for namespace, class_type, interface_type, union_type, enumeration_type and enumerator partial symbol types (for the last two, only for java and cplus). 2008-02-12 Aleksandar Ristovski * bcache.c (bcache_data): Call bcache_added function. (bcache_added): New function name. Body of function bcache_data is used here with the addition of 'added' argument. The argument is optional and if specified, bcache_added returns 1 if the data was added and 0 if it was found in the hash. * bcache.h (bcache_added): New function. * dwarf2read.c (add_partial_symbol): Make call to new function add_psymbol_to_global_list for namespace, class_type, interface_type, union_type, enumeration_type and enumerator partial symbol types. * symfile.c (add_psymbol_to_bcache): New helper function, takes part of work from add_psymbol_to_list - initialises partial symbol and stashes it in objfile's cache. (append_psymbol_to_list): New helper function, takes other part of work from add_psymbol_to_list - adds partial symbol to the given list. (add_psymbol_to_list): Call helper functions instead of doing work here. Functionally, the function hasn't changed. (add_psymbol_to_global_list): New function, adds partial symbol to global list and does it only once per objfile. * symfile.h (add_psymbol_to_global_list): New function. --------------040809060300090803040101 Content-Type: text/plain; name="bcache_added_donotduplicatepsyms.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="bcache_added_donotduplicatepsyms.diff" Content-length: 11694 Index: gdb/bcache.c =================================================================== RCS file: /cvs/src/src/gdb/bcache.c,v retrieving revision 1.20 diff -u -p -r1.20 bcache.c --- gdb/bcache.c 1 Jan 2008 22:53:09 -0000 1.20 +++ gdb/bcache.c 13 Feb 2008 04:24:37 -0000 @@ -197,11 +197,34 @@ expand_hash_table (struct bcache *bcache static void * bcache_data (const void *addr, int length, struct bcache *bcache) { + return bcache_added (addr, length, bcache, NULL); +} + + +void * +deprecated_bcache (const void *addr, int length, struct bcache *bcache) +{ + return bcache_data (addr, length, bcache); +} + +const void * +bcache (const void *addr, int length, struct bcache *bcache) +{ + return bcache_data (addr, length, bcache); +} + +void * +bcache_added (const void *addr, int length, struct bcache *bcache, + int *added) +{ unsigned long full_hash; unsigned short half_hash; int hash_index; struct bstring *s; + if (added) + *added = 0; + /* If our average chain length is too high, expand the hash table. */ if (bcache->unique_count >= bcache->num_buckets * CHAIN_LENGTH_THRESHOLD) expand_hash_table (bcache); @@ -242,21 +265,12 @@ bcache_data (const void *addr, int lengt bcache->unique_size += length; bcache->structure_size += BSTRING_SIZE (length); + if (added) + *added = 1; + return &new->d.data; } } - -void * -deprecated_bcache (const void *addr, int length, struct bcache *bcache) -{ - return bcache_data (addr, length, bcache); -} - -const void * -bcache (const void *addr, int length, struct bcache *bcache) -{ - return bcache_data (addr, length, bcache); -} /* Allocating and freeing bcaches. */ Index: gdb/bcache.h =================================================================== RCS file: /cvs/src/src/gdb/bcache.h,v retrieving revision 1.12 diff -u -p -r1.12 bcache.h --- gdb/bcache.h 1 Jan 2008 22:53:09 -0000 1.12 +++ gdb/bcache.h 13 Feb 2008 04:24:37 -0000 @@ -150,6 +150,8 @@ extern void *deprecated_bcache (const vo extern const void *bcache (const void *addr, int length, struct bcache *bcache); +extern void *bcache_added (const void *addr, int length, + struct bcache *bcache, int *added); /* Free all the storage used by BCACHE. */ extern void bcache_xfree (struct bcache *bcache); Index: gdb/dwarf2read.c =================================================================== RCS file: /cvs/src/src/gdb/dwarf2read.c,v retrieving revision 1.251 diff -u -p -r1.251 dwarf2read.c --- gdb/dwarf2read.c 1 Feb 2008 22:45:13 -0000 1.251 +++ gdb/dwarf2read.c 13 Feb 2008 04:24:40 -0000 @@ -1976,9 +1976,8 @@ add_partial_symbol (struct partial_die_i 0, (CORE_ADDR) 0, cu->language, objfile); break; case DW_TAG_namespace: - add_psymbol_to_list (actual_name, strlen (actual_name), + add_psymbol_to_global_list (actual_name, strlen (actual_name), VAR_DOMAIN, LOC_TYPEDEF, - &objfile->global_psymbols, 0, (CORE_ADDR) 0, cu->language, objfile); break; case DW_TAG_class_type: @@ -2000,32 +1999,37 @@ add_partial_symbol (struct partial_die_i /* NOTE: carlton/2003-10-07: See comment in new_symbol about static vs. global. */ - add_psymbol_to_list (actual_name, strlen (actual_name), - STRUCT_DOMAIN, LOC_TYPEDEF, - (cu->language == language_cplus - || cu->language == language_java) - ? &objfile->global_psymbols - : &objfile->static_psymbols, - 0, (CORE_ADDR) 0, cu->language, objfile); + if (cu->language == language_cplus + || cu->language == language_java) + add_psymbol_to_global_list (actual_name, strlen(actual_name), + STRUCT_DOMAIN, LOC_TYPEDEF, + 0, (CORE_ADDR) 0, cu->language, objfile); + else + add_psymbol_to_list (actual_name, strlen (actual_name), + STRUCT_DOMAIN, LOC_TYPEDEF, + &objfile->static_psymbols, + 0, (CORE_ADDR) 0, cu->language, objfile); if (cu->language == language_cplus || cu->language == language_java || cu->language == language_ada) { /* For C++ and Java, these implicitly act as typedefs as well. */ - add_psymbol_to_list (actual_name, strlen (actual_name), - VAR_DOMAIN, LOC_TYPEDEF, - &objfile->global_psymbols, - 0, (CORE_ADDR) 0, cu->language, objfile); + add_psymbol_to_global_list (actual_name, strlen (actual_name), + VAR_DOMAIN, LOC_TYPEDEF, + 0, (CORE_ADDR) 0, cu->language, objfile); } break; case DW_TAG_enumerator: - add_psymbol_to_list (actual_name, strlen (actual_name), + if (cu->language == language_cplus + || cu->language == language_java) + add_psymbol_to_global_list (actual_name, strlen (actual_name), + VAR_DOMAIN, LOC_CONST, + 0, (CORE_ADDR) 0, cu->language, objfile); + else + add_psymbol_to_list (actual_name, strlen (actual_name), VAR_DOMAIN, LOC_CONST, - (cu->language == language_cplus - || cu->language == language_java) - ? &objfile->global_psymbols - : &objfile->static_psymbols, + &objfile->static_psymbols, 0, (CORE_ADDR) 0, cu->language, objfile); break; default: Index: gdb/symfile.c =================================================================== RCS file: /cvs/src/src/gdb/symfile.c,v retrieving revision 1.198 diff -u -p -r1.198 symfile.c --- gdb/symfile.c 29 Jan 2008 22:47:20 -0000 1.198 +++ gdb/symfile.c 13 Feb 2008 04:24:41 -0000 @@ -3079,38 +3079,27 @@ start_psymtab_common (struct objfile *ob return (psymtab); } -/* Add a symbol with a long value to a psymtab. - Since one arg is a struct, we pass in a ptr and deref it (sigh). - Return the partial symbol that has been added. */ - -/* NOTE: carlton/2003-09-11: The reason why we return the partial - symbol is so that callers can get access to the symbol's demangled - name, which they don't have any cheap way to determine otherwise. - (Currenly, dwarf2read.c is the only file who uses that information, - though it's possible that other readers might in the future.) - Elena wasn't thrilled about that, and I don't blame her, but we - couldn't come up with a better way to get that information. If - it's needed in other situations, we could consider breaking up - SYMBOL_SET_NAMES to provide access to the demangled name lookup - cache. */ - -const struct partial_symbol * -add_psymbol_to_list (char *name, int namelength, domain_enum domain, - enum address_class class, - struct psymbol_allocation_list *list, long val, /* Value as a long */ - CORE_ADDR coreaddr, /* Value as a CORE_ADDR */ - enum language language, struct objfile *objfile) +static struct partial_symbol * +add_psymbol_to_bcache (char *name, int namelength, domain_enum domain, + enum address_class class, + long val, /* Value as a long */ + CORE_ADDR coreaddr, /* Value as a CORE_ADDR */ + enum language language, struct objfile *objfile, + int *added) { - struct partial_symbol *psym; - char *buf = alloca (namelength + 1); + char *buf = name; /* psymbol is static so that there will be no uninitialized gaps in the structure which might contain random data, causing cache misses in bcache. */ static struct partial_symbol psymbol; - - /* Create local copy of the partial symbol */ - memcpy (buf, name, namelength); - buf[namelength] = '\0'; + + if (name[namelength] != '\0') + { + buf = alloca (namelength + 1); + /* Create local copy of the partial symbol */ + memcpy (buf, name, namelength); + buf[namelength] = '\0'; + } /* val and coreaddr are mutually exclusive, one of them *will* be zero */ if (val != 0) { @@ -3128,17 +3117,78 @@ add_psymbol_to_list (char *name, int nam SYMBOL_SET_NAMES (&psymbol, buf, namelength, objfile); /* Stash the partial symbol away in the cache */ - psym = deprecated_bcache (&psymbol, sizeof (struct partial_symbol), - objfile->psymbol_cache); + return bcache_added (&psymbol, sizeof (struct partial_symbol), + objfile->psymbol_cache, added); +} - /* Save pointer to partial symbol in psymtab, growing symtab if needed. */ - if (list->next >= list->list + list->size) +static void +append_psymbol_to_list (struct psymbol_allocation_list *list, + struct partial_symbol *psym, + struct objfile *objfile) +{ +if (list->next >= list->list + list->size) { extend_psymbol_list (list, objfile); } *list->next++ = psym; OBJSTAT (objfile, n_psyms++); +} + +/* Add a symbol with a long value to a psymtab. + Since one arg is a struct, we pass in a ptr and deref it (sigh). + Return the partial symbol that has been added. */ +/* NOTE: carlton/2003-09-11: The reason why we return the partial + symbol is so that callers can get access to the symbol's demangled + name, which they don't have any cheap way to determine otherwise. + (Currenly, dwarf2read.c is the only file who uses that information, + though it's possible that other readers might in the future.) + Elena wasn't thrilled about that, and I don't blame her, but we + couldn't come up with a better way to get that information. If + it's needed in other situations, we could consider breaking up + SYMBOL_SET_NAMES to provide access to the demangled name lookup + cache. */ + +const struct partial_symbol * +add_psymbol_to_list (char *name, int namelength, domain_enum domain, + enum address_class class, + struct psymbol_allocation_list *list, long val, /* Value as a long */ + CORE_ADDR coreaddr, /* Value as a CORE_ADDR */ + enum language language, struct objfile *objfile) +{ + struct partial_symbol *psym; + /* Stash the partial symbol away in the cache */ + psym = add_psymbol_to_bcache (name, namelength, domain, class, + val, coreaddr, language, objfile, NULL); + + /* Save pointer to partial symbol in psymtab, growing symtab if needed. */ + append_psymbol_to_list (list, psym, objfile); + return psym; +} + +/* This function should be called only for those types (and for supporting + languages) that do not need duplicate global type information. For C++, + and java these partial symbol types are: + namespace, class_type, interface_type. */ + +const struct partial_symbol * +add_psymbol_to_global_list (char *name, int namelength, domain_enum domain, + enum address_class class, + long val, /* Value as a long */ + CORE_ADDR coreaddr, /* Value as a CORE_ADDR */ + enum language language, struct objfile *objfile) +{ + struct partial_symbol *psym; + int added; + + psym = add_psymbol_to_bcache (name, namelength, domain, class, val, + coreaddr, language, objfile, &added); + + if (!added) + return psym; + + /* Save pointer to partial symbol in psymtab, growing symtab if needed. */ + append_psymbol_to_list (&objfile->global_psymbols, psym, objfile); return psym; } Index: gdb/symfile.h =================================================================== RCS file: /cvs/src/src/gdb/symfile.h,v retrieving revision 1.46 diff -u -p -r1.46 symfile.h --- gdb/symfile.h 3 Feb 2008 22:13:29 -0000 1.46 +++ gdb/symfile.h 13 Feb 2008 04:24:41 -0000 @@ -199,6 +199,12 @@ struct partial_symbol *add_psymbol_to_li long, CORE_ADDR, enum language, struct objfile *); +extern const +struct partial_symbol *add_psymbol_to_global_list (char *, int, domain_enum, + enum address_class, + long, CORE_ADDR, + enum language, struct objfile *); + extern void init_psymbol_list (struct objfile *, int); extern void sort_pst_symbols (struct partial_symtab *); --------------040809060300090803040101--