Re: [patch] Do not add partial_symbol again and again to the list

Aleksandar Ristovski
2008-02-13T05:23:09Z

[-- Attachment #1: Type: text/plain, Size: 4742 bytes --]

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 <aristovski@qnx.com>

* 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.

[-- Attachment #2: bcache_added_donotduplicatepsyms.diff --]
[-- Type: text/plain, Size: 11694 bytes --]

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);
-}
\f
/* 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);
}
\f
-/* 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 *);