[RFA] massively speed up "info var foo" on large programs

[Doug Evans]

Hi.

I'm not entirely sure this patch is correct, but it feels correct (*1),
and is a massive win.
"info var Task" in one large program goes from 350 seconds to 28 seconds.

The problem is the two loops over all minsyms of all objfiles.
The call to lookup_symbol looks up all those minsyms in all symtabs
of all objfiles.
Given an early test in the code of where the minsym lives
(e.g. for variables the code checks mst_data, mst_bss, mst_file_data,
mst_file_bss), it doesn't feel right to look in other objfiles.
It's possible there's a good reason though.  Alas I can't think of one
(I'm probably just blinded by the massive speedup ...).

This issue has been looked at before.
E.g., http://sourceware.org/ml/gdb-patches/2011-09/msg00461.html

(*1): There's a FIXME in the patch that I want to remove prior to check-in,
I'm just not sure what the right fix is.
For functions: why call both find_pc_symtab and lookup_symbol?
The patch leaves the code as is, so I could just change the FIXME
into a comment explaining why both calls are made.
I'd rather remove the call to find_pc_symtab (in the second minsym loop).
An alternative is to remove both find_pc_symtab calls and just
call lookup_msymbol_in_objfile for variables and functions,
I'm guessing the find_pc_symtab is used in the first minsym loop because
it's faster, but I wish that weren't just a guess.
[Gotta love comments that explain why things are they way they are. :-)]

One thing the patch does is change the test in the first minsym loop
to not call find_pc_symtab for variables.  I can put the test back,
but the call seems unnecessary for variables given that lookup_symbol
(or now lookup_msymbol_in_objfile) is also called.

The patch gives identical results for the cases I tried (with multi-1000 line
output), which raises my confidence that it's correct, but not entirely so.

Regression-tested on amd64-linux.

What's the right way to fix the FIXME?
And ok to check in after that's fixed?

2012-05-23  Doug Evans  <dje@google.com>

	* symtab.c (lookup_msymbol_in_objfile): New function.
	(search_symbols): Call it.

Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.306
diff -u -p -r1.306 symtab.c
--- symtab.c	24 May 2012 02:51:48 -0000	1.306
+++ symtab.c	24 May 2012 04:19:31 -0000
@@ -1559,6 +1559,30 @@ lookup_symbol_aux_symtabs (int block_ind
   return NULL;
 }
 
+/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
+   Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE.  */
+
+static struct symbol *
+lookup_msymbol_in_objfile (struct objfile *objfile,
+			   struct minimal_symbol *msymbol,
+			   domain_enum domain)
+{
+  const char *name = SYMBOL_LINKAGE_NAME (msymbol);
+  enum language lang = current_language->la_language;
+  const char *modified_name;
+  struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name);
+  struct symbol *returnval;
+
+  returnval = lookup_symbol_aux_objfile (objfile, GLOBAL_BLOCK,
+					 modified_name, domain);
+  if (returnval == NULL)
+    returnval = lookup_symbol_aux_objfile (objfile, STATIC_BLOCK,
+					   modified_name, domain);
+
+  do_cleanups (cleanup);
+  return returnval;
+}
+
 /* A helper function for lookup_symbol_aux that interfaces with the
    "quick" symbol table functions.  */
 
@@ -3463,21 +3487,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
-		  {
-		    /* FIXME: carlton/2003-02-04: Given that the
-		       semantics of lookup_symbol keeps on changing
-		       slightly, it would be a nice idea if we had a
-		       function lookup_symbol_minsym that found the
-		       symbol associated to a given minimal symbol (if
-		       any).  */
-		    if (kind == FUNCTIONS_DOMAIN
-			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-					  (struct block *) NULL,
-					  VAR_DOMAIN, 0)
-			== NULL)
-		      found_misc = 1;
-		  }
+		/* Note: An important side-effect of these lookup functions
+		   is to expand the symbol table if msymbol is found.  */
+		if (kind == FUNCTIONS_DOMAIN
+		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
+		    : lookup_msymbol_in_objfile (objfile, msymbol,
+						 VAR_DOMAIN) == NULL)
+		  found_misc = 1;
 	      }
 	  }
       }
@@ -3570,13 +3586,13 @@ search_symbols (char *regexp, enum searc
 			    NULL, 0) == 0)
 	      {
 		/* Functions:  Look up by address.  */
-		if (kind != FUNCTIONS_DOMAIN ||
-		    (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
+		if (kind != FUNCTIONS_DOMAIN
+		    || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL)
 		  {
 		    /* Variables/Absolutes:  Look up by name.  */
-		    if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-				       (struct block *) NULL, VAR_DOMAIN, 0)
-			 == NULL)
+		    /* FIXME: Why do we also look up fns by name?  */
+		    if (lookup_msymbol_in_objfile (objfile, msymbol,
+						   VAR_DOMAIN) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)