[RFA] move dwarf2_free_objfile to dwarf2_per_objfile_cleanup

[RFA] move dwarf2_free_objfile to dwarf2_per_objfile_cleanup

[Doug Evans]

Hi.

While working on the dwarf2 mmapped strings issue, I noticed that
there are two routines in dwarf2read.c that are called to release
memory for an objfile.  This patch cleans things up and moves
everything to dwarf2_per_objfile_cleanup.

Ok to check in?

Joel, did you want to have some kind of freeze before the branch?
[Maybe you said so and I missed it.]
This can wait until after the branch.

2009-09-04  Doug Evans  <dje@google.com>

	* symfile.h (dwarf2_free_objfile): Delete.
	* dwarf2read.c (dwarf2_build_psymtabs_hard): Pass dwarf2_per_objfile
	to free_cached_comp_units.
	(free_cached_comp_units): Remove reference to global
	dwarf2_per_objfile, use passed parameter instead.
	(dwarf2_free_objfile): Delete, contents moved to ...
	(dwarf2_per_objfile_cleanup): ... here.
	* coffread.c (coff_symfile_finish): Delete call to dwarf2_free_objfile.
	* elfread.c (elf_symfile_finish): Delete call to dwarf2_free_objfile.

Index: coffread.c
===================================================================
RCS file: /cvs/src/src/gdb/coffread.c,v
retrieving revision 1.95
diff -u -p -r1.95 coffread.c
--- coffread.c	29 Jun 2009 13:18:37 -0000	1.95
+++ coffread.c	4 Sep 2009 23:30:18 -0000
@@ -670,8 +670,6 @@ coff_symfile_finish (struct objfile *obj
 
   /* Let stabs reader clean up */
   stabsread_clear_cache ();
-
-  dwarf2_free_objfile (objfile);
 }
 \f
 
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.325
diff -u -p -r1.325 dwarf2read.c
--- dwarf2read.c	28 Aug 2009 10:49:05 -0000	1.325
+++ dwarf2read.c	4 Sep 2009 23:30:18 -0000
@@ -2049,7 +2049,7 @@ dwarf2_build_psymtabs_hard (struct objfi
 
   /* Any cached compilation units will be linked by the per-objfile
      read_in_chain.  Make sure to free them when we're done.  */
-  back_to = make_cleanup (free_cached_comp_units, NULL);
+  back_to = make_cleanup (free_cached_comp_units, dwarf2_per_objfile);
 
   build_type_psymtabs (objfile);
 
@@ -11535,12 +11548,13 @@ free_stack_comp_unit (void *data)
 /* Free all cached compilation units.  */
 
 static void
-free_cached_comp_units (void *data)
+free_cached_comp_units (void *d)
 {
+  struct dwarf2_per_objfile *data = d;
   struct dwarf2_per_cu_data *per_cu, **last_chain;
 
-  per_cu = dwarf2_per_objfile->read_in_chain;
-  last_chain = &dwarf2_per_objfile->read_in_chain;
+  per_cu = data->read_in_chain;
+  last_chain = &data->read_in_chain;
   while (per_cu != NULL)
     {
       struct dwarf2_per_cu_data *next_cu;
@@ -11620,22 +11634,6 @@ free_one_cached_comp_unit (void *target_
     }
 }
 
-/* Release all extra memory associated with OBJFILE.  */
-
-void
-dwarf2_free_objfile (struct objfile *objfile)
-{
-  dwarf2_per_objfile = objfile_data (objfile, dwarf2_objfile_data_key);
-
-  if (dwarf2_per_objfile == NULL)
-    return;
-
-  /* Cached DIE trees use xmalloc and the comp_unit_obstack.  */
-  free_cached_comp_units (NULL);
-
-  /* Everything else should be on the objfile obstack.  */
-}
-
 /* A pair of DIE offset and GDB type pointer.  We store these
    in a hash table separate from the DIEs, and preserve them
    when the DIEs are flushed out of cache.  */
@@ -11834,12 +11832,14 @@ munmap_section_buffer (struct dwarf2_sec
     }
 }
 
-/* munmap debug sections for OBJFILE, if necessary.  */
+/* Release all extra memory associated with OBJFILE.
+   And munmap debug sections for OBJFILE, if necessary.  */
 
 static void
 dwarf2_per_objfile_cleanup (struct objfile *objfile, void *d)
 {
   struct dwarf2_per_objfile *data = d;
+
   munmap_section_buffer (&data->info);
   munmap_section_buffer (&data->abbrev);
   munmap_section_buffer (&data->line);
@@ -11849,6 +11849,11 @@ dwarf2_per_objfile_cleanup (struct objfi
   munmap_section_buffer (&data->loc);
   munmap_section_buffer (&data->frame);
   munmap_section_buffer (&data->eh_frame);
+
+  /* Cached DIE trees use xmalloc and the comp_unit_obstack.  */
+  free_cached_comp_units (data);
+
+  /* Everything else should be on the objfile obstack.  */
 }
 
 void _initialize_dwarf2_read (void);
Index: elfread.c
===================================================================
RCS file: /cvs/src/src/gdb/elfread.c,v
retrieving revision 1.77
diff -u -p -r1.77 elfread.c
--- elfread.c	30 Apr 2009 21:59:03 -0000	1.77
+++ elfread.c	4 Sep 2009 23:30:18 -0000
@@ -787,8 +787,6 @@ elf_symfile_finish (struct objfile *objf
     {
       xfree (objfile->deprecated_sym_stab_info);
     }
-
-  dwarf2_free_objfile (objfile);
 }
 
 /* ELF specific initialization routine for reading symbols.
Index: symfile.h
===================================================================
RCS file: /cvs/src/src/gdb/symfile.h,v
retrieving revision 1.52
diff -u -p -r1.52 symfile.h
--- symfile.h	27 Aug 2009 21:56:38 -0000	1.52
+++ symfile.h	4 Sep 2009 23:30:18 -0000
@@ -382,8 +382,6 @@ extern int dwarf2_has_info (struct objfi
 extern void dwarf2_build_psymtabs (struct objfile *, int);
 extern void dwarf2_build_frame_info (struct objfile *);
 
-void dwarf2_free_objfile (struct objfile *);
-
 /* From mdebugread.c */
 
 /* Hack to force structures to exist before use in parameter list.  */

Re: [RFA] move dwarf2_free_objfile to dwarf2_per_objfile_cleanup

[Joel Brobecker]

> Joel, did you want to have some kind of freeze before the branch?
> [Maybe you said so and I missed it.]

I generally don't like the idea of an absolute freeze outside of
a branch. That was until I saw the performance improvement patch
from Jan ;-). For now, my plan is to NOT have any formal freeze,
but to ask to hold the patch until after the branch if it makes
us a little nervous.

> This can wait until after the branch.

If any of the GMs feels confident about it, then there is no reason
to wait.  Otherwise, we might be better off to hold it a little.

-- 
Joel

Re: [RFA] move dwarf2_free_objfile to dwarf2_per_objfile_cleanup

[Jan Kratochvil]

On Sat, 05 Sep 2009 02:08:05 +0200, Joel Brobecker wrote:
> That was until I saw the performance improvement patch from Jan ;-).

