[RFA/dwarf] Allocate abbrevs from a new obstack

[RFA/dwarf] Allocate abbrevs from a new obstack

[Daniel Jacobowitz]

Another independent performance optimization from my profiling.  This one is
good for several percent speedup on mainline.  My original description:

  - It plugs a huge memory leak.  After loading partial symbols for
  libc, GDB uses 5.6MB memory without this patch and 1.6MB with it.
  We weren't emptying the abbrev table.

[Specifically, we were emptying it after dwarf2_build_psymtabs_hard,
instead of once each time through the loop.  dwarf2_read_abbrevs then
zeroed out the allocated, unreferenced pointers.]

  - I spent a lot of time trying to find alternate data structures that
  would be more efficient.  The expandable hash table is slower, even if
  you initially allocate it to the right size.  The splay tree I expected
  to be quite good in this case, but it isn't either; the overhead is simply
  too high.  The fixed-size ad-hoc hash table appears to be the best we can
  do.

Tested on i686-pc-linux-gnu, no regressions.  OK to commit?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-02-24  Daniel Jacobowitz  <drow@mvista.com>

	* dwarf2read.c (struct comp_unit_head): Remove dwarf2_abbrevs array.
	(struct dwarf2_cu): Add abbrev_obstack and dwarf2_abbrevs
	pointer.  Update comment about comp_unit_head.
	(struct abbrev_info): Shorten two int flags.
	(dwarf_alloc_abbrev): Take a CU argument.
	(dwarf2_build_psymtabs_hard): Call dwarf2_free_abbrev_table
	each time through the loop.  Update cleanup argument.
	(psymtab_to_symtab_1): Update cleanup call.
	(dwarf2_read_abbrevs, dwarf2_alloc_abbrev): Allocate on the
	abbrev_obstack.
	(dwarf2_free_abbrev_table): Renamed from dwarf2_empty_abbrev_table.
	Just call obstack_free and clear the pointer.

Index: dwarf2read.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/dwarf2read.c,v
retrieving revision 1.135
diff -u -p -r1.135 dwarf2read.c
--- dwarf2read.c	21 Feb 2004 02:13:35 -0000	1.135
+++ dwarf2read.c	25 Feb 2004 03:31:08 -0000
@@ -205,10 +205,6 @@ struct comp_unit_head
 
     struct comp_unit_head *next;
 
-    /* DWARF abbreviation table associated with this compilation unit */
-
-    struct abbrev_info *dwarf2_abbrevs[ABBREV_HASH_SIZE];
-
     /* Base address of this compilation unit.  */
 
     CORE_ADDR base_address;
@@ -227,8 +223,7 @@ struct dwarf2_cu
   /* The header of the compilation unit.
 
      FIXME drow/2003-11-10: Some of the things from the comp_unit_head
-     should be moved to the dwarf2_cu structure; for instance the abbrevs
-     hash table.  */
+     should logically be moved to the dwarf2_cu structure.  */
   struct comp_unit_head header;
 
   struct function_range *first_fn, *last_fn, *cached_fn;
@@ -258,6 +253,12 @@ struct dwarf2_cu
      FT_NUM_MEMBERS compile time constant, which is the number of predefined
      fundamental types gdb knows how to construct.  */
   struct type *ftypes[FT_NUM_MEMBERS];	/* Fundamental types */
+
+  /* DWARF abbreviation table associated with this compilation unit.  */
+  struct abbrev_info **dwarf2_abbrevs;
+
+  /* Storage for the abbrev table.  */
+  struct obstack abbrev_obstack;
 };
 
 /* The line number information for a compilation unit (found in the
@@ -329,8 +330,8 @@ struct abbrev_info
   {
     unsigned int number;	/* number identifying abbrev */
     enum dwarf_tag tag;		/* dwarf tag */
-    int has_children;		/* boolean */
-    unsigned int num_attrs;	/* number of attributes */
+    unsigned short has_children;		/* boolean */
+    unsigned short num_attrs;	/* number of attributes */
     struct attr_abbrev *attrs;	/* an array of attribute descriptions */
     struct abbrev_info *next;	/* next in chain */
   };
