From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12958 invoked by alias); 27 May 2019 06:46:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 12946 invoked by uid 89); 27 May 2019 06:46:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 May 2019 06:46:53 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 615CAAE2F for ; Mon, 27 May 2019 06:46:51 +0000 (UTC) Subject: [PING][PATCH][gdb/symtab] Fix symbol loading performance regression From: Tom de Vries To: gdb-patches@sourceware.org References: <20190513092700.GA9733@delia> Message-ID: <949dd5d8-4855-24da-b1bb-c6f9ba0b586b@suse.de> Date: Mon, 27 May 2019 06:46:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190513092700.GA9733@delia> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg00587.txt.bz2 On 13-05-19 11:27, Tom de Vries wrote: > Hi, > > The commit "[gdb/symtab] Fix language of duplicate static minimal symbol" > introduces a performance regression, when loading a cc1 executable build with > -O0 -g and gcc 7.4.0. The performance regression, measured in 'real' time is > about 175%. > > The slower execution comes from the fact that the fix in symbol_set_names > makes the call to symbol_find_demangled_name unconditional. > > Fix this by reverting the commit, and redoing the fix as follows. > > Recapturing the original problem, the first time symbol_set_names is called > with gsymbol.language == lang_auto and linkage_name == "_ZL3foov", the name is > not present in the per_bfd->demangled_names_hash hash table, so > symbol_find_demangled_name is called to demangle the name, after which the > mangled/demangled pair is added to the hashtable. The call to > symbol_find_demangled_name also sets gsymbol.language to lang_cplus. > The second time symbol_set_names is called with gsymbol.language == lang_auto > and linkage_name == "_ZL3foov", the name is present in the hash table, so the > demangled name from the hash table is used. However, the language of the > symbol remains lang_auto. > > Fix this by adding a field language in struct demangled_name_entry, and using > the field in symbol_set_names to set the language of gsymbol, if necessary. > > Tested on x86_64-linux. > > OK for trunk? > Ping. Thanks, - Tom > [gdb/symtab] Fix symbol loading performance regression > > gdb/ChangeLog: > > 2019-05-11 Tom de Vries > > PR symtab/24545 > * symtab.c (struct demangled_name_entry): Add language field. > (symbol_set_names): Revert "[gdb/symtab] Fix language of duplicate > static minimal symbol". Set and use language field. > > --- > gdb/symtab.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 130d5cd48f..44964533ee 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -713,6 +713,7 @@ symbol_set_language (struct general_symbol_info *gsymbol, > struct demangled_name_entry > { > const char *mangled; > + ENUM_BITFIELD(language) language : LANGUAGE_BITS; > char demangled[1]; > }; > > @@ -853,11 +854,6 @@ symbol_set_names (struct general_symbol_info *gsymbol, > else > linkage_name_copy = linkage_name; > > - /* Set the symbol language. */ > - char *demangled_name_ptr > - = symbol_find_demangled_name (gsymbol, linkage_name_copy); > - gdb::unique_xmalloc_ptr demangled_name (demangled_name_ptr); > - > entry.mangled = linkage_name_copy; > slot = ((struct demangled_name_entry **) > htab_find_slot (per_bfd->demangled_names_hash.get (), > @@ -870,7 +866,9 @@ symbol_set_names (struct general_symbol_info *gsymbol, > || (gsymbol->language == language_go > && (*slot)->demangled[0] == '\0')) > { > - int demangled_len = demangled_name ? strlen (demangled_name.get ()) : 0; > + char *demangled_name = symbol_find_demangled_name (gsymbol, > + linkage_name_copy); > + int demangled_len = demangled_name ? strlen (demangled_name) : 0; > > /* Suppose we have demangled_name==NULL, copy_name==0, and > linkage_name_copy==linkage_name. In this case, we already have the > @@ -906,12 +904,19 @@ symbol_set_names (struct general_symbol_info *gsymbol, > strcpy (mangled_ptr, linkage_name_copy); > (*slot)->mangled = mangled_ptr; > } > + (*slot)->language = gsymbol->language; > > if (demangled_name != NULL) > - strcpy ((*slot)->demangled, demangled_name.get()); > + { > + strcpy ((*slot)->demangled, demangled_name); > + xfree (demangled_name); > + } > else > (*slot)->demangled[0] = '\0'; > } > + else if (gsymbol->language == language_unknown > + || gsymbol->language == language_auto) > + gsymbol->language = (*slot)->language; > > gsymbol->name = (*slot)->mangled; > if ((*slot)->demangled[0] != '\0') >