From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22208 invoked by alias); 6 May 2008 19:24:56 -0000 Received: (qmail 22198 invoked by uid 22791); 6 May 2008 19:24:54 -0000 X-Spam-Check-By: sourceware.org Received: from qnxmail.qnx.com (HELO nimbus.ott.qnx.com) (209.226.137.76) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 06 May 2008 19:24:37 +0000 Received: from [10.42.100.129] (dhcp-100-129.ott.qnx.com [10.42.100.129]) by nimbus.ott.qnx.com with SMTP (Microsoft Exchange Internet Mail Service Version 5.5.2653.13) id J6QQVL0A; Tue, 6 May 2008 15:24:35 -0400 Message-ID: <4820B072.8030906@qnx.com> Date: Wed, 07 May 2008 08:22:00 -0000 From: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.14 (Windows/20080421) MIME-Version: 1.0 Newsgroups: gmane.comp.gdb.patches To: gdb-patches@sourceware.org, Daniel Jacobowitz CC: Aleksandar Ristovski Subject: Re: [patch] Do not add partial_symbol again and again to the list References: <20080211214750.GA1953@caradoc.them.org> <47B0C7C6.9090605@qnx.com> <20080211223056.GA3833@caradoc.them.org> <47B0CF8A.6080306@qnx.com> <20080211225314.GA5832@caradoc.them.org> <47B27EB1.6030606@qnx.com> <20080503205413.GA22704@caradoc.them.org> <481F58ED.5070207@qnx.com> <20080506141754.GA17275@caradoc.them.org> <48209891.3000400@qnx.com> <20080506175636.GA31803@caradoc.them.org> In-Reply-To: <20080506175636.GA31803@caradoc.them.org> Content-Type: multipart/mixed; boundary="------------040004090505050509010404" 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-05/txt/msg00252.txt.bz2 This is a multi-part message in MIME format. --------------040004090505050509010404 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1298 Daniel Jacobowitz wrote: > > I think we should finish up this patch, and then proceed from there. > > Should we use this optimization for all global psymtabs? If so, then > we ought to do it in common code so that all the other symbol readers, > like stabs, benefit. > Ok, here is simplified, but with broader consequences patch. The patch now affects all readers that use add_psymbol_to_list by not allowing duplicate partial symbols in the global psymbol list (for a given objfile). Tested on linux dwarf2 format, no regression. I did not test other debug formats. Thanks, Aleksandar ChangeLog * 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. * bcache.h (bcache_added): New function. * symfile.c (add_psymbol_to_bcache): New helper function, takes part of work from add_psymbol_to_list - initializes 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. If adding to global list, do not duplicate partial symbols in the partial symtab. --------------040004090505050509010404 Content-Type: text/plain; name="bcache_added_donotduplicatepsyms20080506.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="bcache_added_donotduplicatepsyms20080506.diff" Content-length: 8082 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 6 May 2008 18:31:17 -0000 @@ -197,11 +197,40 @@ 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); +} + +/* Find a copy of the LENGTH bytes at ADDR in BCACHE. If BCACHE has + never seen those bytes before, add a copy of them to BCACHE. In + either case, return a pointer to BCACHE's copy of that string. If + optional ADDED is not NULL, return 1 in case of new entry or 0 if + returning an old entry. */ + +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 +271,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 6 May 2008 18:31:18 -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/symfile.c =================================================================== RCS file: /cvs/src/src/gdb/symfile.c,v retrieving revision 1.203 diff -u -p -r1.203 symfile.c --- gdb/symfile.c 5 May 2008 16:13:49 -0000 1.203 +++ gdb/symfile.c 6 May 2008 18:31:19 -0000 @@ -3082,38 +3082,33 @@ 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) +/* Helper function, initialises partial symbol structure and stashes + it into objfile's bcache. Note that our caching mechanism will + use all fields of struct partial_symbol to determine hash value of the + structure. In other words, having two symbols with the same name but + different domain (or address) is possible and correct. */ + +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) { @@ -3131,17 +3126,62 @@ 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. */ +/* Helper function, adds partial symbol to the given partial symbol + list. */ + +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); - } + 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; + + int added; + + /* Stash the partial symbol away in the cache */ + psym = add_psymbol_to_bcache (name, namelength, domain, class, + val, coreaddr, language, objfile, &added); + + /* Do not duplicate global partial symbols. */ + if (list == &objfile->global_psymbols + && !added) + return psym; + + /* Save pointer to partial symbol in psymtab, growing symtab if needed. */ + append_psymbol_to_list (list, psym, objfile); return psym; } --------------040004090505050509010404-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22725 invoked by alias); 6 May 2008 19:25:12 -0000 Received: (qmail 22690 invoked by uid 22791); 6 May 2008 19:25:09 -0000 X-Spam-Check-By: sourceware.org Received: from main.gmane.org (HELO ciao.gmane.org) (80.91.229.2) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 06 May 2008 19:24:50 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1JtSmK-0007N3-Q8 for gdb-patches@sources.redhat.com; Tue, 06 May 2008 19:24:45 +0000 Received: from mobius.qnx.com ([209.226.137.108]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 06 May 2008 19:24:44 +0000 Received: from aristovski by mobius.qnx.com with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 06 May 2008 19:24:44 +0000 To: gdb-patches@sources.redhat.com From: Aleksandar Ristovski Subject: Re: [patch] Do not add partial_symbol again and again to the list Date: Wed, 07 May 2008 09:01:00 -0000 Message-ID: <4820B072.8030906@qnx.com> References: <20080211214750.GA1953@caradoc.them.org> <47B0C7C6.9090605@qnx.com> <20080211223056.GA3833@caradoc.them.org> <47B0CF8A.6080306@qnx.com> <20080211225314.GA5832@caradoc.them.org> <47B27EB1.6030606@qnx.com> <20080503205413.GA22704@caradoc.them.org> <481F58ED.5070207@qnx.com> <20080506141754.GA17275@caradoc.them.org> <48209891.3000400@qnx.com> <20080506175636.GA31803@caradoc.them.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------040004090505050509010404" Cc: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.14 (Windows/20080421) In-Reply-To: <20080506175636.GA31803@caradoc.them.org> 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-05/txt/msg00253.txt.bz2 Message-ID: <20080507090100.O1ghoUfl-GpiDhMAjOxBudHq9O28vXtUOmEvxmj7NZM@z> This is a multi-part message in MIME format. --------------040004090505050509010404 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1298 Daniel Jacobowitz wrote: > > I think we should finish up this patch, and then proceed from there. > > Should we use this optimization for all global psymtabs? If so, then > we ought to do it in common code so that all the other symbol readers, > like stabs, benefit. > Ok, here is simplified, but with broader consequences patch. The patch now affects all readers that use add_psymbol_to_list by not allowing duplicate partial symbols in the global psymbol list (for a given objfile). Tested on linux dwarf2 format, no regression. I did not test other debug formats. Thanks, Aleksandar ChangeLog * 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. * bcache.h (bcache_added): New function. * symfile.c (add_psymbol_to_bcache): New helper function, takes part of work from add_psymbol_to_list - initializes 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. If adding to global list, do not duplicate partial symbols in the partial symtab. --------------040004090505050509010404 Content-Type: text/plain; name="bcache_added_donotduplicatepsyms20080506.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="bcache_added_donotduplicatepsyms20080506.diff" Content-length: 8082 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 6 May 2008 18:31:17 -0000 @@ -197,11 +197,40 @@ 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); +} + +/* Find a copy of the LENGTH bytes at ADDR in BCACHE. If BCACHE has + never seen those bytes before, add a copy of them to BCACHE. In + either case, return a pointer to BCACHE's copy of that string. If + optional ADDED is not NULL, return 1 in case of new entry or 0 if + returning an old entry. */ + +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 +271,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 6 May 2008 18:31:18 -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/symfile.c =================================================================== RCS file: /cvs/src/src/gdb/symfile.c,v retrieving revision 1.203 diff -u -p -r1.203 symfile.c --- gdb/symfile.c 5 May 2008 16:13:49 -0000 1.203 +++ gdb/symfile.c 6 May 2008 18:31:19 -0000 @@ -3082,38 +3082,33 @@ 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) +/* Helper function, initialises partial symbol structure and stashes + it into objfile's bcache. Note that our caching mechanism will + use all fields of struct partial_symbol to determine hash value of the + structure. In other words, having two symbols with the same name but + different domain (or address) is possible and correct. */ + +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) { @@ -3131,17 +3126,62 @@ 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. */ +/* Helper function, adds partial symbol to the given partial symbol + list. */ + +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); - } + 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; + + int added; + + /* Stash the partial symbol away in the cache */ + psym = add_psymbol_to_bcache (name, namelength, domain, class, + val, coreaddr, language, objfile, &added); + + /* Do not duplicate global partial symbols. */ + if (list == &objfile->global_psymbols + && !added) + return psym; + + /* Save pointer to partial symbol in psymtab, growing symtab if needed. */ + append_psymbol_to_list (list, psym, objfile); return psym; } --------------040004090505050509010404--