@@ -673,7 +674,7 @@ char *dwarf2_read_section (struct objfil
 
 static void dwarf2_read_abbrevs (bfd *abfd, struct dwarf2_cu *cu);
 
-static void dwarf2_empty_abbrev_table (void *);
+static void dwarf2_free_abbrev_table (void *);
 
 static struct abbrev_info *dwarf2_lookup_abbrev (unsigned int,
 						 struct dwarf2_cu *);
@@ -905,7 +906,7 @@ static void dwarf2_free_tmp_obstack (voi
 
 static struct dwarf_block *dwarf_alloc_block (void);
 
-static struct abbrev_info *dwarf_alloc_abbrev (void);
+static struct abbrev_info *dwarf_alloc_abbrev (struct dwarf2_cu *);
 
 static struct die_info *dwarf_alloc_die (void);
 
@@ -1201,6 +1202,7 @@ dwarf2_build_psymtabs_hard (struct objfi
      left at all should be sufficient.  */
   while (info_ptr < dwarf_info_buffer + dwarf_info_size)
     {
+      struct cleanup *back_to_inner;
       struct dwarf2_cu cu;
       beg_of_comp_unit = info_ptr;
 
@@ -1238,7 +1240,7 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       /* Read the abbrevs for this compilation unit into a table */
       dwarf2_read_abbrevs (abfd, &cu);
-      make_cleanup (dwarf2_empty_abbrev_table, cu.header.dwarf2_abbrevs);
+      back_to_inner = make_cleanup (dwarf2_free_abbrev_table, &cu);
 
       /* Read the compilation unit die */
       info_ptr = read_partial_die (&comp_unit_die, abfd, info_ptr,
@@ -1315,6 +1317,8 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       info_ptr = beg_of_comp_unit + cu.header.length 
                                   + cu.header.initial_length_size;
+
+      do_cleanups (back_to_inner);
     }
   do_cleanups (back_to);
 }
@@ -1861,7 +1865,7 @@ psymtab_to_symtab_1 (struct partial_symt
 
   /* Read the abbrevs for this compilation unit  */
   dwarf2_read_abbrevs (abfd, &cu);
-  make_cleanup (dwarf2_empty_abbrev_table, cu.header.dwarf2_abbrevs);
+  make_cleanup (dwarf2_free_abbrev_table, &cu);
 
   cu.header.offset = offset;
 
@@ -4170,19 +4174,28 @@ dwarf2_read_abbrevs (bfd *abfd, struct d
   struct abbrev_info *cur_abbrev;
   unsigned int abbrev_number, bytes_read, abbrev_name;
   unsigned int abbrev_form, hash_number;
+  struct attr_abbrev *cur_attrs;
+  unsigned int allocated_attrs;
 
   /* Initialize dwarf2 abbrevs */
-  memset (cu_header->dwarf2_abbrevs, 0,
-          ABBREV_HASH_SIZE*sizeof (struct abbrev_info *));
+  obstack_init (&cu->abbrev_obstack);
+  cu->dwarf2_abbrevs = obstack_alloc (&cu->abbrev_obstack,
+				      (ABBREV_HASH_SIZE
+				       * sizeof (struct abbrev_info *)));
+  memset (cu->dwarf2_abbrevs, 0,
+          ABBREV_HASH_SIZE * sizeof (struct abbrev_info *));
 
   abbrev_ptr = dwarf_abbrev_buffer + cu_header->abbrev_offset;
   abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
   abbrev_ptr += bytes_read;
 
+  allocated_attrs = ATTR_ALLOC_CHUNK;
+  cur_attrs = xmalloc (allocated_attrs * sizeof (struct attr_abbrev));
+  
   /* loop until we reach an abbrev number of 0 */
   while (abbrev_number)
     {
-      cur_abbrev = dwarf_alloc_abbrev ();
+      cur_abbrev = dwarf_alloc_abbrev (cu);
 
       /* read in abbrev header */
       cur_abbrev->number = abbrev_number;
@@ -4198,24 +4211,30 @@ dwarf2_read_abbrevs (bfd *abfd, struct d
       abbrev_ptr += bytes_read;
       while (abbrev_name)
 	{
-	  if ((cur_abbrev->num_attrs % ATTR_ALLOC_CHUNK) == 0)
+	  if (cur_abbrev->num_attrs == allocated_attrs)
 	    {
-	      cur_abbrev->attrs = (struct attr_abbrev *)
-		xrealloc (cur_abbrev->attrs,
-			  (cur_abbrev->num_attrs + ATTR_ALLOC_CHUNK)
-			  * sizeof (struct attr_abbrev));
+	      allocated_attrs += ATTR_ALLOC_CHUNK;
+	      cur_attrs
+		= xrealloc (cur_attrs, (allocated_attrs
+					* sizeof (struct attr_abbrev)));
 	    }
-	  cur_abbrev->attrs[cur_abbrev->num_attrs].name = abbrev_name;
-	  cur_abbrev->attrs[cur_abbrev->num_attrs++].form = abbrev_form;
+	  cur_attrs[cur_abbrev->num_attrs].name = abbrev_name;
+	  cur_attrs[cur_abbrev->num_attrs++].form = abbrev_form;
 	  abbrev_name = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
 	  abbrev_ptr += bytes_read;
 	  abbrev_form = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
 	  abbrev_ptr += bytes_read;
 	}
 
+      cur_abbrev->attrs = obstack_alloc (&cu->abbrev_obstack,
+					 (cur_abbrev->num_attrs
+					  * sizeof (struct attr_abbrev)));
+      memcpy (cur_abbrev->attrs, cur_attrs,
+	      cur_abbrev->num_attrs * sizeof (struct attr_abbrev));
+
       hash_number = abbrev_number % ABBREV_HASH_SIZE;
-      cur_abbrev->next = cu_header->dwarf2_abbrevs[hash_number];
-      cu_header->dwarf2_abbrevs[hash_number] = cur_abbrev;
+      cur_abbrev->next = cu->dwarf2_abbrevs[hash_number];
+      cu->dwarf2_abbrevs[hash_number] = cur_abbrev;
 
       /* Get next abbreviation.
          Under Irix6 the abbreviations for a compilation unit are not
@@ -4232,32 +4251,19 @@ dwarf2_read_abbrevs (bfd *abfd, struct d
       if (dwarf2_lookup_abbrev (abbrev_number, cu) != NULL)
 	break;
     }
+
+  xfree (cur_attrs);
 }
 
-/* Empty the abbrev table for a new compilation unit.  */
+/* Release the memory used by the abbrev table for a compilation unit.  */
 
 static void
-dwarf2_empty_abbrev_table (void *ptr_to_abbrevs_table)
+dwarf2_free_abbrev_table (void *ptr_to_cu)
 {
-  int i;
-  struct abbrev_info *abbrev, *next;
-  struct abbrev_info **abbrevs;
-
-  abbrevs = (struct abbrev_info **)ptr_to_abbrevs_table;
+  struct dwarf2_cu *cu = ptr_to_cu;
 
-  for (i = 0; i < ABBREV_HASH_SIZE; ++i)
-    {
-      next = NULL;
-      abbrev = abbrevs[i];
-      while (abbrev)
-	{
-	  next = abbrev->next;
-	  xfree (abbrev->attrs);
-	  xfree (abbrev);
-	  abbrev = next;
-	}
-      abbrevs[i] = NULL;
-    }
+  obstack_free (&cu->abbrev_obstack, NULL);
+  cu->dwarf2_abbrevs = NULL;
 }
 
 /* Lookup an abbrev_info structure in the abbrev hash table.  */
@@ -4265,12 +4271,11 @@ dwarf2_empty_abbrev_table (void *ptr_to_
 static struct abbrev_info *
 dwarf2_lookup_abbrev (unsigned int number, struct dwarf2_cu *cu)
 {
-  struct comp_unit_head *cu_header = &cu->header;
   unsigned int hash_number;
   struct abbrev_info *abbrev;
 
   hash_number = number % ABBREV_HASH_SIZE;
-  abbrev = cu_header->dwarf2_abbrevs[hash_number];
+  abbrev = cu->dwarf2_abbrevs[hash_number];
 
   while (abbrev)
     {
@@ -7585,11 +7590,12 @@ dwarf_alloc_block (void)
 }
 
 static struct abbrev_info *
-dwarf_alloc_abbrev (void)
+dwarf_alloc_abbrev (struct dwarf2_cu *cu)
 {
   struct abbrev_info *abbrev;
 
-  abbrev = (struct abbrev_info *) xmalloc (sizeof (struct abbrev_info));
+  abbrev = (struct abbrev_info *)
+    obstack_alloc (&cu->abbrev_obstack, sizeof (struct abbrev_info));
   memset (abbrev, 0, sizeof (struct abbrev_info));
   return (abbrev);
 }

Re: [RFA/dwarf] Allocate abbrevs from a new obstack

[Jim Blandy]

Daniel Jacobowitz <drow@false.org> writes:
> Another independent performance optimization from my profiling.  This one is
> good for several percent speedup on mainline.  My original description:
> 
>   - It plugs a huge memory leak.  After loading partial symbols for
>   libc, GDB uses 5.6MB memory without this patch and 1.6MB with it.
>   We weren't emptying the abbrev table.
> 
> [Specifically, we were emptying it after dwarf2_build_psymtabs_hard,
> instead of once each time through the loop.  dwarf2_read_abbrevs then
> zeroed out the allocated, unreferenced pointers.]
> 
>   - I spent a lot of time trying to find alternate data structures that
>   would be more efficient.  The expandable hash table is slower, even if
>   you initially allocate it to the right size.  The splay tree I expected
>   to be quite good in this case, but it isn't either; the overhead is simply
>   too high.  The fixed-size ad-hoc hash table appears to be the best we can
>   do.
> 
> Tested on i686-pc-linux-gnu, no regressions.  OK to commit?

Looks great --- please do.

Re: [RFA/dwarf] Allocate abbrevs from a new obstack

[Elena Zannoni]

Jim Blandy writes:
 > 
 > Daniel Jacobowitz <drow@false.org> writes:
 > > Another independent performance optimization from my profiling.  This one is
 > > good for several percent speedup on mainline.  My original description:
 > > 
 > >   - It plugs a huge memory leak.  After loading partial symbols for
 > >   libc, GDB uses 5.6MB memory without this patch and 1.6MB with it.
 > >   We weren't emptying the abbrev table.
 > > 
 > > [Specifically, we were emptying it after dwarf2_build_psymtabs_hard,
 > > instead of once each time through the loop.  dwarf2_read_abbrevs then
 > > zeroed out the allocated, unreferenced pointers.]
 > > 
 > >   - I spent a lot of time trying to find alternate data structures that
 > >   would be more efficient.  The expandable hash table is slower, even if
 > >   you initially allocate it to the right size.  The splay tree I expected
 > >   to be quite good in this case, but it isn't either; the overhead is simply
 > >   too high.  The fixed-size ad-hoc hash table appears to be the best we can
 > >   do.
 > > 
 > > Tested on i686-pc-linux-gnu, no regressions.  OK to commit?
 > 
 > Looks great --- please do.

Jim, there was agreement to not touch these central files until after
the branch.

Re: [RFA/dwarf] Allocate abbrevs from a new obstack

[Daniel Jacobowitz]

On Thu, Feb 26, 2004 at 05:22:21PM -0500, Elena Zannoni wrote:
> Jim Blandy writes:
>  > 
>  > Daniel Jacobowitz <drow@false.org> writes:
>  > > Another independent performance optimization from my profiling.  This one is
>  > > good for several percent speedup on mainline.  My original description:
>  > > 
>  > >   - It plugs a huge memory leak.  After loading partial symbols for
>  > >   libc, GDB uses 5.6MB memory without this patch and 1.6MB with it.
>  > >   We weren't emptying the abbrev table.
>  > > 
>  > > [Specifically, we were emptying it after dwarf2_build_psymtabs_hard,
>  > > instead of once each time through the loop.  dwarf2_read_abbrevs then
>  > > zeroed out the allocated, unreferenced pointers.]
>  > > 
>  > >   - I spent a lot of time trying to find alternate data structures that
>  > >   would be more efficient.  The expandable hash table is slower, even if
>  > >   you initially allocate it to the right size.  The splay tree I expected
>  > >   to be quite good in this case, but it isn't either; the overhead is simply
>  > >   too high.  The fixed-size ad-hoc hash table appears to be the best we can
>  > >   do.
>  > > 
>  > > Tested on i686-pc-linux-gnu, no regressions.  OK to commit?
>  > 
>  > Looks great --- please do.
> 
> Jim, there was agreement to not touch these central files until after
> the branch.

Was there?  The closest I saw was an agreement not to merge intercu
support.  This patch is not particularly important, or the other
optimization patch, but they are independent of that.  I would like to
see the duplicate symbols patch reviewed before the branch.

In any case, I won't commit this one until after the branch - which I
think is coming up in about an hour.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/dwarf] Allocate abbrevs from a new obstack

[Daniel Jacobowitz]

On Thu, Feb 26, 2004 at 05:09:36PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@false.org> writes:
> > Another independent performance optimization from my profiling.  This one is
> > good for several percent speedup on mainline.  My original description:
> > 
> >   - It plugs a huge memory leak.  After loading partial symbols for
> >   libc, GDB uses 5.6MB memory without this patch and 1.6MB with it.
> >   We weren't emptying the abbrev table.
> > 
> > [Specifically, we were emptying it after dwarf2_build_psymtabs_hard,
> > instead of once each time through the loop.  dwarf2_read_abbrevs then
> > zeroed out the allocated, unreferenced pointers.]
> > 
> >   - I spent a lot of time trying to find alternate data structures that
> >   would be more efficient.  The expandable hash table is slower, even if
> >   you initially allocate it to the right size.  The splay tree I expected
> >   to be quite good in this case, but it isn't either; the overhead is simply
> >   too high.  The fixed-size ad-hoc hash table appears to be the best we can
> >   do.
> > 
> > Tested on i686-pc-linux-gnu, no regressions.  OK to commit?
> 
> Looks great --- please do.

OK, I've retested this and checked it in.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA/dwarf] Allocate abbrevs from a new obstack

[Daniel Jacobowitz]

On Thu, Feb 26, 2004 at 05:09:36PM -0500, Jim Blandy wrote:
> 
> Daniel Jacobowitz <drow@false.org> writes:
> > Another independent performance optimization from my profiling.  This one is
> > good for several percent speedup on mainline.  My original description:
> > 
> >   - It plugs a huge memory leak.  After loading partial symbols for
> >   libc, GDB uses 5.6MB memory without this patch and 1.6MB with it.
> >   We weren't emptying the abbrev table.
> > 
> > [Specifically, we were emptying it after dwarf2_build_psymtabs_hard,
> > instead of once each time through the loop.  dwarf2_read_abbrevs then
> > zeroed out the allocated, unreferenced pointers.]
> > 
> >   - I spent a lot of time trying to find alternate data structures that
> >   would be more efficient.  The expandable hash table is slower, even if
> >   you initially allocate it to the right size.  The splay tree I expected
> >   to be quite good in this case, but it isn't either; the overhead is simply
> >   too high.  The fixed-size ad-hoc hash table appears to be the best we can
> >   do.
> > 
> > Tested on i686-pc-linux-gnu, no regressions.  OK to commit?
> 
> Looks great --- please do.

OK, I've retested this and checked it in.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer