[patch] expanding tables for bfd/hash.c

[patch] expanding tables for bfd/hash.c

[DJ Delorie]

I originally proposed this patch nearly two years ago, but only
recently got access to a testcase that proved it fixed what I
originall wrote it for (in this case, a 1.5Gb executable went from 65
minutes of CPU time to 45 seconds!)  I've tested this for regressions
with a mips64-elf GCC test run, and I'm finally committing it.

PS: anyone who wants to volunteer to replace bfd's has with
libiberty's hash may do so by saying "why aren't we using libiberty's
hash functions?" ;-)

2006-05-01  DJ Delorie  <dj@redhat.com>

	* bfd-in.h (bfd_hash_table): Add count field.
	* bfd-in2.h: Regenerate.
	* hash.c (higher_prime_number): New.
	(bfd_hash_table_inint_n): Init count field.
	(bfd_hash_lookup): Grow table as needed.

Index: bfd-in.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in.h,v
retrieving revision 1.113
diff -p -U3 -r1.113 bfd-in.h
--- bfd-in.h	16 Mar 2006 12:20:15 -0000	1.113
+++ bfd-in.h	1 May 2006 19:32:05 -0000
@@ -376,6 +376,8 @@ struct bfd_hash_table
   struct bfd_hash_entry **table;
   /* The number of slots in the hash table.  */
   unsigned int size;
+  /* The number of entries in the hash table.  */
+  unsigned int count;
   /* The size of elements.  */
   unsigned int entsize;
   /* A function used to create new elements in the hash table.  The
Index: bfd-in2.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in2.h,v
retrieving revision 1.387
diff -p -U3 -r1.387 bfd-in2.h
--- bfd-in2.h	26 Mar 2006 00:38:42 -0000	1.387
+++ bfd-in2.h	1 May 2006 19:32:06 -0000
@@ -383,6 +383,8 @@ struct bfd_hash_table
   struct bfd_hash_entry **table;
   /* The number of slots in the hash table.  */
   unsigned int size;
+  /* The number of entries in the hash table.  */
+  unsigned int count;
   /* The size of elements.  */
   unsigned int entsize;
   /* A function used to create new elements in the hash table.  The
Index: hash.c
===================================================================
RCS file: /cvs/src/src/bfd/hash.c,v
retrieving revision 1.20
diff -p -U3 -r1.20 hash.c
--- hash.c	16 Mar 2006 12:20:15 -0000	1.20
+++ hash.c	1 May 2006 19:32:06 -0000
@@ -298,7 +298,72 @@ SUBSUBSECTION
 */
 
 /* The default number of entries to use when creating a hash table.  */
-#define DEFAULT_SIZE 4051
+#define DEFAULT_SIZE (4093)
+
+/* The following function returns a nearest prime number which is
+   greater than N, and near a power of two.  Copied from libiberty.  */
+
+static unsigned long
+higher_prime_number (unsigned long n)
+{
+  /* These are primes that are near, but slightly smaller than, a
+     power of two.  */
+  static const unsigned long primes[] = {
+    (unsigned long) 7,
+    (unsigned long) 13,
+    (unsigned long) 31,
+    (unsigned long) 61,
+    (unsigned long) 127,
+    (unsigned long) 251,
+    (unsigned long) 509,
+    (unsigned long) 1021,
+    (unsigned long) 2039,
+    (unsigned long) 4093,
+    (unsigned long) 8191,
+    (unsigned long) 16381,
+    (unsigned long) 32749,
+    (unsigned long) 65521,
+    (unsigned long) 131071,
+    (unsigned long) 262139,
+    (unsigned long) 524287,
+    (unsigned long) 1048573,
+    (unsigned long) 2097143,
+    (unsigned long) 4194301,
+    (unsigned long) 8388593,
+    (unsigned long) 16777213,
+    (unsigned long) 33554393,
+    (unsigned long) 67108859,
+    (unsigned long) 134217689,
+    (unsigned long) 268435399,
+    (unsigned long) 536870909,
+    (unsigned long) 1073741789,
+    (unsigned long) 2147483647,
+					/* 4294967291L */
+    ((unsigned long) 2147483647) + ((unsigned long) 2147483644),
+  };
+
+  const unsigned long *low = &primes[0];
+  const unsigned long *high = &primes[sizeof(primes) / sizeof(primes[0])];
+
+  while (low != high)
+    {
+      const unsigned long *mid = low + (high - low) / 2;
+      if (n >= *mid)
+	low = mid + 1;
+      else
+	high = mid;
+    }
+
+  /* If we've run out of primes, abort.  */
+  if (n > *low)
+    {
+      fprintf (stderr, "Cannot find prime bigger than %lu\n", n);
+      abort ();
+    }
+
+  return *low;
+}
+
 static size_t bfd_default_hash_table_size = DEFAULT_SIZE;
 
 /* Create a new hash table, given a number of entries.  */
