Re: RFA: Don't use obsavestring in dwarf2read

[Daniel Jacobowitz <drow at mvista dot com>]

On Sun, Jan 11, 2004 at 08:57:26PM -0500, Daniel Jacobowitz wrote:
> This patch is pretty self-explanatory, and pretty effective: With -readnow
> to force immediate loading of full symbols, this is good for 3% startup time
> and 30% memory savings (that's 100MB out of 330MB!) for a gdb session
> against "monotone".  We already rely on the lifetimes of this data, so
> there's no point in duplicating it onto another obstack with the exact same
> lifetime.
> 
> OK?
> 
> [My current C++ work may have significant memory and startup time impact. 
> I'm trying to clean house at the same time, so that I don't introduce a net
> loss.  This is low-hanging fruit; higher-hanging fruit will take somewhat
> longer.]

Updated for Michael's comments, and to fix merge issues (and a new
introduction of obsavestring).  I also updated the leading comment to
mention that symbols and types can now point into each other's
obstacks.

OK?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

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

	* dwarf2read.c: Add comment describing memory lifetimes.
	(dwarf2_add_field, dwarf2_add_member_fn, read_structure_scope)
	(read_enumeration, new_symbol): Don't use obsavestring.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.130
diff -u -p -r1.130 dwarf2read.c
--- dwarf2read.c	28 Jan 2004 18:43:06 -0000	1.130
+++ dwarf2read.c	2 Feb 2004 18:19:45 -0000
@@ -55,6 +55,21 @@
 #define DWARF2_REG_TO_REGNUM(REG) (REG)
 #endif
 
+/* A note on memory usage: at the present time, this code reads the debug
+   info sections into the objfile's psymbol_obstack.  A definite improvement
+   for startup time, on platforms which do not emit relocations for debug
+   sections, would be to use mmap instead.
+   
+   In either case, the sections should remain loaded until the objfile is
+   released, and pointers into the section data can be used for any other
+   data associated to the objfile (symbol names, type names, location expressions
+   to name a few).
+
+   Also, this code occasionally sets symbol names to point at type names, and
+   vice versa.  This can cause symbols pointing into the type obstack and
+   types pointing into the symbol obstack.  They have the same lifetime
+   in current GDB so this is harmless, and prevents wasteful duplication.  */
+
 #if 0
 /* .debug_info header for a compilation unit
    Because of alignment constraints, this structure has padding and cannot
@@ -2665,8 +2680,7 @@ dwarf2_add_field (struct field_info *fip
       attr = dwarf2_attr (die, DW_AT_name, cu);
       if (attr && DW_STRING (attr))
 	fieldname = DW_STRING (attr);
-      fp->name = obsavestring (fieldname, strlen (fieldname),
-			       &objfile->type_obstack);
+      fp->name = fieldname;
 
       /* Change accessibility for artificial fields (e.g. virtual table
          pointer or virtual base class pointer) to private.  */
@@ -2697,11 +2711,9 @@ dwarf2_add_field (struct field_info *fip
       /* Get physical name.  */
       physname = dwarf2_linkage_name (die, cu);
 
-      SET_FIELD_PHYSNAME (*fp, obsavestring (physname, strlen (physname),
-					     &objfile->type_obstack));
+      SET_FIELD_PHYSNAME (*fp, physname ? physname : "");
       FIELD_TYPE (*fp) = die_type (die, cu);
-      FIELD_NAME (*fp) = obsavestring (fieldname, strlen (fieldname),
-				       &objfile->type_obstack);
+      FIELD_NAME (*fp) = fieldname;
     }
   else if (die->tag == DW_TAG_inheritance)
     {
@@ -2869,8 +2881,7 @@ dwarf2_add_member_fn (struct field_info 
 
   /* Fill in the member function field info.  */
   fnp = &new_fnfield->fnfield;
-  fnp->physname = obsavestring (physname, strlen (physname),
-				&objfile->type_obstack);
+  fnp->physname = physname ? physname : "";
   fnp->type = alloc_type (objfile);
   if (die->type && TYPE_CODE (die->type) == TYPE_CODE_FUNC)
     {
@@ -3046,8 +3057,7 @@ read_structure_scope (struct die_info *d
 	}
       else
 	{
-	  TYPE_TAG_NAME (type) = obsavestring (name, strlen (name),
-					       &objfile->type_obstack);
+	  TYPE_TAG_NAME (type) = name;
 	  need_to_update_name = (cu->language == language_cplus);
 	}
     }
@@ -3263,10 +3273,7 @@ read_enumeration (struct die_info *die, 
 					   name);
 	}
       else
-	{
-	  TYPE_TAG_NAME (type) = obsavestring (name, strlen (name),
-					       &objfile->type_obstack);
-	}
+	TYPE_TAG_NAME (type) = name;
     }
 
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
@@ -5679,10 +5686,7 @@ new_symbol (struct die_info *die, struct
 		  /* FIXME: carlton/2003-11-10: Should this use
 		     SYMBOL_SET_NAMES instead?  (The same problem also
 		     arises a further down in the function.)  */
-		  SYMBOL_LINKAGE_NAME (sym)
-		    = obsavestring (TYPE_TAG_NAME (type),
-				    strlen (TYPE_TAG_NAME (type)),
-				    &objfile->symbol_obstack);
+		  SYMBOL_LINKAGE_NAME (sym) = TYPE_TAG_NAME (type);
 		}
 	    }
 
@@ -5714,10 +5718,7 @@ new_symbol (struct die_info *die, struct
 		*typedef_sym = *sym;
 		SYMBOL_DOMAIN (typedef_sym) = VAR_DOMAIN;
 		if (TYPE_NAME (SYMBOL_TYPE (sym)) == 0)
-		  TYPE_NAME (SYMBOL_TYPE (sym)) =
-		    obsavestring (SYMBOL_NATURAL_NAME (sym),
-				  strlen (SYMBOL_NATURAL_NAME (sym)),
-				  &objfile->type_obstack);
+		  TYPE_NAME (SYMBOL_TYPE (sym)) = SYMBOL_NATURAL_NAME (sym);
 		add_symbol_to_list (typedef_sym, list_to_add);
 	      }
 	  }