As during its development there were some unexpected regressions I understand
it may be more suitable for 7.1.  If 7.0 is going to happen ;-).


Regards,
Jan

Re: [RFA] move dwarf2_free_objfile to dwarf2_per_objfile_cleanup

[Joel Brobecker]

> 2009-09-04  Doug Evans  <dje@google.com>
> 
> 	* symfile.h (dwarf2_free_objfile): Delete.
> 	* dwarf2read.c (dwarf2_build_psymtabs_hard): Pass dwarf2_per_objfile
> 	to free_cached_comp_units.
> 	(free_cached_comp_units): Remove reference to global
> 	dwarf2_per_objfile, use passed parameter instead.
> 	(dwarf2_free_objfile): Delete, contents moved to ...
> 	(dwarf2_per_objfile_cleanup): ... here.
> 	* coffread.c (coff_symfile_finish): Delete call to dwarf2_free_objfile.
> 	* elfread.c (elf_symfile_finish): Delete call to dwarf2_free_objfile.

Not really an expert in this area, but does it look like you are
re-reading the per-objfile data a second time when converting from
a psymtab to a symtab? On the plus side, I really find it makes
the code clearer to understand - it's a bit hard to figure out
the lifetime of this global... Was the memory every released before?
(I see two calls to sym_finish, one when free_objfile is called,
and one when reread_symbols is called - these two are not necessarily
called after the symtab has been computed)

-- 
Joel