From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7037 invoked by alias); 2 Feb 2004 18:22:22 -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 6967 invoked from network); 2 Feb 2004 18:22:20 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 2 Feb 2004 18:22:20 -0000 Received: from drow by nevyn.them.org with local (Exim 4.30 #1 (Debian)) id 1AniiB-0000wQ-DI for ; Mon, 02 Feb 2004 13:22:19 -0500 Date: Mon, 02 Feb 2004 18:22:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: Re: RFA: Don't use obsavestring in dwarf2read Message-ID: <20040202182218.GA3405@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com References: <20040112015726.GA7151@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040112015726.GA7151@nevyn.them.org> User-Agent: Mutt/1.5.1i X-SW-Source: 2004-02/txt/msg00028.txt.bz2 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 * 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); } }