Re: RFA: Don't use obsavestring in dwarf2read

Daniel Jacobowitz
2004-02-08T04:41:17Z

On Thu, Feb 05, 2004 at 06:16:55PM -0500, Elena Zannoni wrote:
> Daniel Jacobowitz writes:
> > On Thu, Feb 05, 2004 at 03:34:30PM -0500, Elena Zannoni wrote:
> > > > The obstacks themselves are probably a good idea. Once upon a time,
> > > > Peter informed me, there was a plan to free the psymbol obstack when
> > > > all symbols had been read in; but that doesn't seem like a useful
> > > > optimization, and I can't think offhand of any use for separate symbol
> > > > and type obstacks. I wouldn't object to having a per-objfile obstack
> > > > instead, and un-seperating them.
> > >
> > > I think it would be worthwhile to see how much doing that would save us.
> >
> > Well, it wouldn't save anything by itself - there's immeasurable
> > overhead to the obstacks. It would let us eliminate this sort of
> > duplication, but they're pretty tricky to identify; it took me a couple
> > of hours to convince myself about this set of 'em.
> >
>
> I meant in general, yes. Since the possible 'shorcuts' are difficult
> to identify, and they are only for dwarf2 (that I've looked at) I am
> bit worried about the cross pointers. I am thinking to kill the triad.

Now that you've done this, I've respun the patch. I also had to remove
two consts - the value returns by DW_STRING should be const in an ideal
world, but that won't work until the symbol and type names are const
also. I also re-ran tests, just in case - no regressions.

FYI, I just tested this with straight C - debug information for glibc's
libc.so.6 (also with -readnow). It's good for 10MB savings out of a
total of 120MB used. Not bad.

Is this version OK?

--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer

2004-02-07 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.133
diff -u -p -r1.133 dwarf2read.c
--- dwarf2read.c 7 Feb 2004 23:13:47 -0000 1.133
+++ dwarf2read.c 8 Feb 2004 04:31:38 -0000
@@ -55,6 +55,16 @@
#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 objfile_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). */
+
#if 0
/* .debug_info header for a compilation unit
Because of alignment constraints, this structure has padding and cannot
@@ -2665,8 +2675,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->objfile_obstack);
+ fp->name = fieldname;

/* Change accessibility for artificial fields (e.g. virtual table
pointer or virtual base class pointer) to private. */
@@ -2697,11 +2706,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->objfile_obstack));
+ SET_FIELD_PHYSNAME (*fp, physname ? physname : "");
FIELD_TYPE (*fp) = die_type (die, cu);
- FIELD_NAME (*fp) = obsavestring (fieldname, strlen (fieldname),
- &objfile->objfile_obstack);
+ FIELD_NAME (*fp) = fieldname;
}
else if (die->tag == DW_TAG_inheritance)
{
@@ -2869,8 +2876,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->objfile_obstack);
+ fnp->physname = physname ? physname : "";
fnp->type = alloc_type (objfile);
if (die->type && TYPE_CODE (die->type) == TYPE_CODE_FUNC)
{
@@ -3001,7 +3007,7 @@ read_structure_scope (struct die_info *d
struct objfile *objfile = cu->objfile;
struct type *type;
struct attribute *attr;
- const char *name = NULL;
+ char *name = NULL;
const char *previous_prefix = processing_current_prefix;
struct cleanup *back_to = NULL;
/* This says whether or not we want to try to update the structure's
@@ -3046,8 +3052,7 @@ read_structure_scope (struct die_info *d
}
else
{
- TYPE_TAG_NAME (type) = obsavestring (name, strlen (name),
- &objfile->objfile_obstack);
+ TYPE_TAG_NAME (type) = name;
need_to_update_name = (cu->language == language_cplus);
}
}
@@ -3252,7 +3257,7 @@ read_enumeration (struct die_info *die,
attr = dwarf2_attr (die, DW_AT_name, cu);
if (attr && DW_STRING (attr))
{
- const char *name = DW_STRING (attr);
+ char *name = DW_STRING (attr);

if (processing_has_namespace_info)
{
@@ -3263,10 +3268,7 @@ read_enumeration (struct die_info *die,
name);
}
else
- {
- TYPE_TAG_NAME (type) = obsavestring (name, strlen (name),
- &objfile->objfile_obstack);
- }
+ TYPE_TAG_NAME (type) = name;
}

attr = dwarf2_attr (die, DW_AT_byte_size, cu);
@@ -5678,11 +5680,8 @@ 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->objfile_obstack);
+ arises further down in this function.) */
+ SYMBOL_LINKAGE_NAME (sym) = TYPE_TAG_NAME (type);
}
}

@@ -5714,10 +5713,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->objfile_obstack);
+ TYPE_NAME (SYMBOL_TYPE (sym)) = SYMBOL_NATURAL_NAME (sym);
add_symbol_to_list (typedef_sym, list_to_add);
}
}