From: Doug Evans <xdje42@gmail.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 0/5] cp-namespace.c cleanup pass
Date: Mon, 15 Dec 2014 05:50:00 -0000 [thread overview]
Message-ID: <m37fxtihsy.fsf@seba.sebabeach.org> (raw)
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/ *(.*$//" </tmp/gdb-call.log >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/ *(.*$//" </tmp/gdb-call.log >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 <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include <stdarg.h>
#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)
next reply other threads:[~2014-12-15 5:50 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-15 5:50 Doug Evans [this message]
2014-12-22 17:33 ` Doug Evans
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m37fxtihsy.fsf@seba.sebabeach.org \
--to=xdje42@gmail.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).