@@ -330,6 +395,7 @@ bfd_hash_table_init_n (struct bfd_hash_t
   memset ((void *) table->table, 0, alloc);
   table->size = size;
   table->entsize = entsize;
+  table->count = 0;
   table->newfunc = newfunc;
   return TRUE;
 }
@@ -402,6 +468,7 @@ bfd_hash_lookup (struct bfd_hash_table *
   if (copy)
     {
       char *new;
+  table->count ++;
 
       new = objalloc_alloc ((struct objalloc *) table->memory, len + 1);
       if (!new)
@@ -417,6 +484,38 @@ bfd_hash_lookup (struct bfd_hash_table *
   hashp->next = table->table[index];
   table->table[index] = hashp;
 
+  if (table->count > table->size * 3 / 4)
+    {
+      int newsize = higher_prime_number (table->size);
+      struct bfd_hash_entry **newtable;
+      unsigned int hi;
+      unsigned int alloc;
+
+      alloc = newsize * sizeof (struct bfd_hash_entry *);
+
+      newtable = ((struct bfd_hash_entry **)
+		  objalloc_alloc ((struct objalloc *) table->memory, alloc));
+      memset ((PTR) newtable, 0, alloc);
+
+      for (hi = 0; hi < table->size; hi ++)
+	while (table->table[hi])
+	  {
+	    struct bfd_hash_entry *chain = table->table[hi];
+	    struct bfd_hash_entry *chain_end = chain;
+	    int index;
+
+	    while (chain_end->next && chain_end->next->hash == chain->hash)
+	      chain_end = chain_end->next; 
+
+	    table->table[hi] = chain_end->next;
+	    index = chain->hash % newsize;
+	    chain_end->next = newtable[index];
+	    newtable[index] = chain;
+	  }
+      table->table = newtable;
+      table->size = newsize;
+    }
+
   return hashp;
 }
 

Re: [patch] expanding tables for bfd/hash.c

[H. J. Lu]

On Mon, May 01, 2006 at 03:37:40PM -0400, DJ Delorie wrote:
> 
> I originally proposed this patch nearly two years ago, but only
> recently got access to a testcase that proved it fixed what I
> originall wrote it for (in this case, a 1.5Gb executable went from 65
> minutes of CPU time to 45 seconds!)  I've tested this for regressions
> with a mips64-elf GCC test run, and I'm finally committing it.
> 
> PS: anyone who wants to volunteer to replace bfd's has with
> libiberty's hash may do so by saying "why aren't we using libiberty's
> hash functions?" ;-)
> 

It doesn't work due to the way how string merge works.


H.J.

Re: [patch] expanding tables for bfd/hash.c

[Alan Modra]

On Mon, May 01, 2006 at 03:37:40PM -0400, DJ Delorie wrote:
>  /* The default number of entries to use when creating a hash table.  */
> -#define DEFAULT_SIZE 4051
> +#define DEFAULT_SIZE (4093)

I would guess this is why I'm seeing the following on x86.

Running /src/binutils-current/ld/testsuite/ld-elfvers/vers.exp ...
FAIL: vers6
FAIL: vers15
Running /src/binutils-current/ld/testsuite/ld-elfvsb/elfvsb.exp ...
Running /src/binutils-current/ld/testsuite/ld-elfweak/elfweak.exp ...
FAIL: ELF weak data first
FAIL: ELF weak data last
FAIL: ELF weak data first common
FAIL: ELF weak data last common
Running /src/binutils-current/ld/testsuite/ld-fastcall/fastcall.exp ...
Running /src/binutils-current/ld/testsuite/ld-frv/fdpic.exp ...
Running /src/binutils-current/ld/testsuite/ld-frv/frv-elf.exp ...
Running /src/binutils-current/ld/testsuite/ld-frv/tls.exp ...
Running /src/binutils-current/ld/testsuite/ld-h8300/h8300.exp ...
Running /src/binutils-current/ld/testsuite/ld-i386/i386.exp ...
FAIL: TLS -fpic -shared transitions
FAIL: TLS descriptor -fpic -shared transitions
FAIL: TLS -fpic and -fno-pic exec transitions
FAIL: TLS descriptor -fpic and -fno-pic exec transitions
FAIL: TLS -fno-pic -shared
FAIL: TLS with global dynamic and descriptors

Changing the symbol hash table size can change the order of symbols.
Are you offering to fix all the testsuite?

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] expanding tables for bfd/hash.c

[DJ Delorie]

> Are you offering to fix all the testsuite?

Yes, let me know which break and I'll rearrange the expected results
to match the new ordering.

Re: [patch] expanding tables for bfd/hash.c

[Daniel Jacobowitz]

On Tue, May 02, 2006 at 09:16:33PM -0400, DJ Delorie wrote:
> 
> > Are you offering to fix all the testsuite?
> 
> Yes, let me know which break and I'll rearrange the expected results
> to match the new ordering.

... Can't you run the testsuite?  If not, why are you committing
patches? :-)

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] expanding tables for bfd/hash.c

