This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Reverse order of find_function_symbols/find_method in linespec.c


Hi.

find_linespec_symbols does two things:

1) Breaks namespace::class::method into namespace::class and method,
   builds a list of all classes (expanding the CUs that define them),
   and then searches for method in that list.
2) If that fails it "falls back" to calling find_function_symbols to
   lookup "namespace::class::method"
   (e.g., for the (anonymous namespace)::function case).

For the (anonymous namespace)::function case this can be REALLY SLOW.
The code will, when performing (1), first expand every CU that contains
something from (anonymous namespace).
There's no need to do (1) for this case,
and that's what the patch here addresses:

http://sourceware.org/ml/gdb-patches/2013-02/msg00041.html

[Keith: I have an improved version of that patch,
but it is supplanted by this patch.]

It turns out that, AFAICT, find_function_symbols works just fine
when looking up namespace::class::method, and (again AFAICT)
this patch results in no change in behaviour.
So we can fix the (anonymous namespace)::function case,
*and* namespace::class::method (where method is in class and not a baseclass)
simply by reversing the order of (1) and (2).

This patch speeds up "b (anonymous namespace)::function" for one
app here from 217 seconds (and significantly worse with cold caches!)
to 0.6 seconds (not unexpected because we're using .gdb_index and all
the code has to effectively do is a lookup in the index's symbol table,
expand that CU, and we're done).

It also speeds up "b namespace::class::~class" mentioned in this patch:

http://sourceware.org/ml/gdb-patches/2013-03/msg00159.html

from 5 seconds to 1 second.
There are still more sources of symbol table inefficiences,
I've added them to the symbol table wiki page:
http://sourceware.org/gdb/wiki/SymbolHandling
And there is still the case of finding method in a baseclass to speed up,
but that's for a separate patch.

No regressions on amd64-linux.
Ok to check in?

2013-03-05  Doug Evans  <dje@google.com>

	* linespec.c (find_linespec_symbols): Call find_function_symbols
	first, and then call lookup_prefix_sym/find_method.

Index: linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.179
diff -u -p -r1.179 linespec.c
--- linespec.c	5 Mar 2013 21:15:34 -0000	1.179
+++ linespec.c	5 Mar 2013 22:28:16 -0000
@@ -3076,7 +3076,6 @@ find_linespec_symbols (struct linespec_s
   char *klass, *method, *canon;
   const char *lookup_name, *last, *p, *scope_op;
   struct cleanup *cleanup;
-  VEC (symbolp) *classes;
   volatile struct gdb_exception except;
 
   cleanup = demangle_for_lookup (name, state->language->la_language,
@@ -3123,7 +3122,7 @@ find_linespec_symbols (struct linespec_s
       return;
     }
 
-  /* NAME points to the class name.
+  /* LOOKUP_NAME points to the class name.
      LAST points to the method name.  */
   klass = xmalloc ((last - lookup_name + 1) * sizeof (char));
   make_cleanup (xfree, klass);
@@ -3136,35 +3135,43 @@ find_linespec_symbols (struct linespec_s
   make_cleanup (xfree, method);
   strcpy (method, last);
 
-  /* Find a list of classes named KLASS.  */
-  classes = lookup_prefix_sym (state, file_symtabs, klass);
-  make_cleanup (VEC_cleanup (symbolp), &classes);
-  if (!VEC_empty (symbolp, classes))
-    {
-      /* Now locate a list of suitable methods named METHOD.  */
-      TRY_CATCH (except, RETURN_MASK_ERROR)
-	{
-	  find_method (state, file_symtabs, klass, method, classes,
-		       symbols, minsyms);
-	}
+  /* It's important to not call expand_symtabs_matching unnecessarily
+     as it can really slow things down (by unnecessarily expanding
+     potentially 1000s of symtabs, which when debugging some apps can
+     cost 100s of seconds).  Avoid this to some extent by *first* calling
+     find_function_symbols, and only if that doesn't find anything
+     *then* call find_method.  This handles two important cases:
+     1) break (anonymous namespace)::foo
+     2) break class::method where method is in class (and not a baseclass)  */
+
+  find_function_symbols (state, file_symtabs, lookup_name, symbols, minsyms);
+
+  if (VEC_empty (symbolp, *symbols)
+      && VEC_empty (minsym_and_objfile_d, *minsyms))
+    {
+      VEC (symbolp) *classes;
+
+      /* Find a list of classes named KLASS.  */
+      classes = lookup_prefix_sym (state, file_symtabs, klass);
+      make_cleanup (VEC_cleanup (symbolp), &classes);
 
-      /* If successful, we're done.  If NOT_FOUND_ERROR
-	 was not thrown, rethrow the exception that we did get.
-	 Otherwise, fall back to looking up the entire name as a symbol.
-	 This can happen with namespace::function.  */
-      if (except.reason >= 0)
+      if (!VEC_empty (symbolp, classes))
 	{
-	  do_cleanups (cleanup);
-	  return;
+	  /* Now locate a list of suitable methods named METHOD.  */
+	  TRY_CATCH (except, RETURN_MASK_ERROR)
+	    {
+	      find_method (state, file_symtabs, klass, method, classes,
+			   symbols, minsyms);
+	    }
+
+	  /* If successful, we're done.  If NOT_FOUND_ERROR
+	     was not thrown, rethrow the exception that we did get.  */
+	  if (except.reason < 0 && except.error != NOT_FOUND_ERROR)
+	    throw_exception (except);
 	}
-      else if (except.error != NOT_FOUND_ERROR)
-	throw_exception (except);
     }
 
-  /* We couldn't find a class, so we check the entire name as a symbol
-     instead.  */
-   find_function_symbols (state, file_symtabs, lookup_name, symbols, minsyms);
-   do_cleanups (cleanup);
+  do_cleanups (cleanup);
 }
 
 /* Return all labels named NAME in FUNCTION_SYMBOLS.  Return the


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]