Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org, Joel Brobecker <brobecker@adacore.com>
Subject: Re: Performance regression (12x): Re: RFC: add relative file name handling for linespecs
Date: Tue, 31 Jan 2012 20:18:00 -0000	[thread overview]
Message-ID: <m37h07ej9g.fsf@fleche.redhat.com> (raw)
In-Reply-To: <20120119153236.GA6229@host2.jankratochvil.net> (Jan Kratochvil's	message of "Thu, 19 Jan 2012 16:32:36 +0100")

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> I have noticed nightly regression testing to start randomly timing
Jan> out a lot for gdb.ada/* testcases, narrowed it down to this
Jan> check-in.

I looked into this.

I still don't know why my patch exposed this problem, but I don't think
it is actually particular to my patch -- instead, I think it is a latent
bug that the patch exposed.

Profiling gdb on gdb.ada/tasks.exp showed:

 time   seconds   seconds    calls  ms/call  ms/call  name    
 60.67      3.07     3.07 80168399     0.00     0.00  advance_wild_match
  9.49      3.55     0.48  1797312     0.00     0.00  match_partial_symbol
  8.50      3.98     0.43 22218321     0.00     0.00  wild_match
[...]

I tracked this down to ada_iterate_over_symbols calling
ada_lookup_symbol_list.  The former expects to iterate upwards starting
from a given block; but the latter will search all top-level blocks of
all objfiles if no local symbol matches.  This means we are doing an N^2
search, because linespec is also doing this sort of iteration.

This fix seems to fix the issue.  The resulting gdb is still a bit
slower than the status quo ante, but not absurdly so.  The profile now
looks like:

 11.11      0.03     0.03    40318     0.00     0.00  d_demangle
  7.41      0.05     0.02   424963     0.00     0.00  advance_wild_match
  7.41      0.07     0.02   351053     0.00     0.00  hash_continue
[...]


Built and regtested on x86-64 Fedora 15.

Joel, is this ok with you?

Tom

2012-01-31  Tom Tromey  <tromey@redhat.com>

	* ada-lang.c (ada_lookup_symbol_list_full): New function, from
	ada_lookup_symbol_list.  Add 'full_search' argument.
	(ada_lookup_symbol_list): Rewrite in terms of
	ada_lookup_symbol_list_full.
	(ada_iterate_over_symbols): Use ada_lookup_symbol_list_full.

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 55e318f..089f666 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4979,16 +4979,18 @@ add_nonlocal_symbols (struct obstack *obstackp, const char *name,
    the next call of ada_lookup_symbol_list.  Any non-function/non-enumeral 
    symbol match within the nest of blocks whose innermost member is BLOCK0,
    is the one match returned (no other matches in that or
-     enclosing blocks is returned).  If there are any matches in or
-   surrounding BLOCK0, then these alone are returned.  Otherwise, the
-   search extends to global and file-scope (static) symbol tables.
+   enclosing blocks is returned).  If there are any matches in or
+   surrounding BLOCK0, then these alone are returned.  Otherwise, if
+   FULL_SEARCH is non-zero, then the search extends to global and
+   file-scope (static) symbol tables.
    Names prefixed with "standard__" are handled specially: "standard__" 
    is first stripped off, and only static and global symbols are searched.  */
 
-int
-ada_lookup_symbol_list (const char *name0, const struct block *block0,
-                        domain_enum namespace,
-                        struct ada_symbol_info **results)
+static int
+ada_lookup_symbol_list_full (const char *name0, const struct block *block0,
+			     domain_enum namespace,
+			     struct ada_symbol_info **results,
+			     int full_search)
 {
   struct symbol *sym;
   struct block *block;
@@ -5026,7 +5028,7 @@ ada_lookup_symbol_list (const char *name0, const struct block *block0,
 
   ada_add_local_symbols (&symbol_list_obstack, name, block, namespace,
                          wild_match);
-  if (num_defns_collected (&symbol_list_obstack) > 0)
+  if (num_defns_collected (&symbol_list_obstack) > 0 || !full_search)
     goto done;
 
   /* No non-global symbols found.  Check our cache to see if we have
@@ -5070,6 +5072,17 @@ done:
   return ndefns;
 }
 
+/* A wrapper for ada_lookup_symbol_list_full that sets
+   FULL_SEARCH=1.  */
+
+int
+ada_lookup_symbol_list (const char *name0, const struct block *block0,
+			domain_enum namespace,
+			struct ada_symbol_info **results)
+{
+  return ada_lookup_symbol_list_full (name0, block0, namespace, results, 1);
+}
+
 /* If NAME is the name of an entity, return a string that should
    be used to look that entity up in Ada units.  This string should
    be deallocated after use using xfree.
@@ -5106,7 +5119,7 @@ ada_iterate_over_symbols (const struct block *block,
   int ndefs, i;
   struct ada_symbol_info *results;
 
-  ndefs = ada_lookup_symbol_list (name, block, domain, &results);
+  ndefs = ada_lookup_symbol_list_full (name, block, domain, &results, 0);
   for (i = 0; i < ndefs; ++i)
     {
       if (! (*callback) (results[i].sym, data))


  parent reply	other threads:[~2012-01-31 19:59 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-10 16:32 Tom Tromey
2012-01-10 17:40 ` Eli Zaretskii
2012-01-11 21:52   ` Tom Tromey
2012-01-12  6:23     ` Eli Zaretskii
2012-01-16 19:01       ` Tom Tromey
2012-12-08 16:34     ` Jan Kratochvil
2012-12-08 18:33       ` Eli Zaretskii
2013-01-07 15:17         ` [testcase patch] New testcase: DOS drive letters in linespec [Re: RFC: add relative file name handling for linespecs] Jan Kratochvil
2013-01-17 20:42           ` [commit] " Jan Kratochvil
2012-01-11  9:03 ` RFC: add relative file name handling for linespecs Joel Brobecker
2012-01-16 19:29   ` Tom Tromey
2012-01-16 21:07 ` Tom Tromey
2012-01-19 15:39   ` Performance regression (12x): " Jan Kratochvil
2012-01-19 16:35     ` Tom Tromey
2012-01-31 20:18     ` Tom Tromey [this message]
2012-02-01 10:22       ` Joel Brobecker
2012-02-01 15:04         ` Tom Tromey
2012-02-01 15:07         ` Tom Tromey
2012-02-01 15:18           ` Joel Brobecker
2012-02-01 15:35             ` Tom Tromey

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=m37h07ej9g.fsf@fleche.redhat.com \
    --to=tromey@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=jan.kratochvil@redhat.com \
    /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