Re: RFA: Don't use obsavestring in dwarf2read

Daniel Jacobowitz
2004-02-02T18:22:22Z

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);
}
}