[Alan Modra]

On Tue, May 02, 2006 at 09:16:33PM -0400, DJ Delorie wrote:
> 
> > Are you offering to fix all the testsuite?
> 
> Yes, let me know which break and I'll rearrange the expected results
> to match the new ordering.

A quicker alternative might be to leave the default unchanged.  There
are a lot of tests that depend on symbol table order, and some only run
natively.

A simple change that accomplishes this is to depopulate primes[], which
might be a good idea in itself.  What is the point in growing a table
from 7 to 13?  You'd probably be better off growing to something quite a
lot larger, at least when the size is relatively small.

How about this?

	* hash.c (DEFAULT_SIZE): Revert last change.
	(higher_prime_number): Correct test for no larger prime.  Don't
	abort on error, instead return 0.  Depopulate primes[].
	(bfd_hash_lookup): If we overflow size, refuse to grow table.

Index: bfd/hash.c
===================================================================
RCS file: /cvs/src/src/bfd/hash.c,v
retrieving revision 1.21
diff -u -p -r1.21 hash.c
--- bfd/hash.c	1 May 2006 19:36:27 -0000	1.21
+++ bfd/hash.c	3 May 2006 03:43:05 -0000
@@ -298,10 +298,11 @@ SUBSUBSECTION
 */
 
 /* The default number of entries to use when creating a hash table.  */
-#define DEFAULT_SIZE (4093)
+#define DEFAULT_SIZE 4051
 
 /* The following function returns a nearest prime number which is
-   greater than N, and near a power of two.  Copied from libiberty.  */
+   greater than N, and near a power of two.  Copied from libiberty.
+   Returns zero for ridiculously large N to signify an error.  */
 
 static unsigned long
 higher_prime_number (unsigned long n)
