This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Daniel Jacobowitz wrote:
On Mon, Feb 11, 2008 at 04:41:08PM -0500, Aleksandar Ristovski wrote:
Daniel Jacobowitz wrote:
On Mon, Feb 11, 2008 at 03:51:59PM -0500, Aleksandar Ristovski wrote:
Daniel Jacobowitz wrote:
On Mon, Feb 11, 2008 at 03:23:35PM -0500, Aleksandar Ristovski wrote:
The attached patch checks if partial_symbol has already been added to the list instead of adding duplicate records.
How does this ever happen?  It seems very wrong.  Also, I am worried
that the linear search will be a bottleneck (this is quadratic as each
psymtab grows).
Yes, I understand your concern about the complexity... but...
That's only part of the problem.  You have this huge duplication of
identical partial symbols within the same block.  How did that happen?
It shouldn't.  Maybe we can avoid creating them in the first place.

Daniel, could you clarify: when you say "maybe we can avoid..." who is "we" - gdb or gcc?

Probably GDB, but I don't know. Can you show me an example of these unnecessary psymbols?

Some of them:
unsigned int
_GCC_ATTR_ALIGN_64t
long long int
_GCC_ATTR_ALIGN_u64t
long long unsigned int
_Int64t
_Uint64t
_GCC_ATTR_ALIGN_u32t
unsigned int
_GCC_ATTR_ALIGN_32t
int
_Uint32t
_Int32t
_GCC_ATTR_ALIGN_16t
short int
_GCC_ATTR_ALIGN_u16t
short unsigned int
_Int16t
_Uint16t
_GCC_ATTR_ALIGN_8t
signed char
_GCC_ATTR_ALIGN_u8t
unsigned char
_Int8t
_Uint8t
_Intptrt
_Uintptrt
_Longlong
_ULonglong

Additionally, please take a look at the modified patch, I have added bcache_added function to return whether it added the data or returned cached one. This way linear search is avoided.

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	11 Feb 2008 21:55:41 -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/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	11 Feb 2008 21:55:41 -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/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	11 Feb 2008 21:55:46 -0000
@@ -3102,15 +3103,20 @@ add_psymbol_to_list (char *name, int nam
 		     enum language language, struct objfile *objfile)
 {
   struct partial_symbol *psym;
-  char *buf = alloca (namelength + 1);
+  int added;
+  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,8 +3134,11 @@ 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);
+  psym = bcache_added (&psymbol, sizeof (struct partial_symbol),
+			    objfile->psymbol_cache, &added);
+
+  if (!added)
+    return psym;
 
   /* Save pointer to partial symbol in psymtab, growing symtab if needed. */
   if (list->next >= list->list + list->size)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]