From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11212 invoked by alias); 8 Feb 2004 04:41:17 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 11203 invoked from network); 8 Feb 2004 04:41:16 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 8 Feb 2004 04:41:16 -0000 Received: from drow by nevyn.them.org with local (Exim 4.30 #1 (Debian)) id 1Apgkt-0007a9-Jy; Sat, 07 Feb 2004 23:41:15 -0500 Date: Sun, 08 Feb 2004 04:41:00 -0000 From: Daniel Jacobowitz To: Elena Zannoni Cc: gdb-patches@sources.redhat.com Subject: Re: RFA: Don't use obsavestring in dwarf2read Message-ID: <20040208044115.GD13033@nevyn.them.org> Mail-Followup-To: Elena Zannoni , gdb-patches@sources.redhat.com References: <20040112015726.GA7151@nevyn.them.org> <20040202182218.GA3405@nevyn.them.org> <16418.39156.566837.685666@localhost.redhat.com> <20040205194821.GA30363@nevyn.them.org> <16418.43222.486879.784465@localhost.redhat.com> <20040205204756.GA2465@nevyn.them.org> <16418.52967.149289.557083@localhost.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <16418.52967.149289.557083@localhost.redhat.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2004-02/txt/msg00170.txt.bz2 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 * 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); } }