@@ -309,20 +310,8 @@ higher_prime_number (unsigned long n)
   /* These are primes that are near, but slightly smaller than, a
      power of two.  */
   static const unsigned long primes[] = {
-    (unsigned long) 7,
-    (unsigned long) 13,
-    (unsigned long) 31,
-    (unsigned long) 61,
-    (unsigned long) 127,
-    (unsigned long) 251,
     (unsigned long) 509,
-    (unsigned long) 1021,
-    (unsigned long) 2039,
-    (unsigned long) 4093,
     (unsigned long) 8191,
-    (unsigned long) 16381,
-    (unsigned long) 32749,
-    (unsigned long) 65521,
     (unsigned long) 131071,
     (unsigned long) 262139,
     (unsigned long) 524287,
@@ -343,7 +332,7 @@ higher_prime_number (unsigned long n)
   };
 
   const unsigned long *low = &primes[0];
-  const unsigned long *high = &primes[sizeof(primes) / sizeof(primes[0])];
+  const unsigned long *high = &primes[sizeof (primes) / sizeof (primes[0])];
 
   while (low != high)
     {
@@ -354,12 +343,8 @@ higher_prime_number (unsigned long n)
 	high = mid;
     }
 
-  /* If we've run out of primes, abort.  */
-  if (n > *low)
-    {
-      fprintf (stderr, "Cannot find prime bigger than %lu\n", n);
-      abort ();
-    }
+  if (n >= *low)
+    return 0;
 
   return *low;
 }
@@ -486,12 +471,19 @@ bfd_hash_lookup (struct bfd_hash_table *
 
   if (table->count > table->size * 3 / 4)
     {
-      int newsize = higher_prime_number (table->size);
+      unsigned long newsize = higher_prime_number (table->size);
       struct bfd_hash_entry **newtable;
       unsigned int hi;
-      unsigned int alloc;
+      unsigned long alloc = newsize * sizeof (struct bfd_hash_entry *);
 
-      alloc = newsize * sizeof (struct bfd_hash_entry *);
+      /* If we can't find a higher prime, or we can't possibly alloc
+	 that much memory, don't try to grow the table.  */
+      if (newsize == 0 || alloc / sizeof (struct bfd_hash_entry *) != newsize)
+	{
+	  /* Lie.  Stops us trying to grow again for a while.  */
+	  table->count = 0;
+	  return hashp;
+	}
 
       newtable = ((struct bfd_hash_entry **)
 		  objalloc_alloc ((struct objalloc *) table->memory, alloc));

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] expanding tables for bfd/hash.c

[Alan Modra]

On Tue, May 02, 2006 at 11:23:34PM -0400, Daniel Jacobowitz wrote:
> ... Can't you run the testsuite?  If not, why are you committing
> patches? :-)

I think some of the tests affected are native only.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] expanding tables for bfd/hash.c

[DJ Delorie]

> A quicker alternative might be to leave the default unchanged.  There
> are a lot of tests that depend on symbol table order, and some only run
> natively.

I assume none (or few) tests have more than 4k symbols then?

> How about this?
> 
> 	* hash.c (DEFAULT_SIZE): Revert last change.
> 	(higher_prime_number): Correct test for no larger prime.  Don't
> 	abort on error, instead return 0.  Depopulate primes[].
> 	(bfd_hash_lookup): If we overflow size, refuse to grow table.

That looks like a good change.

Re: [patch] expanding tables for bfd/hash.c

[Alan Modra]

On Tue, May 02, 2006 at 11:52:14PM -0400, DJ Delorie wrote:
> 
> > A quicker alternative might be to leave the default unchanged.  There
> > are a lot of tests that depend on symbol table order, and some only run
> > natively.
> 
> I assume none (or few) tests have more than 4k symbols then?

Fortunately, that seems to be the case.

> > How about this?
> > 
> > 	* hash.c (DEFAULT_SIZE): Revert last change.
> > 	(higher_prime_number): Correct test for no larger prime.  Don't
> > 	abort on error, instead return 0.  Depopulate primes[].
> > 	(bfd_hash_lookup): If we overflow size, refuse to grow table.
> 
> That looks like a good change.

