From patchwork Mon Dec 15 05:49:17 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 4244 Received: (qmail 17177 invoked by alias); 15 Dec 2014 05:50:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 17164 invoked by uid 89); 15 Dec 2014 05:50:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f52.google.com Received: from mail-pa0-f52.google.com (HELO mail-pa0-f52.google.com) (209.85.220.52) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 15 Dec 2014 05:50:13 +0000 Received: by mail-pa0-f52.google.com with SMTP id eu11so11045831pac.25 for ; Sun, 14 Dec 2014 21:50:11 -0800 (PST) X-Received: by 10.67.23.134 with SMTP id ia6mr48833568pad.50.1418622611275; Sun, 14 Dec 2014 21:50:11 -0800 (PST) Received: from sspiff.org (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id co1sm8010737pbc.22.2014.12.14.21.50.08 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 14 Dec 2014 21:50:10 -0800 (PST) Received: by sspiff.org (sSMTP sendmail emulation); Sun, 14 Dec 2014 21:49:17 -0800 From: Doug Evans To: gdb-patches@sourceware.org Subject: [PATCH 0/5] cp-namespace.c cleanup pass Date: Sun, 14 Dec 2014 21:49:17 -0800 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi. This patch set is a cleanup pass over cp-namespace.c. There are two things I wish to achieve here: 1a) Remove redundancy to make the code more readable. There are several redundant lookups here, and it's hard to reason about the code because one has to ask if there's something one is missing or whether the lookup really is redundant. Plus it's more code to have to follow while studying the flow of any particular function. 1b) Performance. While redundant lookups don't always hurt performance if a symbol is found, efficient handling of the null case (symbol not found) is at least as important. 2) Lay the groundwork so that I can finish this patch, that adds support for looking up primitive types as symbols: https://sourceware.org/ml/gdb-patches/2014-12/msg00169.html 1/5: whitespace 2/5: simplify cp_lookup_symbol_in_namespace 3/5: introduce cp_lookup_nested_symbol_1 4/5: remove redundant calls to cp_lookup_symbol_in_namespace 5/5: remove redundancies in cp_lookup_symbol_nonlocal Appended is a patch I wrote to log symbol lookup calls while running the testsuite, and here are the before/after numbers: [dje@seba gdb]$ sed -e "s/ *(.*$//" before.tmp [dje@seba gdb]$ sort before.tmp | uniq -c 40230 basic_lookup_symbol_nonlocal 74647 lookup_global_symbol 18834 lookup_language_this 73063 lookup_local_symbol 30849 lookup_static_symbol 73063 lookup_symbol_aux 59411 lookup_symbol_in_block 656240 lookup_symbol_in_objfile 23 lookup_symbol_in_objfile_from_linkage_name 656298 lookup_symbol_in_objfile_symtabs 74075 lookup_symbol_in_static_block 614899 lookup_symbol_via_quick_fns [dje@seba gdb]$ sed -e "s/ *(.*$//" after.tmp [dje@seba gdb]$ sort after.tmp | uniq -c 39019 basic_lookup_symbol_nonlocal 61959 lookup_global_symbol 9614 lookup_language_this 71826 lookup_local_symbol 29565 lookup_static_symbol 71826 lookup_symbol_aux 55214 lookup_symbol_in_block 504688 lookup_symbol_in_objfile 23 lookup_symbol_in_objfile_from_linkage_name 504746 lookup_symbol_in_objfile_symtabs 63217 lookup_symbol_in_static_block 463884 lookup_symbol_via_quick_fns There's a clear reduction in the number of symbol looks being done here. A later patch will augment our performance testsuite, but my main focus at the moment is getting to the "use the index better" speedup patch. [And along the way cleaning things up so that I understand the code better, and *hopefully* making it so that others will too.] Regression tested on amd64-linux. I still need to do a bit more testing, but it's at the point where it could use another pair of eyes. diff --git a/gdb/symtab.c b/gdb/symtab.c index 54e4be4..85eaf10 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -18,6 +18,7 @@ along with this program. If not, see . */ #include "defs.h" +#include #include "symtab.h" #include "gdbtypes.h" #include "gdbcore.h" @@ -1264,6 +1265,26 @@ demangle_for_lookup (const char *name, enum language lang, return cleanup; } +extern void log_call (const char *fmt, ...); + +void +log_call (const char *fmt, ...) +{ + static FILE *f; + va_list args; + + if (f == NULL) + f = fopen ("/tmp/gdb-call.log", "a"); + if (f == NULL) + abort (); + + va_start (args, fmt); + vfprintf (f, fmt, args); + va_end (args); + fputc ('\n', f); + fflush (f); +} + /* See symtab.h. This function (or rather its subordinates) have a bunch of loops and @@ -1310,6 +1331,8 @@ struct symbol * lookup_language_this (const struct language_defn *lang, const struct block *block) { + log_call ("lookup_language_this (...)"); + if (lang->la_name_of_this == NULL || block == NULL) return NULL; @@ -1387,6 +1410,8 @@ lookup_symbol_aux (const char *name, const struct block *block, struct symbol *sym; const struct language_defn *langdef; + log_call ("lookup_symbol_aux (%s, ...)", name); + /* Make sure we do something sensible with is_a_field_of_this, since the callers that set this parameter to some non-null value will certainly use it later. If we don't set it, the contents of @@ -1458,6 +1483,8 @@ lookup_local_symbol (const char *name, const struct block *block, struct symbol *sym; const struct block *static_block = block_static_block (block); const char *scope = block_scope (block); + + log_call ("lookup_local_symbol (%s, ...)", name); /* Check if either no block is specified or it's a global block. */ @@ -1522,6 +1549,8 @@ lookup_symbol_in_block (const char *name, const struct block *block, { struct symbol *sym; + log_call ("lookup_symbol_in_block (%s, ...)", name); + sym = block_lookup_symbol (block, name, domain); if (sym) { @@ -1541,6 +1570,8 @@ lookup_global_symbol_from_objfile (struct objfile *main_objfile, { struct objfile *objfile; + log_call ("lookup_global_symbol_from_objfile (%s, ...)", name); + for (objfile = main_objfile; objfile; objfile = objfile_separate_debug_iterate (main_objfile, objfile)) @@ -1568,6 +1599,8 @@ lookup_symbol_in_objfile_symtabs (struct objfile *objfile, int block_index, gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK); + log_call ("lookup_symbol_in_objfile_symtabs (%s, ...)", name); + ALL_OBJFILE_COMPUNITS (objfile, cust) { const struct blockvector *bv; @@ -1607,6 +1640,8 @@ lookup_symbol_in_objfile_from_linkage_name (struct objfile *objfile, &modified_name); struct objfile *main_objfile, *cur_objfile; + log_call ("lookup_symbol_in_objfile_from_linkage_name (%s, ...)", linkage_name); + if (objfile->separate_debug_objfile_backlink) main_objfile = objfile->separate_debug_objfile_backlink; else @@ -1663,6 +1698,8 @@ lookup_symbol_via_quick_fns (struct objfile *objfile, int block_index, const struct block *block; struct symbol *sym; + log_call ("lookup_symbol_via_quick_fns (%s, ...)", name); + if (!objfile->sf) return NULL; cust = objfile->sf->qf->lookup_symbol (objfile, block_index, name, domain); @@ -1719,6 +1756,8 @@ basic_lookup_symbol_nonlocal (const char *name, the current objfile. Searching the current objfile first is useful for both matching user expectations as well as performance. */ + log_call ("basic_lookup_symbol_nonlocal (%s, ...)", name); + sym = lookup_symbol_in_static_block (name, block, domain); if (sym != NULL) return sym; @@ -1735,6 +1774,8 @@ lookup_symbol_in_static_block (const char *name, { const struct block *static_block = block_static_block (block); + log_call ("lookup_symbol_in_static_block (%s, ...)", name); + if (static_block != NULL) return lookup_symbol_in_block (name, static_block, domain); else @@ -1752,6 +1793,8 @@ lookup_symbol_in_objfile (struct objfile *objfile, int block_index, { struct symbol *result; + log_call ("lookup_symbol_in_objfile (%s, ...)", name); + result = lookup_symbol_in_objfile_symtabs (objfile, block_index, name, domain); if (result == NULL) @@ -1771,6 +1814,8 @@ lookup_static_symbol (const char *name, const domain_enum domain) struct objfile *objfile; struct symbol *result; + log_call ("lookup_static_symbol (%s, ...)", name); + ALL_OBJFILES (objfile) { result = lookup_symbol_in_objfile (objfile, STATIC_BLOCK, name, domain); @@ -1829,6 +1874,8 @@ lookup_global_symbol (const char *name, struct objfile *objfile = NULL; struct global_sym_lookup_data lookup_data; + log_call ("lookup_global_symbol (%s, ...)", name); + /* Call library-specific lookup procedure. */ objfile = lookup_objfile_from_block (block); if (objfile != NULL)