I'm committing a slightly different patch after realising that leaving
8191 in the table means the first table expansion is only doubling in
size.

Index: bfd/hash.c
===================================================================
RCS file: /cvs/src/src/bfd/hash.c,v
retrieving revision 1.21
diff -u -p -r1.21 hash.c
--- bfd/hash.c	1 May 2006 19:36:27 -0000	1.21
+++ bfd/hash.c	3 May 2006 04:13:05 -0000
@@ -298,10 +298,11 @@ SUBSUBSECTION
 */
 
 /* The default number of entries to use when creating a hash table.  */
-#define DEFAULT_SIZE (4093)
+#define DEFAULT_SIZE 4051
 
 /* The following function returns a nearest prime number which is
-   greater than N, and near a power of two.  Copied from libiberty.  */
+   greater than N, and near a power of two.  Copied from libiberty.
+   Returns zero for ridiculously large N to signify an error.  */
 
 static unsigned long
 higher_prime_number (unsigned long n)
@@ -309,18 +310,8 @@ higher_prime_number (unsigned long n)
   /* These are primes that are near, but slightly smaller than, a
      power of two.  */
   static const unsigned long primes[] = {
-    (unsigned long) 7,
-    (unsigned long) 13,
-    (unsigned long) 31,
-    (unsigned long) 61,
     (unsigned long) 127,
-    (unsigned long) 251,
-    (unsigned long) 509,
-    (unsigned long) 1021,
     (unsigned long) 2039,
-    (unsigned long) 4093,
-    (unsigned long) 8191,
-    (unsigned long) 16381,
     (unsigned long) 32749,
     (unsigned long) 65521,
     (unsigned long) 131071,
@@ -343,7 +334,7 @@ higher_prime_number (unsigned long n)
   };
 
   const unsigned long *low = &primes[0];
-  const unsigned long *high = &primes[sizeof(primes) / sizeof(primes[0])];
+  const unsigned long *high = &primes[sizeof (primes) / sizeof (primes[0])];
 
   while (low != high)
     {
@@ -354,12 +345,8 @@ higher_prime_number (unsigned long n)
 	high = mid;
     }
 
-  /* If we've run out of primes, abort.  */
-  if (n > *low)
-    {
-      fprintf (stderr, "Cannot find prime bigger than %lu\n", n);
-      abort ();
-    }
+  if (n >= *low)
+    return 0;
 
   return *low;
 }
@@ -486,12 +473,19 @@ bfd_hash_lookup (struct bfd_hash_table *
 
   if (table->count > table->size * 3 / 4)
     {
-      int newsize = higher_prime_number (table->size);
+      unsigned long newsize = higher_prime_number (table->size);
       struct bfd_hash_entry **newtable;
       unsigned int hi;
-      unsigned int alloc;
+      unsigned long alloc = newsize * sizeof (struct bfd_hash_entry *);
 
-      alloc = newsize * sizeof (struct bfd_hash_entry *);
+      /* If we can't find a higher prime, or we can't possibly alloc
+	 that much memory, don't try to grow the table.  */
+      if (newsize == 0 || alloc / sizeof (struct bfd_hash_entry *) != newsize)
+	{
+	  /* Lie.  Stops us trying to grow again for a while.  */
+	  table->count = 0;
+	  return hashp;
+	}
 
       newtable = ((struct bfd_hash_entry **)
 		  objalloc_alloc ((struct objalloc *) table->memory, alloc));
@@ -505,7 +499,7 @@ bfd_hash_lookup (struct bfd_hash_table *
 	    int index;
 
 	    while (chain_end->next && chain_end->next->hash == chain->hash)
-	      chain_end = chain_end->next; 
+	      chain_end = chain_end->next;
 
 	    table->table[hi] = chain_end->next;
 	    index = chain->hash % newsize;


-- 
Alan Modra
IBM OzLabs - Linux Technology Centre